FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

buffer: optimize Buffer.prototype.copy · nodejs/node@509cd1b · GitHub

/ node Public

Commit 509cd1b

Browse files
ronag
authored and
aduh95
committed
buffer: optimize Buffer.prototype.copy
Route the native backing of Buffer.prototype.copy (CopyImpl, the `_copy` binding) through V8's new v8::ArrayBuffer::CopyArrayBufferBytes API (added in the preceding commit) instead of materializing an ArrayBufferViewContents and doing a manual memmove. This speeds up partial copies (sourceStart > 0). All copies now go through this binding: the previous %TypedArray%.prototype.set fast-path for whole-buffer copies is dropped, since it would throw on a detached or immutable target rather than report a 0-byte no-op, and the benchmarks below show the native path is comparable for that case. When both sides are backed by a SharedArrayBuffer the relaxed-atomic overload is used, which honors the SharedArrayBuffer memory model. Mixed copies (one SharedArrayBuffer, one ArrayBuffer) use the regular overload. copy() now reports the number of bytes actually copied, as returned by V8: 0 when the target is backed by a detached or immutable ArrayBuffer. The copy is then a no-op rather than a write to read-only memory. The native binding now plumbs byte offsets and the copied-byte count through as size_t, passed across the fast/slow API boundary as doubles (exact for integer values below 2^53), so copies that cross the 4 GiB boundary are no longer truncated to 32 bits. buffer-copy.js vs node v26.3.0 (x64, 30 runs): partial=false bytes=1024: +7.91% (***) partial=false bytes=128: +0.17% partial=false bytes=8: -0.89% partial=true bytes=1024: +22.33% (***) partial=true bytes=128: +19.58% (***) partial=true bytes=8: +18.70% (***) This supersedes the prototype in #62491, which added a bespoke ArrayBufferView::FastCopy instead of using the upstream-friendly CopyArrayBufferBytes API. Adds SharedArrayBuffer and immutable-ArrayBuffer coverage to the buffer copy tests, plus a pummel regression test for copies larger than 2^32 bytes. Refs: #55422 Signed-off-by: Robert Nagy <ronagy@icloud.com> Assisted-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com> PR-URL: #63828 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: René <contact.9a5d6388@renegade334.me.uk>
1 parent 0bf8e12 commit 509cd1b

5 files changed

Lines changed: 205 additions & 35 deletions

File tree

‎lib/buffer.js‎

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -267,7 +267,7 @@ function copyImpl(source, target, targetStart, sourceStart, sourceEnd) {
267267
return _copyActual(source, target, targetStart, sourceStart, sourceEnd);
268268
}
269269

270-
function _copyActual(source, target, targetStart, sourceStart, sourceEnd, isUint8Copy = false) {
270+
function _copyActual(source, target, targetStart, sourceStart, sourceEnd) {
271271
if (sourceEnd - sourceStart > target.byteLength - targetStart)
272272
sourceEnd = sourceStart + target.byteLength - targetStart;
273273

@@ -279,13 +279,10 @@ function _copyActual(source, target, targetStart, sourceStart, sourceEnd, isUint
279279
if (nb <= 0)
280280
return 0;
281281

282-
if (sourceStart === 0 && nb === sourceLen && (isUint8Copy || isUint8Array(target))) {
283-
TypedArrayPrototypeSet(target, source, targetStart);
284-
} else {
285-
_copy(source, target, targetStart, sourceStart, nb);
286-
}
287-
288-
return nb;
282+
// `_copy` returns the number of bytes actually copied, which is `nb` except
283+
// when the target is backed by a detached or immutable ArrayBuffer, in which
284+
// case nothing is copied and it returns 0.
285+
return _copy(source, target, targetStart, sourceStart, nb);
289286
}
290287

291288
/**

‎src/node_buffer.cc‎

Lines changed: 61 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -597,44 +597,78 @@ void StringSlice(const FunctionCallbackInfo<Value>& args) {
597597
}
598598
}
599599

600-
void CopyImpl(Local<Value> source_obj,
601-
Local<Value> target_obj,
602-
const uint32_t target_start,
603-
const uint32_t source_start,
604-
const uint32_t to_copy) {
605-
ArrayBufferViewContents<char> source(source_obj);
606-
SPREAD_BUFFER_ARG(target_obj, target);
607-
608-
memmove(target_data + target_start, source.data() + source_start, to_copy);
600+
// Returns the number of bytes actually copied. This is normally |to_copy|, but
601+
// V8 copies nothing (and returns 0) when the target is backed by a detached or
602+
// immutable ArrayBuffer.
603+
size_t CopyImpl(Local<Value> source_obj,
604+
Local<Value> target_obj,
605+
const size_t target_start,
606+
const size_t source_start,
607+
const size_t to_copy) {
608+
Local<ArrayBufferView> source = source_obj.As<ArrayBufferView>();
609+
Local<ArrayBufferView> target = target_obj.As<ArrayBufferView>();
610+
611+
Local<ArrayBuffer> source_ab = source->Buffer();
612+
Local<ArrayBuffer> target_ab = target->Buffer();
613+
614+
const size_t source_offset = source->ByteOffset() + source_start;
615+
const size_t target_offset = target->ByteOffset() + target_start;
616+
617+
// Defer byte-range clamping and detached/immutable handling to V8. When both
618+
// sides are backed by a SharedArrayBuffer the relaxed atomic overload is
619+
// used, which honors the SharedArrayBuffer memory model. Any other
620+
// combination (both regular, or one of each) goes through the ArrayBuffer
621+
// overload: it operates on the underlying backing store regardless of
622+
// shared-ness, so a plain memmove is performed (matching the historical
623+
// behavior for SharedArrayBuffer-backed buffers). The V8 API has no overload
624+
// that mixes ArrayBuffer and SharedArrayBuffer, so the two must never be
625+
// cross-cast.
626+
if (source_ab->IsSharedArrayBuffer() && target_ab->IsSharedArrayBuffer()) {
627+
return source_ab.As<SharedArrayBuffer>()->CopyArrayBufferBytes(
628+
source_offset,
629+
to_copy,
630+
target_ab.As<SharedArrayBuffer>(),
631+
target_offset);
632+
}
633+
return source_ab->CopyArrayBufferBytes(
634+
source_offset, to_copy, target_ab, target_offset);
609635
}
610636

611637
// Assume caller has properly validated args.
612638
void SlowCopy(const FunctionCallbackInfo<Value>& args) {
613639
Local<Value> source_obj = args[0];
614640
Local<Value> target_obj = args[1];
615-
const uint32_t target_start = args[2].As<Uint32>()->Value();
616-
const uint32_t source_start = args[3].As<Uint32>()->Value();
617-
const uint32_t to_copy = args[4].As<Uint32>()->Value();
618-
619-
CopyImpl(source_obj, target_obj, target_start, source_start, to_copy);
620-
621-
args.GetReturnValue().Set(to_copy);
641+
// Byte offsets and lengths can exceed uint32 for buffers larger than 4 GiB,
642+
// so they are passed and returned as doubles (exact for integers < 2^53).
643+
const size_t target_start =
644+
static_cast<size_t>(args[2].As<Number>()->Value());
645+
const size_t source_start =
646+
static_cast<size_t>(args[3].As<Number>()->Value());
647+
const size_t to_copy = static_cast<size_t>(args[4].As<Number>()->Value());
648+
649+
const size_t copied =
650+
CopyImpl(source_obj, target_obj, target_start, source_start, to_copy);
651+
652+
args.GetReturnValue().Set(static_cast<double>(copied));
622653
}
623654

624655
// Assume caller has properly validated args.
625-
uint32_t FastCopy(Local<Value> receiver,
626-
Local<Value> source_obj,
627-
Local<Value> target_obj,
628-
uint32_t target_start,
629-
uint32_t source_start,
630-
uint32_t to_copy,
631-
// NOLINTNEXTLINE(runtime/references)
632-
FastApiCallbackOptions& options) {
656+
double FastCopy(Local<Value> receiver,
657+
Local<Value> source_obj,
658+
Local<Value> target_obj,
659+
double target_start,
660+
double source_start,
661+
double to_copy,
662+
// NOLINTNEXTLINE(runtime/references)
663+
FastApiCallbackOptions& options) {
664+
TRACK_V8_FAST_API_CALL("buffer.copy");
633665
HandleScope scope(options.isolate);
634666

635-
CopyImpl(source_obj, target_obj, target_start, source_start, to_copy);
636-
637-
return to_copy;
667+
return static_cast<double>(CopyImpl(source_obj,
668+
target_obj,
669+
static_cast<size_t>(target_start),
670+
static_cast<size_t>(source_start),
671+
static_cast<size_t>(to_copy)));
638672
}
639673

640674
static CFunction fast_copy(CFunction::Make(FastCopy));

‎test/parallel/test-buffer-copy-immutable.js‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
// Flags: --js-immutable-arraybuffer
2+
'use strict';
3+
const common = require('../common');
4+
const assert = require('assert');
5+
6+
// transferToImmutable is gated behind --js-immutable-arraybuffer (set above).
7+
// Skip if this V8 build does not expose the API even with the flag set.
8+
if (typeof ArrayBuffer.prototype.transferToImmutable !== 'function')
9+
common.skip('ArrayBuffer.prototype.transferToImmutable is not available');
10+
11+
// Copying *into* a buffer backed by an immutable ArrayBuffer must not write to
12+
// the read-only backing store. The copy is a no-op and reports 0 bytes copied.
13+
{
14+
const ab = new ArrayBuffer(8);
15+
new Uint8Array(ab).set([1, 2, 3, 4, 5, 6, 7, 8]);
16+
const target = Buffer.from(ab.transferToImmutable());
17+
const source = Buffer.from([9, 9, 9, 9, 9, 9, 9, 9]);
18+
19+
assert.strictEqual(source.copy(target), 0);
20+
assert.deepStrictEqual([...target], [1, 2, 3, 4, 5, 6, 7, 8]);
21+
22+
// A partial / offset copy is also a no-op.
23+
assert.strictEqual(source.copy(target, 2, 0, 4), 0);
24+
assert.deepStrictEqual([...target], [1, 2, 3, 4, 5, 6, 7, 8]);
25+
}
26+
27+
// Copying *from* a buffer backed by an immutable ArrayBuffer is allowed (reads
28+
// do not require a writable backing store) and reports the bytes copied.
29+
{
30+
const ab = new ArrayBuffer(8);
31+
new Uint8Array(ab).set([10, 20, 30, 40, 50, 60, 70, 80]);
32+
const source = Buffer.from(ab.transferToImmutable());
33+
const target = Buffer.alloc(8);
34+
35+
assert.strictEqual(source.copy(target), 8);
36+
assert.deepStrictEqual([...target], [10, 20, 30, 40, 50, 60, 70, 80]);
37+
}
38+
39+
// A mutable Uint8Array view onto an immutable ArrayBuffer is still a read-only
40+
// target through Buffer.prototype.copy.
41+
{
42+
const ab = new ArrayBuffer(4);
43+
new Uint8Array(ab).set([100, 101, 102, 103]);
44+
const target = new Uint8Array(ab.transferToImmutable());
45+
46+
assert.strictEqual(Buffer.from([1, 2, 3, 4]).copy(target), 0);
47+
assert.deepStrictEqual([...target], [100, 101, 102, 103]);
48+
}

‎test/parallel/test-buffer-copy.js‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -251,3 +251,54 @@ assert.deepStrictEqual(c, b.slice(0, c.length));
251251
// No copying took place:
252252
assert.deepStrictEqual(c.toString(), 'C'.repeat(c.length));
253253
}
254+
255+
// Copying to/from SharedArrayBuffer-backed buffers. The relaxed-atomic copy
256+
// path is used only when both sides are backed by a SharedArrayBuffer; mixed
257+
// copies (one shared, one not) go through the regular non-atomic overload.
258+
{
259+
// SharedArrayBuffer -> SharedArrayBuffer.
260+
const src = Buffer.from(new SharedArrayBuffer(512)).fill(0x61);
261+
const dst = Buffer.from(new SharedArrayBuffer(512)).fill(0x62);
262+
const copied = src.copy(dst, 0, 0, 512);
263+
assert.strictEqual(copied, 512);
264+
assert.deepStrictEqual(Buffer.from(dst), Buffer.from(src));
265+
}
266+
267+
{
268+
// SharedArrayBuffer source -> regular Buffer target (mixed).
269+
const src = Buffer.from(new SharedArrayBuffer(256)).fill(0x63);
270+
const dst = Buffer.allocUnsafe(256).fill(0x64);
271+
assert.strictEqual(src.copy(dst), 256);
272+
assert.deepStrictEqual(dst, Buffer.from(src));
273+
}
274+
275+
{
276+
// Regular Buffer source -> SharedArrayBuffer target (mixed).
277+
const src = Buffer.allocUnsafe(256).fill(0x65);
278+
const dst = Buffer.from(new SharedArrayBuffer(256)).fill(0x66);
279+
assert.strictEqual(src.copy(dst), 256);
280+
assert.deepStrictEqual(Buffer.from(dst), src);
281+
}
282+
283+
{
284+
// Views with a non-zero byteOffset over a SharedArrayBuffer. The native copy
285+
// is relative to the underlying ArrayBuffer, so the view's byteOffset must be
286+
// accounted for.
287+
const sab = new SharedArrayBuffer(16);
288+
const whole = Buffer.from(sab);
289+
for (let i = 0; i < 16; i++) whole[i] = i;
290+
const src = Buffer.from(sab, 4, 8); // sab bytes 4..11
291+
const dst = Buffer.from(sab, 12, 4); // sab bytes 12..15
292+
assert.strictEqual(src.copy(dst, 0, 0, 4), 4);
293+
assert.deepStrictEqual(
294+
[...whole],
295+
[0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 4, 5, 6, 7]);
296+
}
297+
298+
{
299+
// Overlapping copy within a single SharedArrayBuffer uses memmove semantics.
300+
const buf = Buffer.from(new SharedArrayBuffer(8));
301+
for (let i = 0; i < 8; i++) buf[i] = i + 1; // [1,2,3,4,5,6,7,8]
302+
assert.strictEqual(buf.copy(buf, 2, 0, 6), 6);
303+
assert.deepStrictEqual([...buf], [1, 2, 1, 2, 3, 4, 5, 6]);
304+
}

‎test/pummel/test-buffer-large-size-copy.js‎

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
'use strict';
2+
const common = require('../common');
3+
4+
// Buffer.prototype.copy with offsets and a byte count > 2 ** 32. Regression
5+
// test for the .copy() side of https://github.com/nodejs/node/issues/55422,
6+
// where the byte count and the return value were truncated to 32 bits.
7+
common.skipIf32Bits();
8+
9+
const assert = require('node:assert');
10+
11+
// A little past the 2 ** 32 boundary, with headroom for the shift below.
12+
const size = 2 ** 32 + 16;
13+
14+
let buf;
15+
try {
16+
buf = Buffer.alloc(size);
17+
} catch (e) {
18+
if (
19+
e.code === 'ERR_MEMORY_ALLOCATION_FAILED' ||
20+
/Array buffer allocation failed/.test(e.message)
21+
) {
22+
common.skip('insufficient memory for Buffer.alloc');
23+
}
24+
25+
throw e;
26+
}
27+
28+
// Place a marker near the end of the source range, at an index > 2 ** 32.
29+
const marker = 0x42;
30+
buf[size - 9] = marker;
31+
32+
// Shift the whole buffer right by 8 bytes within itself. `to_copy` is
33+
// size - 8 (> 2 ** 32), so a truncated count would copy only 8 bytes. The
34+
// source byte at size - 9 must land at size - 1.
35+
const copied = buf.copy(buf, 8, 0, size - 8);
36+
37+
// The return value must not be truncated to 32 bits ...
38+
assert.strictEqual(copied, size - 8);
39+
// ... and the byte must actually have moved across the 2 ** 32 boundary.
40+
assert.strictEqual(buf[size - 1], marker);

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL