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.53%. Comparing base (dbe45b7) to head (3cf7088).
⚠️ Report is 4 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.53%   -0.01%     
==========================================
  Files         704      704              
  Lines      207843   207847       +4     
  Branches    40044    40042       -2     
==========================================
- Hits       184028   184026       -2     
+ Misses      15858    15848      -10     
- Partials     7957     7973      +16     

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.67% <100.00%> (+0.03%)	⬆️
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 31 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