buffer: optimize Buffer.prototype.copy

common.gypi

@@ -40,7 +40,7 @@
 
     # Reset this number to 0 on major V8 upgrades.
     # Increment by one for each non-official patch applied to deps/v8.
-    'v8_embedder_string': '-node.20',
+    'v8_embedder_string': '-node.21',
 
     ##### V8 defaults for Node.js #####
 

deps/v8/include/v8-array-buffer.h

@@ -338,6 +338,17 @@ class V8_EXPORT ArrayBuffer : public Object {
    */
   bool IsImmutable() const;
 
+  /**
+   * Copy up to |bytes_to_copy| bytes from this ArrayBuffer starting at
+   * position |source_start| to the target ArrayBuffer starting at position
+   * |target_start|. Nothing is copied if the source ArrayBuffer is detached,
+   * or if the target ArrayBuffer is detached or immutable.
+   * Returns the number of bytes actually copied.
+   */
+  size_t CopyArrayBufferBytes(size_t source_start, size_t bytes_to_copy,
+                              Local<ArrayBuffer> target,
+                              size_t target_start) const;
+
   /**
    * Detaches this ArrayBuffer and all its views (typed arrays).
    * Detaching sets the byte length of the buffer and all typed arrays to zero,
@@ -605,6 +616,16 @@ class V8_EXPORT SharedArrayBuffer : public Object {
    */
   void* Data() const;
 
+  /**
+   * Copy up to |bytes_to_copy| bytes from this SharedArrayBuffer starting at
+   * position |source_start| to the target SharedArrayBuffer starting at
+   * position |target_start|.
+   * Returns the number of bytes actually copied.
+   */
+  size_t CopyArrayBufferBytes(size_t source_start, size_t bytes_to_copy,
+                              Local<SharedArrayBuffer> target,
+                              size_t target_start) const;
+
   V8_INLINE static SharedArrayBuffer* Cast(Value* value) {
 #ifdef V8_ENABLE_CHECKS
     CheckCast(value);

deps/v8/src/api/api.cc

@@ -4221,6 +4221,44 @@ void* v8::SharedArrayBuffer::Data() const {
   return Utils::OpenDirectHandle(this)->backing_store();
 }
 
+template <bool is_shared>
+static size_t CopyArrayBufferBytesImpl(const void* source_buffer,
+                                       size_t source_start,
+                                       size_t source_length,
+                                       void* target_buffer, size_t target_start,
+                                       size_t target_length,
+                                       size_t bytes_to_copy) {
+  source_start = std::min(source_start, source_length);
+  target_start = std::min(target_start, target_length);
+  size_t source_size = source_length - source_start;
+  size_t target_size = target_length - target_start;
+  bytes_to_copy = std::min({bytes_to_copy, source_size, target_size});
+  if (bytes_to_copy == 0) return 0;
+  const char* src = static_cast<const char*>(source_buffer) + source_start;
+  char* dst = static_cast<char*>(target_buffer) + target_start;
+  if (is_shared) {
+    base::Relaxed_Memmove(reinterpret_cast<base::Atomic8*>(dst),
+                          reinterpret_cast<const base::Atomic8*>(src),
+                          bytes_to_copy);
+  } else {
+    std::memmove(dst, src, bytes_to_copy);
+  }
+  return bytes_to_copy;
+}
+
+size_t v8::SharedArrayBuffer::CopyArrayBufferBytes(
+    size_t source_start, size_t bytes_to_copy, Local<SharedArrayBuffer> target,
+    size_t target_start) const {
+  i::DisallowGarbageCollection no_gc;
+  auto self = Utils::OpenDirectHandle(this);
+  auto that = Utils::OpenDirectHandle(*target);
+  DCHECK(!that->is_immutable());
+  return CopyArrayBufferBytesImpl<true>(self->backing_store(), source_start,
+                                        self->GetByteLength(),
+                                        that->backing_store(), target_start,
+                                        that->GetByteLength(), bytes_to_copy);
+}
+
 void v8::ArrayBuffer::CheckCast(Value* that) {
   auto obj = *Utils::OpenDirectHandle(that);
   Utils::ApiCheck(
@@ -8907,6 +8945,21 @@ bool v8::ArrayBuffer::IsImmutable() const {
   return Utils::OpenDirectHandle(this)->is_immutable();
 }
 
+size_t v8::ArrayBuffer::CopyArrayBufferBytes(size_t source_start,
+                                             size_t bytes_to_copy,
+                                             Local<ArrayBuffer> target,
+                                             size_t target_start) const {
+  i::DisallowGarbageCollection no_gc;
+  auto self = Utils::OpenDirectHandle(this);
+  auto that = Utils::OpenDirectHandle(*target);
+  if (self->was_detached()) return 0;
+  if (that->was_detached() || that->is_immutable()) return 0;
+  return CopyArrayBufferBytesImpl<false>(self->backing_store(), source_start,
+                                         self->GetByteLength(),
+                                         that->backing_store(), target_start,
+                                         that->GetByteLength(), bytes_to_copy);
+}
+
 namespace {
 std::shared_ptr<i::BackingStore> ToInternal(
     std::shared_ptr<i::BackingStoreBase> backing_store) {

deps/v8/test/cctest/test-api-array-buffer.cc

@@ -1301,3 +1301,53 @@ TEST(ArrayBuffer_ImmutableBackingStore) {
 
   CHECK(ab->IsImmutable());
 }
+
+TEST(ArrayBuffer_CopyArrayBufferBytes) {
+  LocalContext env;
+  v8::Isolate* isolate = env.isolate();
+  v8::HandleScope scope(isolate);
+  auto ab1 = v8::ArrayBuffer::New(isolate, 6);
+  auto ab2 = v8::ArrayBuffer::New(isolate, 4);
+  std::memcpy(ab1->Data(), "123456", 6);
+  std::memcpy(ab2->Data(), "ABCD", 4);
+  CHECK_EQ(0, ab1->CopyArrayBufferBytes(0, 0, ab2, 0));
+  CHECK_EQ(0, ab1->CopyArrayBufferBytes(6, 0, ab2, 6));
+  CHECK_EQ(0, ab1->CopyArrayBufferBytes(0, 4, ab2, 6));
+  CHECK_EQ(0, std::memcmp(ab2->Data(), "ABCD", 4));
+  CHECK_EQ(4, ab1->CopyArrayBufferBytes(0, 6, ab2, 0));
+  CHECK_EQ(0, std::memcmp(ab2->Data(), "1234", 4));
+  CHECK_EQ(2, ab1->CopyArrayBufferBytes(0, 6, ab2, 2));
+  CHECK_EQ(0, std::memcmp(ab2->Data(), "1212", 4));
+  ab2->Detach(v8::Local<v8::Value>()).Check();
+  CHECK_EQ(0, ab1->CopyArrayBufferBytes(0, 6, ab2, 0));
+  std::unique_ptr<v8::BackingStore> backing_store =
+      v8::ArrayBuffer::NewBackingStore(isolate, 6);
+  CHECK(backing_store);
+  v8::internal::BackingStore* i_backing_store =
+      reinterpret_cast<v8::internal::BackingStore*>(backing_store.get());
+  i_backing_store->set_is_immutable(true);
+  CHECK(i_backing_store->is_immutable());
+  std::shared_ptr<v8::BackingStore> shared_backing_store =
+      std::move(backing_store);
+  auto ab3 = v8::ArrayBuffer::New(isolate, shared_backing_store);
+  CHECK(ab3->IsImmutable());
+  CHECK_EQ(0, ab1->CopyArrayBufferBytes(0, 6, ab3, 0));
+}
+
+TEST(SharedArrayBuffer_CopyArrayBufferBytes) {
+  LocalContext env;
+  v8::Isolate* isolate = env.isolate();
+  v8::HandleScope scope(isolate);
+  auto ab1 = v8::SharedArrayBuffer::New(isolate, 6);
+  auto ab2 = v8::SharedArrayBuffer::New(isolate, 4);
+  std::memcpy(ab1->Data(), "123456", 6);
+  std::memcpy(ab2->Data(), "ABCD", 4);
+  CHECK_EQ(0, ab1->CopyArrayBufferBytes(0, 0, ab2, 0));
+  CHECK_EQ(0, ab1->CopyArrayBufferBytes(6, 0, ab2, 6));
+  CHECK_EQ(0, ab1->CopyArrayBufferBytes(0, 4, ab2, 6));
+  CHECK_EQ(0, std::memcmp(ab2->Data(), "ABCD", 4));
+  CHECK_EQ(4, ab1->CopyArrayBufferBytes(0, 6, ab2, 0));
+  CHECK_EQ(0, std::memcmp(ab2->Data(), "1234", 4));
+  CHECK_EQ(2, ab1->CopyArrayBufferBytes(0, 6, ab2, 2));
+  CHECK_EQ(0, std::memcmp(ab2->Data(), "1212", 4));
+}

lib/buffer.js

@@ -267,7 +267,7 @@ function copyImpl(source, target, targetStart, sourceStart, sourceEnd) {
   return _copyActual(source, target, targetStart, sourceStart, sourceEnd);
 }
 
-function _copyActual(source, target, targetStart, sourceStart, sourceEnd, isUint8Copy = false) {
+function _copyActual(source, target, targetStart, sourceStart, sourceEnd) {
   if (sourceEnd - sourceStart > target.byteLength - targetStart)
     sourceEnd = sourceStart + target.byteLength - targetStart;
 
@@ -279,13 +279,10 @@ function _copyActual(source, target, targetStart, sourceStart, sourceEnd, isUint
   if (nb <= 0)
     return 0;
 
-  if (sourceStart === 0 && nb === sourceLen && (isUint8Copy || isUint8Array(target))) {
-    TypedArrayPrototypeSet(target, source, targetStart);
-  } else {
-    _copy(source, target, targetStart, sourceStart, nb);
-  }
-
-  return nb;
+  // `_copy` returns the number of bytes actually copied, which is `nb` except
+  // when the target is backed by a detached or immutable ArrayBuffer, in which
+  // case nothing is copied and it returns 0.
+  return _copy(source, target, targetStart, sourceStart, nb);
 }
 
 /**

src/node_buffer.cc

@@ -597,44 +597,78 @@ void StringSlice(const FunctionCallbackInfo<Value>& args) {
   }
 }
 
-void CopyImpl(Local<Value> source_obj,
-              Local<Value> target_obj,
-              const uint32_t target_start,
-              const uint32_t source_start,
-              const uint32_t to_copy) {
-  ArrayBufferViewContents<char> source(source_obj);
-  SPREAD_BUFFER_ARG(target_obj, target);
-
-  memmove(target_data + target_start, source.data() + source_start, to_copy);
+// Returns the number of bytes actually copied. This is normally |to_copy|, but
+// V8 copies nothing (and returns 0) when the target is backed by a detached or
+// immutable ArrayBuffer.
+size_t CopyImpl(Local<Value> source_obj,
+                Local<Value> target_obj,
+                const size_t target_start,
+                const size_t source_start,
+                const size_t to_copy) {
+  Local<ArrayBufferView> source = source_obj.As<ArrayBufferView>();
+  Local<ArrayBufferView> target = target_obj.As<ArrayBufferView>();
+
+  Local<ArrayBuffer> source_ab = source->Buffer();
+  Local<ArrayBuffer> target_ab = target->Buffer();
+
+  const size_t source_offset = source->ByteOffset() + source_start;
+  const size_t target_offset = target->ByteOffset() + target_start;
+
+  // Defer byte-range clamping and detached/immutable handling to V8. When both
+  // sides are backed by a SharedArrayBuffer the relaxed atomic overload is
+  // used, which honors the SharedArrayBuffer memory model. Any other
+  // combination (both regular, or one of each) goes through the ArrayBuffer
+  // overload: it operates on the underlying backing store regardless of
+  // shared-ness, so a plain memmove is performed (matching the historical
+  // behavior for SharedArrayBuffer-backed buffers). The V8 API has no overload
+  // that mixes ArrayBuffer and SharedArrayBuffer, so the two must never be
+  // cross-cast.
+  if (source_ab->IsSharedArrayBuffer() && target_ab->IsSharedArrayBuffer()) {
+    return source_ab.As<SharedArrayBuffer>()->CopyArrayBufferBytes(
+        source_offset,
+        to_copy,
+        target_ab.As<SharedArrayBuffer>(),
+        target_offset);
+  }
+  return source_ab->CopyArrayBufferBytes(
+      source_offset, to_copy, target_ab, target_offset);
 }
 
 // Assume caller has properly validated args.
 void SlowCopy(const FunctionCallbackInfo<Value>& args) {
   Local<Value> source_obj = args[0];
   Local<Value> target_obj = args[1];
-  const uint32_t target_start = args[2].As<Uint32>()->Value();
-  const uint32_t source_start = args[3].As<Uint32>()->Value();
-  const uint32_t to_copy = args[4].As<Uint32>()->Value();
-
-  CopyImpl(source_obj, target_obj, target_start, source_start, to_copy);
-
-  args.GetReturnValue().Set(to_copy);
+  // Byte offsets and lengths can exceed uint32 for buffers larger than 4 GiB,
+  // so they are passed and returned as doubles (exact for integers < 2^53).
+  const size_t target_start =
+      static_cast<size_t>(args[2].As<Number>()->Value());
+  const size_t source_start =
+      static_cast<size_t>(args[3].As<Number>()->Value());
+  const size_t to_copy = static_cast<size_t>(args[4].As<Number>()->Value());
+
+  const size_t copied =
+      CopyImpl(source_obj, target_obj, target_start, source_start, to_copy);
+
+  args.GetReturnValue().Set(static_cast<double>(copied));
 }
 
 // Assume caller has properly validated args.
-uint32_t FastCopy(Local<Value> receiver,
-                  Local<Value> source_obj,
-                  Local<Value> target_obj,
-                  uint32_t target_start,
-                  uint32_t source_start,
-                  uint32_t to_copy,
-                  // NOLINTNEXTLINE(runtime/references)
-                  FastApiCallbackOptions& options) {
+double FastCopy(Local<Value> receiver,
+                Local<Value> source_obj,
+                Local<Value> target_obj,
+                double target_start,
+                double source_start,
+                double to_copy,
+                // NOLINTNEXTLINE(runtime/references)
+                FastApiCallbackOptions& options) {
+  TRACK_V8_FAST_API_CALL("buffer.copy");
   HandleScope scope(options.isolate);
 
-  CopyImpl(source_obj, target_obj, target_start, source_start, to_copy);
-
-  return to_copy;
+  return static_cast<double>(CopyImpl(source_obj,
+                                      target_obj,
+                                      static_cast<size_t>(target_start),
+                                      static_cast<size_t>(source_start),
+                                      static_cast<size_t>(to_copy)));
 }
 
 static CFunction fast_copy(CFunction::Make(FastCopy));

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

@@ -0,0 +1,48 @@
+// Flags: --js-immutable-arraybuffer
+'use strict';
+const common = require('../common');
+const assert = require('assert');
+
+// transferToImmutable is gated behind --js-immutable-arraybuffer (set above).
+// Skip if this V8 build does not expose the API even with the flag set.
+if (typeof ArrayBuffer.prototype.transferToImmutable !== 'function')
+  common.skip('ArrayBuffer.prototype.transferToImmutable is not available');
+
+// Copying *into* a buffer backed by an immutable ArrayBuffer must not write to
+// the read-only backing store. The copy is a no-op and reports 0 bytes copied.
+{
+  const ab = new ArrayBuffer(8);
+  new Uint8Array(ab).set([1, 2, 3, 4, 5, 6, 7, 8]);
+  const target = Buffer.from(ab.transferToImmutable());
+  const source = Buffer.from([9, 9, 9, 9, 9, 9, 9, 9]);
+
+  assert.strictEqual(source.copy(target), 0);
+  assert.deepStrictEqual([...target], [1, 2, 3, 4, 5, 6, 7, 8]);
+
+  // A partial / offset copy is also a no-op.
+  assert.strictEqual(source.copy(target, 2, 0, 4), 0);
+  assert.deepStrictEqual([...target], [1, 2, 3, 4, 5, 6, 7, 8]);
+}
+
+// Copying *from* a buffer backed by an immutable ArrayBuffer is allowed (reads
+// do not require a writable backing store) and reports the bytes copied.
+{
+  const ab = new ArrayBuffer(8);
+  new Uint8Array(ab).set([10, 20, 30, 40, 50, 60, 70, 80]);
+  const source = Buffer.from(ab.transferToImmutable());
+  const target = Buffer.alloc(8);
+
+  assert.strictEqual(source.copy(target), 8);
+  assert.deepStrictEqual([...target], [10, 20, 30, 40, 50, 60, 70, 80]);
+}
+
+// A mutable Uint8Array view onto an immutable ArrayBuffer is still a read-only
+// target through Buffer.prototype.copy.
+{
+  const ab = new ArrayBuffer(4);
+  new Uint8Array(ab).set([100, 101, 102, 103]);
+  const target = new Uint8Array(ab.transferToImmutable());
+
+  assert.strictEqual(Buffer.from([1, 2, 3, 4]).copy(target), 0);
+  assert.deepStrictEqual([...target], [100, 101, 102, 103]);
+}