googleapis / nodejs-pubsub

Node.js client for Google Cloud Pub/Sub: Ingest event streams from anywhere, at any scale, for simple, reliable, real-time stream analytics.
https://cloud.google.com/pubsub/
Apache License 2.0
521 stars 227 forks source link

Change type Buffer into Uint8Array in publishMessage #1707

Open seo-rii opened 1 year ago

seo-rii commented 1 year ago

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 .

feywind commented 1 year ago

@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 commented 1 year ago

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.