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

Buffer documentation on pooling might be misleading #22139

Closed
ChALkeR opened this issue Aug 5, 2018 · 1 comment
Closed

Buffer documentation on pooling might be misleading #22139

ChALkeR opened this issue Aug 5, 2018 · 1 comment
Labels
buffer Issues and PRs related to the buffer subsystem. doc Issues and PRs related to the documentations.

Comments

@ChALkeR
Copy link
Member

ChALkeR commented Aug 5, 2018

Open https://nodejs.org/api/buffer.html or master/doc/api/buffer.md, and search for «pool».


Buffer.from(), Buffer.alloc(), and Buffer.allocUnsafe()


Buffer instances returned by Buffer.allocUnsafe() may be allocated off a shared internal memory pool if size is less than or equal to half Buffer.poolSize. Instances returned by Buffer.allocUnsafeSlow() never use the shared internal memory pool.

Class Method: Buffer.allocUnsafe(size)

...
Note that the Buffer module pre-allocates an internal Buffer instance of size Buffer.poolSize that is used as a pool for the fast allocation of new Buffer instances created using Buffer.allocUnsafe() and the deprecated new Buffer(size) constructor only when size is less than or equal to Buffer.poolSize >> 1 (floor of Buffer.poolSize divided by two).
Use of this pre-allocated internal memory pool is a key difference between calling Buffer.alloc(size, fill) vs. Buffer.allocUnsafe(size).fill(fill). Specifically, Buffer.alloc(size, fill) will never use the internal Buffer pool, while Buffer.allocUnsafe(size).fill(fill) will use the internal Buffer pool if size is less than or equal to half Buffer.poolSize. The difference is subtle but can be important when an application requires the additional performance that Buffer.allocUnsafe() provides.

Next match is in Buffer.allocUnsafeSlow docs (skipped here).

Class Property: Buffer.poolSize

{integer} Default: 8192
This is the size (in bytes) of pre-allocated internal Buffer instances used for pooling. This value may be modified.

buf.byteOffset

{integer} The byteOffset on the underlying ArrayBuffer object based on which this Buffer object is created.
When setting byteOffset in Buffer.from(ArrayBuffer, byteOffset, length) or sometimes when allocating a buffer smaller than Buffer.poolSize the buffer doesn't start from a zero offset on the underlying ArrayBuffer.


In fact, Buffer.from(arg) is pooled:

$ node
> Buffer.from('xx').offset
0
> Buffer.from('xx').offset
8
> Buffer.from([1,2,3]).offset
16
> Buffer.from('Hello').buffer.byteLength
8192

That is not documented in Buffer.from docs or anywhere where people would search for it — from the docs, it might look like automatic pooling is something that is used only for Buffer.allocUnsafe.

The only place that mentions that Buffer.from(arg) is pooled is «or sometimes when allocating a buffer smaller than Buffer.poolSize» clause in the buf.byteOffset documentation. Also, that one isn't even present in v8.x LTS docs.

Perhaps, Buffer.from documentation should explicitly mention that it might return pooled buffers (as Buffer.allocUnsafe does).

@ChALkeR ChALkeR added buffer Issues and PRs related to the buffer subsystem. doc Issues and PRs related to the documentations. labels Aug 5, 2018
@mritunjayz
Copy link
Contributor

mritunjayz commented Nov 9, 2018

yes, if length > (poolSize - poolOffset) it create pool (return pool).
@ChALkeR would it be solved by adding info just like in Buffer.allocUnsafe https://nodejs.org/api/buffer.html#buffer_class_method_buffer_allocunsafe_size

HarshithaKP added a commit to HarshithaKP/node that referenced this issue Apr 7, 2020
Fixes: nodejs#22139

Co-authored-by: Mritunjay Goutam <mritunjaygoutam2204@gmail.com>
BethGriggs pushed a commit that referenced this issue Apr 14, 2020
Fixes: #22139

Co-authored-by: Mritunjay Goutam <mritunjaygoutam2204@gmail.com>

PR-URL: #32703
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit to targos/node that referenced this issue Apr 25, 2020
Fixes: nodejs#22139

Co-authored-by: Mritunjay Goutam <mritunjaygoutam2204@gmail.com>

PR-URL: nodejs#32703
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
BridgeAR pushed a commit that referenced this issue Apr 28, 2020
Fixes: #22139

Co-authored-by: Mritunjay Goutam <mritunjaygoutam2204@gmail.com>

PR-URL: #32703
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
targos pushed a commit that referenced this issue Apr 28, 2020
Fixes: #22139

Co-authored-by: Mritunjay Goutam <mritunjaygoutam2204@gmail.com>

PR-URL: #32703
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
buffer Issues and PRs related to the buffer subsystem. doc Issues and PRs related to the documentations.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants