search

nodejs / node

Node.js JavaScript runtime ✨🐢🚀✨
https://nodejs.org
Other
104.51k stars 28.19k forks source link

FixedQueue potential memory savings #52989

Closed JaoodxD closed 2 weeks ago

JaoodxD commented 2 weeks ago

During adopting FixedQueue to Piscina in Piscinajs/piscina#555 I spotted that current Node.js implementation always leave 1 empty slot in each internal FixedCircularBuffer.
When we fulfill FixedCircularBuffer with 2047 out of 2048 elements, the isFull() will return true. is this done on purpose or is it a bug?

juanarbol commented 2 weeks ago

I'm probably not the right person to answer this. But it seems to be an intended behavior; apparently Node.js is aware of the "one wasted space" but it trades with a faster check (?)

https://github.com/nodejs/node/commit/9a3ae2fe9d6ddff6a0de81acb5e0a8c068c0c79d#diff-9db996caaaf873d3d9921a19336a08a19fc48d5ad2d42de96a97931dd98f2796R52-R53

JaoodxD commented 2 weeks ago

I'm probably not the right person to answer this. But it seems to be an intended behavior; apparently Node.js is aware of the "one wasted space" but it trades with a faster check (?)

https://github.com/nodejs/node/commit/9a3ae2fe9d6ddff6a0de81acb5e0a8c068c0c79d#diff-9db996caaaf873d3d9921a19336a08a19fc48d5ad2d42de96a97931dd98f2796R52-R53

Seems like you're absolutely right.

juanarbol commented 2 weeks ago

Due to it is an intended behavior and that behavior claims to improve performance in the process.nextTick call; I'll proceed to close this issue.

Thanks for the report.