lib: use fastbuffer for empty buffer allocation

[gurgunday]

As in other places, we don't need to call Buffer.alloc(0), which validates the length and calls FastBuffer anyway, when we're creating an empty Buffer

This bypasses the validation logic and is consequently faster

buffers/fast-buffer.js
buffers/fast-buffer.js operation="fastbuffer" n=1000000: 47,834,970.118450865
buffers/fast-buffer.js operation="bufferalloc" n=1000000: 33,173,868.757805813

'use strict';

// flags: --expose-internals

const common = require('../common');
const { FastBuffer } = require('internal/buffer');

const bench = common.createBenchmark(main, {
  n: [1e6],
  operation: ['fastbuffer', 'bufferalloc'],
});


function main({ n, operation }) {
  switch (operation) {
    case 'fastbuffer':
      bench.start();
      for (let i = 0; i < n; i++) {
        new FastBuffer()
      }
      bench.end(n);
      break;
    case 'bufferalloc':
      bench.start();
      for (let i = 0; i < n; i++) {
        Buffer.alloc(0)
      }
      bench.end(n);
      break;
  }
}

[nodejs-github-bot]

Review requested:

• @nodejs/net
• @nodejs/streams
• @nodejs/test_runner

[codecov]

Codecov Report

❌ Patch coverage is 93.33333% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 88.47%. Comparing base (dbe45b7) to head (10bb3e6).
⚠️ Report is 126 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/streams/readable.js	0.00%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #60558      +/-   ##
==========================================
- Coverage   88.54%   88.47%   -0.07%     
==========================================
  Files         704      701       -3     
  Lines      207843   207793      -50     
  Branches    40044    40009      -35     
==========================================
- Hits       184028   183851     -177     
- Misses      15858    15999     +141     
+ Partials     7957     7943      -14     

Files with missing lines	Coverage Δ
lib/buffer.js	100.00% <100.00%> (ø)
lib/dgram.js	97.45% <100.00%> (+<0.01%)	⬆️
lib/internal/debugger/inspect_client.js	87.57% <100.00%> (-0.07%)	⬇️
lib/internal/test_runner/runner.js	92.81% <100.00%> (+<0.01%)	⬆️
lib/zlib.js	98.27% <100.00%> (+<0.01%)	⬆️
lib/internal/streams/readable.js	96.20% <0.00%> (ø)

... and 110 files with indirect coverage changes

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[ChALkeR]

This bypasses the validation logic and is consequently faster

Is that measurable? If yes, would special-casing Buffer.alloc(0) or otherwise improving Buffer.alloc be faster/cleaner instead?
As it's also used in other places, e.g. undici

[gurgunday]

Is that measurable?

Yes.

buffers/fast-buffer.js
buffers/fast-buffer.js operation="fastbuffer" n=1000000: 47,834,970.118450865
buffers/fast-buffer.js operation="bufferalloc" n=1000000: 33,173,868.757805813

I'd oppose adding this logic to alloc: it's not cleaner – you can always add more exceptions/fast paths to these kinds of functions. This PR only uses something we already use to bypass extra guards as they are not needed here. I'm not interested in doing more for this PR.

Edit: However, I think more work could be done to improve the performance of validators in Node.js like validateInteger, validateNumber, etc., if you're interested. I wouldn't discuss it here though.

[ChALkeR]

The issue with the current approach is that it doesn't optimize e.g. code in undici (or anything in userspace)
If Buffer.alloc/allocUnsafe/of could be improved for similar gains, it would extend the effect to deps and ecosystem

Also I'm not sure validateNumber is related, this doesn't add up

Will check shortly

[ChALkeR]

Ok, this actually seems to be better than attempting to improve alloc

[gurgunday]

@aduh95 sorry for the ping, could you request-ci?

[aduh95]

@aduh95 sorry for the ping, could you request-ci?

Why don't you do it yourself?

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/70111/
Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1754/
Benchmark GHA: https://github.com/aduh95/node/actions/runs/19259914235

[gurgunday]

Why don't you do it yourself?

@aduh95 I'm not a collaborator unfortunately, I can't request it :(

[gurgunday]

Should I add the benchmark? I thought it would be unnecessary in this case

[gurgunday]

@aduh95 I added the benchmark if you'd like to verify it's faster

Edit: but it's not gonna show a difference between main and this branch, it's only to see FastBuffer() is significantly faster than Buffer.alloc(0)

[gurgunday]

It won't work actually since it requires expose internals in the benchmark, but you can verify by looking at the code path I provided or running the benchmark directly

[aduh95]

Ah sorry, I didn't look closely enough, it's certainly fine not to add the benchmark. Given that 3cf7088...10bb3e6 shows no changes, and there's already a green CI and approvals for 3cf7088, I think this is ready to land as is.

[aduh95]

Landed in 2271d2d