buffer: optimize buffer.concat performance by mertcanaltin · Pull Request #61721 · nodejs/node

mertcanaltin · 2026-02-07T15:13:33Z

Removed the _copyActual indirection in the copy loop and called TypedArrayPrototypeSet directly.
Split auto-length and explicit-length paths so the auto-length copy loop is branch free. Replaced Buffer.allocUnsafe with allocate to skip redundant validation.

benchmark results:

➜  node git:(mert/buffer-concat-optimize-js) ✗ node-benchmark-compare ./result.csv
                                                                            confidence improvement accuracy (*)    (**)   (***)
buffers/buffer-concat-fill.js n=800000 extraSize=1                                   *     10.37 %       ±9.47% ±13.37% ±19.13%
buffers/buffer-concat-fill.js n=800000 extraSize=1024                                       2.91 %       ±6.22%  ±8.53% ±11.62%
buffers/buffer-concat-fill.js n=800000 extraSize=256                                 *      7.05 %       ±6.13%  ±8.41% ±11.49%
buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=1 pieces=16           **      1.76 %       ±1.18%  ±1.64%  ±2.26%
buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=1 pieces=4           ***      5.20 %       ±1.21%  ±1.65%  ±2.25%
buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=16 pieces=16          **      1.96 %       ±1.17%  ±1.61%  ±2.21%
buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=16 pieces=4          ***      3.06 %       ±1.50%  ±2.06%  ±2.84%
buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=256 pieces=16                 1.93 %       ±8.35% ±11.48% ±15.73%
buffers/buffer-concat.js n=800000 withTotalLength=0 pieceSize=256 pieces=4                  2.37 %       ±6.69%  ±9.34% ±13.12%
buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=1 pieces=16                  -0.59 %       ±0.99%  ±1.36%  ±1.86%
buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=1 pieces=4           ***      5.17 %       ±1.25%  ±1.71%  ±2.33%
buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=16 pieces=16                  0.98 %       ±1.45%  ±2.02%  ±2.81%
buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=16 pieces=4          ***      4.44 %       ±1.89%  ±2.62%  ±3.62%
buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=256 pieces=16          *     -1.97 %       ±1.89%  ±2.63%  ±3.67%
buffers/buffer-concat.js n=800000 withTotalLength=1 pieceSize=256 pieces=4         ***      3.55 %       ±1.08%  ±1.49%  ±2.04%

Be aware that when doing many comparisons the risk of a false-positive result increases.
In this case, there are 15 comparisons, you can thus expect the following amount of false-positive results:
  0.75 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.15 false positives, when considering a   1% risk acceptance (**, ***),
  0.01 false positives, when considering a 0.1% risk acceptance (***)
➜  node git:(mert/buffer-concat-optimize-js) ✗

codecov · 2026-02-07T16:19:16Z

Codecov Report

❌ Patch coverage is 89.74359% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.72%. Comparing base (3ab9dd8) to head (1898304).
⚠️ Report is 13 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/buffer.js	89.74%	4 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #61721      +/-   ##
==========================================
- Coverage   89.73%   89.72%   -0.01%     
==========================================
  Files         675      675              
  Lines      204502   204555      +53     
  Branches    39304    39311       +7     
==========================================
+ Hits       183502   183542      +40     
- Misses      13283    13309      +26     
+ Partials     7717     7704      -13

Files with missing lines	Coverage Δ
lib/buffer.js	`99.16% <89.74%> (-0.63%)`	⬇️

... and 46 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

See #60399 (comment)

Shouldn't _copyActual be just using TypedArrayPrototypeSet since #60399?
cc @gurgunday

lib/buffer.js

gurgunday

Thanks for the ping @ChALkeR

Yeah so other than inlining what we did in _copyActual (#60399), this PR avoids going through _copy even in the non sourceStart === 0 && nb === sourceLen (partial copy) case (I remember that we'd talked about doing this in a follow-up PR anyway)

I like that change

lib/buffer.js

gurgunday · 2026-02-07T18:56:43Z

lib/buffer.js

+    if (pos < length) {
+      TypedArrayPrototypeFill(buffer, 0, pos, length);
+    }


Can we even hit this part? I think this is unreachable because you moved type validation to the beginning and it will always fill after going through all of them?

It's still reachable if a buffer gets detached between the validation and copy loops buf.length becomes 0, pos won't advance, leaving uninitialized bytes without the zero-fill.

Not really sure how it can be detached unless a length getter is compromised. It's all synchronous code here?

In any case, it would be nice to have coverage for it

Other than coverage, the PR LGTM

How about an assert here?

(Attempting to read from a detached buffer should throw an error from the engine in any case.)

If the buffer gets detached between these points should actually cause the Uint8Array.prototype.set operation to fail.

Consider the example:

const u8_1 = new Uint8Array([1,2,3,4]); const u8_2 = new Uint8Array([5,6,7,8]); let called = false; Object.defineProperty(u8_1, 'length', { get() { // The first time this is called, return the actual length of the array // The second time this is called, we'll also transfer the ArrayBuffer of the second if (!called) { called = true; } else { u8_2.buffer.transfer(); } return 4; }, }); const buf = Buffer.concat([u8_1, u8_2]); console.log(buf);

node:buffer:631 TypedArrayPrototypeSet(buffer, buf, pos); ^ TypeError: Cannot perform %TypedArray%.prototype.set on a detached ArrayBuffer at Buffer.set (<anonymous>) at Buffer.concat (node:buffer:631:7) at Object.<anonymous> (/home/jsnell/tmp/fubar.js:20:20) at Module._compile (node:internal/modules/cjs/loader:1811:14) at Object..js (node:internal/modules/cjs/loader:1942:10) at Module.load (node:internal/modules/cjs/loader:1532:32) at Module._load (node:internal/modules/cjs/loader:1334:12) at wrapModuleLoad (node:internal/modules/cjs/loader:255:19) at Module.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:154:5) at node:internal/main/run_main_module:33:47 Node.js v26.0.0-pre

I'm all for being defensive here tho.

I think the bigger challenge here is that this does introduce a breaking security risk. Consider the following case:

const u8_1 = new Uint8Array([1,2,3,4]); const u8_2 = new Uint8Array([5,6,7,8]); let called = false; Object.defineProperty(u8_1, 'length', { get() { return 100; }, }); const buf = Buffer.concat([u8_1, u8_2]); console.log(buf);

Then comparing the output between current node.js and this PR:

// This PR jsnell@james-cloudflare-build:~/projects/node/node$ ./node ~/tmp/fubar.js <Buffer 01 02 03 04 30 70 00 00 f0 b3 61 64 30 70 00 00 00 2d 8b 1d 77 5e 00 00 00 2d 8b 1d 77 5e 00 00 e9 1e 4a 3e ed 01 00 00 21 1f 4a 3e ed 01 00 00 71 1f ... 54 more bytes> // Original jsnell@james-cloudflare-build:~/projects/node/node$ node ~/tmp/fubar.js <Buffer 01 02 03 04 05 06 07 08 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ... 54 more bytes> jsnell@james-cloudflare-build:~/projects/node/node$

TypedArrayPrototypeGetLength would be the tonic there, presumably.

Probably. Also just make sure that Uint8Array.prototype.set itself does not call the user overide getter (it shouldn't.. but let's confirm)

Thanks, switched to TypedArrayPrototypeGetByteLength to avoid the spoofed length getter. Zero-fill kept as defensive measure.

lib/buffer.js

mertcanaltin · 2026-02-07T19:52:28Z

I updated bench result: #61721 (comment)

lib/buffer.js

buffer: optimize buffer.concat performance

7ded458

nodejs-github-bot added buffer Issues and PRs related to the buffer subsystem. needs-ci PRs that need a full CI run. labels Feb 7, 2026

ChALkeR reviewed Feb 7, 2026

View reviewed changes

lib/buffer.js Show resolved Hide resolved

mertcanaltin added 2 commits February 7, 2026 19:41

revert comment lines

a7fd75d

revert comment lines

885c7a3

benjamingr reviewed Feb 7, 2026

View reviewed changes

lib/buffer.js Outdated Show resolved Hide resolved

revert listLen caching

ba806ae

gurgunday added the needs-benchmark-ci PR that need a benchmark CI run. label Feb 7, 2026

gurgunday reviewed Feb 7, 2026

View reviewed changes

lib/buffer.js Outdated Show resolved Hide resolved

move type validation before allocation

559f5e9

Renegade334 reviewed Feb 7, 2026

View reviewed changes

lib/buffer.js Outdated Show resolved Hide resolved

use TypedArrayPrototypeGetByteLength

1898304

Uh oh!

Conversation

mertcanaltin commented Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ChALkeR left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gurgunday left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

gurgunday Feb 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mertcanaltin Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

gurgunday Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

Renegade334 Feb 7, 2026

Choose a reason for hiding this comment

Uh oh!

jasnell Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

jasnell Feb 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Renegade334 Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

jasnell Feb 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mertcanaltin Feb 8, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mertcanaltin commented Feb 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

mertcanaltin commented Feb 7, 2026 •

edited

Loading

codecov bot commented Feb 7, 2026 •

edited

Loading

gurgunday left a comment •

edited

Loading

gurgunday Feb 7, 2026 •

edited

Loading

jasnell Feb 8, 2026 •

edited

Loading

jasnell Feb 8, 2026 •

edited

Loading