buffer: always use _copy for copy

[ronag]

This fixes a performance regression for Buffer.copy(target, 0) and brings it back inline with Buffer.write.

V8 has a massive TypedArray.prototype.set penalty on SharedArrayBuffer

Buffer.set and Buffer.copy are up to 8.4x slower when writing to a SharedArrayBuffer vs a regular ArrayBuffer, while Buffer.write (string encoding) is completely unaffected.

256 bytes, varying offset (Apple M3 Pro, Node 25.6.1):
                  ArrayBuffer    SharedArrayBuffer    Slowdown
Buffer.set           13.6 ns             56.1 ns       4.1x
Buffer.copy          17.0 ns             65.1 ns       3.8x
Buffer.write         75.8 ns             74.1 ns       1.0x (unaffected)

4096 bytes, varying offset:
                  ArrayBuffer    SharedArrayBuffer    Slowdown
Buffer.set           80.3 ns            674.2 ns       8.4x
Buffer.copy          78.4 ns            677.7 ns       8.6x
Buffer.write        190.6 ns            186.1 ns       1.0x (unaffected)

[Renegade334]

If the figures from #60399 still apply for ArrayBuffer-backed buffers, is there scope to add isArrayBuffer(TypedArrayPrototypeGetBuffer(target)) to the previous conditions, and get the best of both worlds?

[mcollina]

lgtm

[ronag]

If the figures from #60399 still apply for ArrayBuffer-backed buffers, is there scope to add isArrayBuffer(TypedArrayPrototypeGetBuffer(target)) to the previous conditions, and get the best of both worlds?

doing all of these type checks also adds overhead. Furthermore the benchmark from that PR apply to concat so it's unclear what it's actual effect on Buffer.copy is...

[nodejs-github-bot]

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

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.66%. Comparing base (35d3bc8) to head (e88373d).
⚠️ Report is 5 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #62032   +/-   ##
=======================================
  Coverage   89.65%   89.66%           
=======================================
  Files         676      676           
  Lines      206231   206227    -4     
  Branches    39505    39504    -1     
=======================================
+ Hits       184898   184905    +7     
+ Misses      13463    13454    -9     
+ Partials     7870     7868    -2     

Files with missing lines	Coverage Δ
lib/buffer.js	99.15% <100.00%> (-0.01%)	⬇️

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

[Renegade334]

Think it's worth mentioning in the commit message that this is essentially a reversion of 24bebd0, sans tests.

[github-actions]

Failed to start CI

   ⚠  Commits were pushed since the last approving review:
   ⚠  - buffer: always use _copy for copy
   ✘  Refusing to run CI on potentially unsafe PR

https://github.com/nodejs/node/actions/runs/22516891493

[nodejs-github-bot]

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

[gurgunday]

I am seeing a significant regression in non-partial copy performance:

node % ./node benchmark/buffers/buffer-copy.js
buffers/buffer-copy.js n=6000000 partial="true" bytes=8: 37,745,651.53108819
buffers/buffer-copy.js n=6000000 partial="false" bytes=8: 72,230,886.31703596
buffers/buffer-copy.js n=6000000 partial="true" bytes=128: 37,269,250.926878504
buffers/buffer-copy.js n=6000000 partial="false" bytes=128: 70,233,110.24800341
buffers/buffer-copy.js n=6000000 partial="true" bytes=1024: 26,739,253.757964805
buffers/buffer-copy.js n=6000000 partial="false" bytes=1024: 37,699,305.6259145

node % ./node-ronag benchmark/buffers/buffer-copy.js
buffers/buffer-copy.js n=6000000 partial="true" bytes=8: 36,820,630.5993248
buffers/buffer-copy.js n=6000000 partial="false" bytes=8: 37,401,995.084754474
buffers/buffer-copy.js n=6000000 partial="true" bytes=128: 37,360,956.762793645
buffers/buffer-copy.js n=6000000 partial="false" bytes=128: 35,075,651.428067446
buffers/buffer-copy.js n=6000000 partial="true" bytes=1024: 26,155,344.217952482
buffers/buffer-copy.js n=6000000 partial="false" bytes=1024: 22,480,048.984626204

Those checks are necessary to hit the V8 .set optimization, it is faster than our native copy, I think we should instead make those checks more strict if you found a case that's slower, as @Renegade334 suggested

You are right that .concat no longer hits that path, but copy still does

Cc @ChALkeR

[aduh95]

Benchmark CI: https://ci.nodejs.org/view/Node.js%20benchmark/job/benchmark-node-micro-benchmarks/1805/

[ChALkeR]

Let's hold off until we figure out what's going on

@ronag Which benchmark is that in the PR description?

If it's not in tree, could you add it (or modify buffer-copy.js) to test behavior on:

• Buffer.alloc(x, 1)
• Buffer.from(new ArrayBuffer(x)).fill(1)
• Buffer.allocUnsafeSlow(x).fill(1)
• Buffer.from(new SharedArrayBuffer(x)).fill(1)

All of those are important / could give different results

This should be tested with benchmarks code

Also the bech should have 64/128 bytes in sizes (along others lower/higher) as there is a sharp difference there in how the data is stored in some cases.

[ChALkeR]

Also i'm curious why is v8 slower on SAB there
Are we sure that our _copy impl does not lack some logic around that / works correctly on SAB?