buffer: use FastBuffer when fill is set to 0

[ChALkeR]

A large number of libraries seem to use Buffer.alloc(size, 0) instead of just Buffer.alloc(size).

We don't need to follow the «create unsafe buffer and fill it» path (i.e. actually allocate and perform fill) in that situation, that is better handled by Uint8Array constructor.

Buffer.alloc(size) and Buffer.alloc(size, 0) are equivalent, so use the same code path.

Not performing the zero-fill manually and having the underlying memory allocator do it for us can improve speed and reduce the memory usage for situations where Buffer.alloc(size, 0) is used.

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• tests and/or benchmarks are included
• commit message follows commit guidelines

---

Tests/benchmarks are not included as whether or not there would be improvement depends on the underlying memory allocator behavior (and probably also operating system memory management).

This is the difference on the current Node.js version (v10.7.0) and Linux 4.17.9:

> x = Buffer.alloc(1e9); x.length
1000000000
> `${process.memoryUsage().rss / 2**20} MiB`
'31.16796875 MiB'
> x.fill(0x42, 10, 20)
<Buffer 00 00 00 00 00 00 00 00 00 00 42 42 42 42 42 42 42 42 42 42 00 00 00 00 00 ... >
> `${process.memoryUsage().rss / 2**20} MiB`
'31.9921875 MiB'
> x = Buffer.alloc(1e9, 0); x.length
1000000000
> `${process.memoryUsage().rss / 2**20} MiB`
'985.9296875 MiB'

As it can be seen, Buffer.alloc(size) does not actually consume and/or zero-fill physical memory on the time of construction. Underlying memory allocation mechanism is able to track that the memory is supposed to be zero-filled (as calloc behaves), and zero-filled pages could be returned on first read.

That also affects speed in cases when memory is not read but is just overwritten:

> x = process.hrtime(); Buffer.alloc(2e9).fill(2); process.hrtime(x)
[ 0, 916665006 ]
> x = process.hrtime(); Buffer.alloc(2e9, 0).fill(2); process.hrtime(x)
[ 1, 4762215 ]
> x = process.hrtime(); Buffer.alloc(2e9).fill(2); process.hrtime(x)
[ 0, 970604205 ]
> x = process.hrtime(); Buffer.alloc(2e9, 0).fill(2); process.hrtime(x)
[ 1, 19652513 ]
> x = process.hrtime(); Buffer.alloc(2e9).fill(2); process.hrtime(x)
[ 0, 874969612 ]
> x = process.hrtime(); Buffer.alloc(2e9, 0).fill(2); process.hrtime(x)
[ 1, 30765432 ]

This change makes Buffer.alloc(size, 0) follow the same code path as more performant Buffer.alloc(size), which should increase speed in some cases where the buffer is overwritten and reduce memory usage in some cases where part of the buffer is later thrown away without being used (i.e. where more memory than needed is allocated temporarily).

I have seen usage of Buffer.alloc(size, 0) in many packages, including, but not limited to mysql2, hdkey, secp256k1 and more.

/cc @addaleax @bnoordhuis

[nodejs-github-bot]

@ChALkeR build started: https://ci.nodejs.org/blue/organizations/jenkins/node-test-pull-request-lite-pipeline/detail/node-test-pull-request-lite-pipeline/361/pipeline

[gireeshpunathil]

@ChALkeR - can you please show me where the zero-filling happens with the proposed changes?
[ On a side note, Ben is out of office ]

[ChALkeR]

@gireeshpunathil It calls new FastBuffer(size) on line 282, which inherits from Uint8Array, which is zero-filled by default. That is the same code path that is followed when fill is not defined, see https://nodejs.org/api/buffer.html#buffer_class_method_buffer_alloc_size_fill_encoding:

If fill is undefined, the Buffer will be zero-filled.

This change makes fill === 0 case follow the same code path as fill === undefined case.

[trivikr]

CI: https://ci.nodejs.org/job/node-test-pull-request/16124/

[addaleax]

Resume CI: https://ci.nodejs.org/job/node-test-pull-request/16151/

[trivikr]

Resumed CI: https://ci.nodejs.org/job/node-test-pull-request/16172/

[trivikr]

Landed in b07852d