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

numbers.js cache excessive memory usage #139

Open
David-Parker opened this issue Apr 26, 2023 · 2 comments
Open

numbers.js cache excessive memory usage #139

David-Parker opened this issue Apr 26, 2023 · 2 comments

Comments

@David-Parker
Copy link

David-Parker commented Apr 26, 2023

The uint16 cache from numbers.js currently consumes roughly 10Mb, which is too excessive for our application.

We could disable the precache generation, but I think the implementation could be improved.

Currently the cache is an object that maps indexes from 0 to 65535 to 65536 individual Uint8Array objects (using Buffer.allocUnsafe). Each one of these Uint8Array objects is consuming 72 bytes on Chrome (when it's supposed to be representing 2 bytes of unsigned integer data, but each Uint8Array object has quite a bit of overhead).

Another potential issue is we are forcing Big Endian encoding by serializing the bytes manually:

  buffer.writeUInt8(i >> 8, 0)
  buffer.writeUInt8(i & 0x00FF, 0 + 1)

Instead of creating this map of individual arrays, could the same cache be achieved using a single UInt16Array?

I.e.:

const max = 65536
const cache = new Uint16Array(max)

for (let i = 0; i < max; i++) {
  cache[i] = i;
}

With this implementation, the entire cache only consumes roughly 131Kb a significant decrease from 10Mb.

@mcollina
Copy link
Member

mcollina commented Apr 27, 2023

Part of the reason we are allocating all those buffers is to avoid creating them at runtime when this information is sent on to the wire. The question is how much writing a slice of this new massive UInt16Array will cost.

I always welcome perf improvemements, so why don't you prepare a PR and we can benchmark? I don't think the logic of what you propose would be identical to what we have here.

(Note that the endianess is required by the MQTT protocol)

@David-Parker
Copy link
Author

Thanks Matteo, the comment on endianess makes sense.

Yes, the interface is a little bit different, but wondering if these buffers can be combined somehow into a single buffer object with different views. I have some ideas on how to improve the stream writing, will see if I get some time to hack on it and run some benchmarks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants
@mcollina @David-Parker and others