Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change type Buffer into Uint8Array in publishMessage #1707

Open
seo-rii opened this issue Apr 11, 2023 · 2 comments
Open

Change type Buffer into Uint8Array in publishMessage #1707

seo-rii opened this issue Apr 11, 2023 · 2 comments
Assignees
Labels
api: pubsub Issues related to the googleapis/nodejs-pubsub API. priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.

Comments

@seo-rii
Copy link

seo-rii commented Apr 11, 2023

Thanks for stopping by to let us know something could be better!

PLEASE READ: If you have a support contract with Google, please create an issue in the support console instead of filing on GitHub. This will ensure a timely response.

Is your feature request related to a problem? Please describe.
In publishMessage method of Topic, it checks if data is instance of Buffer.
Some of modern serialization library uses Uint8Array as return value instead of Buffer. Although we can change it into Buffer using Buffer.from, it will copy Arraybuffer, which causes performance issue.
Since Buffer is subclass of Uint8Array, I think it can be changed into data && !(data instanceof Uint8Array).

Describe the solution you'd like
Change src/publisher/index.ts L195 into if (data && !(data instanceof Uint8Array)) {.

Describe alternatives you've considered
We can change Uint8Array into Buffer using Buffer.frommethod, but it will 'copy' array and create new one.

Additional context
.

@seo-rii seo-rii added priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design. labels Apr 11, 2023
@product-auto-label product-auto-label bot added the api: pubsub Issues related to the googleapis/nodejs-pubsub API. label Apr 11, 2023
@seo-rii seo-rii changed the title Change type buffer into Uint8Array in publishMessage Change type Buffer into Uint8Array in publishMessage Apr 11, 2023
@feywind
Copy link
Collaborator

feywind commented Apr 12, 2023

@seo-rii Ah, interesting, that doesn't seem unreasonable, though we'd have to make sure the gapic classes will also work with array buffers. I'm curious, if you don't mind sharing more detail, what your code's general workflow is like, that it would benefit from the fix. I'm asking mostly because we're working on a v2 version of the API and I'd like to make sure the common use cases are covered.

@seo-rii
Copy link
Author

seo-rii commented Apr 13, 2023

I'm trying to deliver a message encoded in protobuf to pubsub and deliver that binary message to all users in same channel. Like this : (await topic(chan)).publishMessage({data: encode(obj)});
But the value returned by protobuf.js module is Uint8Array, the code must be modified as follows.

const m = encode(obj), data = Buffer.alloc(m.byteLength);
for (let i = 0; i < m.byteLength; i++) data[i] = m[i];
await (await topic(chan)).publishMessage({data});

This creates a copy operation that looks very unnecessary, and adversely affects both code execution time and memory usage.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: pubsub Issues related to the googleapis/nodejs-pubsub API. priority: p3 Desirable enhancement or fix. May not be included in next release. type: feature request ‘Nice-to-have’ improvement, new feature or different behavior or design.
Projects
None yet
Development

No branches or pull requests

2 participants