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

Some questions about buffer doc #9786

Closed
vsemozhetbyt opened this issue Nov 24, 2016 · 5 comments
Closed

Some questions about buffer doc #9786

vsemozhetbyt opened this issue Nov 24, 2016 · 5 comments
Labels
buffer Issues and PRs related to the buffer subsystem. doc Issues and PRs related to the documentations. question Issues that look for answers.

Comments

@vsemozhetbyt
Copy link
Contributor

vsemozhetbyt commented Nov 24, 2016

  • Version: 7.2.0
  • Platform: Windows 7 x64
  • Subsystem: doc, buffer

I'm preparing a PR with some small fixes in code examples of the buffer doc and I've stumbled on some puzzles I want to clarify before the PR.

  1. buf.lastIndexOf() byteOffset argument explanation.

What does the "(not inclusive)" remark actually mean? In this code (not in the doc) the byteOffset seems to be inclusive in the search and the result.

console.log(Buffer.from('0123').lastIndexOf('1', 1));
1

The same for this code in the doc:

Buffer.from('this buffer is a buffer');

// Prints: 5
console.log(buf.lastIndexOf('buffer', 5));

So the question is this one: should this remark be clarified more or should it be removed?

  1. new Buffer(size) and Buffer.allocUnsafe(size) examples.

It happens that the actual outputs of these fragments do not match with described ones for me for most of the tests:

const buf = new Buffer(5);

// Prints: (contents may vary): <Buffer 78 e0 82 02 01>
console.log(buf);

buf.fill(0);

// Prints: <Buffer 00 00 00 00 00>
console.log(buf);
const buf = Buffer.allocUnsafe(5);

// Prints: (contents may vary): <Buffer 78 e0 82 02 01>
console.log(buf);

buf.fill(0);

// Prints: <Buffer 00 00 00 00 00>
console.log(buf);

While they do output as described in the REPL, tested by script files they output the same lines in some circumstances:

<Buffer 00 00 00 00 00>
<Buffer 00 00 00 00 00>

After some research, I've found out there is a rather strong correlation between script file size and zeroing of a buffer beginning. If the before mentioned fragments are saved in 105-176 bytes file size (adjusted by these changes: ASCII/UTF-8, +/- BOM, +/- comments/space), the first 5 bytes are almost always empty. So I've written a weird script to find out more about this correlation. Here are some notes about results:

a. Zeroing scheme of buffer bytes is the same inside cycles of a script file size increasing by 8 bytes.
b. Correlation is flaky, so the statistics data need to be averaged in the research script and manually filtered after the tests.
c. Buffer.alloc(size) always works safely as intended (the stat log is empty, i.e. no non-zero bytes schemes are detected and saved).
d. Buffer.allocUnsafeSlow(size) also has some correlation, but it is significantly flakier and does not make big zero fragments in the buffer beginning, so it could be ignored in this case.

If somebody is interested in the research code, here is the script. In the comments there, I also give the stat data from a test run on my machine. The stat data format is "script file size: scheme with non-zero bytes indexes in created buffers". Repeated lines inside 8-bytes cycles are filtered. If some flaky case happens inside this cycles, it is marked by * at the beginning of the line. Here is also the diff between new Buffer(size) and Buffer.allocUnsafe(size) stats.

So the question is this one: is this correlation depends on some hardware/software features or is it something stable? In the first case, it could be ignored, in the second case the code examples should be adjusted to fall out of size diapasons that produce big zero fragments at the beginning or buffers, otherwise, novices could be confused by these examples if they run them by themselves.

Sorry for my bad English and all the possible obscurity.

@Fishrock123 Fishrock123 added doc Issues and PRs related to the documentations. buffer Issues and PRs related to the buffer subsystem. question Issues that look for answers. labels Nov 24, 2016
@Fishrock123
Copy link
Contributor

Buffer.allocUnsafeSlow(size)

Note that this does not use memory from the existing memory pool, and instead always allocates a new block of memory.

@vsemozhetbyt
Copy link
Contributor Author

@Fishrock123 Yeah, I've added Buffer.allocUnsafeSlow(size) and Buffer.alloc(size) tests just for curiosity, it could be ignored, especially as the Buffer.allocUnsafeSlow(size) code example does not make any predictions about its output. That is mostly the new Buffer(size) example that puzzles as it is, and the Buffer.allocUnsafe(size) one, if slightly changed.

@addaleax
Copy link
Member

What does the "(not inclusive)" remark actually mean? In this code (not in the doc) the byteOffset seems to be inclusive in the search and the result.

I think that’s a bug in the docs.

@vsemozhetbyt
Copy link
Contributor Author

I've made two GIFs for the second question.

In the first GIF you can see a test attempt with the old code example. See how both output lines are the same in contravention of the doc prediction.

buffer1

In the second GIF you can see a test attempt with the new code example. See how both output lines are different as the doc predicts.

buffer2

@vsemozhetbyt
Copy link
Contributor Author

vsemozhetbyt commented Nov 26, 2016

I've run the initial code in some last Node.js versions (in addition to 7.2.0 above) and I've got the same results several times (except for 5.12.0, it varies):

node.0.10.48.exe test.js
<Buffer 00 00 00 00 00>
<Buffer 00 00 00 00 00>

node.0.12.17.exe test.js
<Buffer 01 00 00 00 ff>
<Buffer 00 00 00 00 00>

node.4.6.2.exe test.js
<Buffer b2 88 fb 71 00>
<Buffer 00 00 00 00 00>

node.5.12.0.exe test.js
<Buffer d8 e5 2c 00 00>
<Buffer 00 00 00 00 00>

node.6.9.1.exe test.js
<Buffer 00 00 00 00 00>
<Buffer 00 00 00 00 00>

node.8.0.0-nightly20161121.exe test.js
<Buffer 00 00 00 00 00>
<Buffer 00 00 00 00 00>

node-v7.0.0-nightly20161121.chakracore\node.exe test.js
<Buffer 00 00 00 00 00>
<Buffer 00 00 00 00 00>

addaleax pushed a commit that referenced this issue Dec 8, 2016
On some systems, some first bytes of allocated buffer can be zeroed
by default, so the example doesn't work well - the buffer looks zeroed
even before the fill.

Fixes: #9786
PR-URL: #9795
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
jmdarling pushed a commit to jmdarling/node that referenced this issue Dec 8, 2016
On some systems, some first bytes of allocated buffer can be zeroed
by default, so the example doesn't work well - the buffer looks zeroed
even before the fill.

Fixes: nodejs#9786
PR-URL: nodejs#9795
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this issue Mar 7, 2017
On some systems, some first bytes of allocated buffer can be zeroed
by default, so the example doesn't work well - the buffer looks zeroed
even before the fill.

Fixes: #9786
PR-URL: #9795
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
MylesBorins pushed a commit that referenced this issue Mar 9, 2017
On some systems, some first bytes of allocated buffer can be zeroed
by default, so the example doesn't work well - the buffer looks zeroed
even before the fill.

Fixes: #9786
PR-URL: #9795
Reviewed-By: Sam Roberts <[email protected]>
Reviewed-By: Roman Reiss <[email protected]>
Reviewed-By: Anna Henningsen <[email protected]>
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. question Issues that look for answers.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants