buffer: use native copy impl

PR-URL: #54087 Reviewed-By: Yagiz Nizipli <[email protected]> Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Daniel Lemire <[email protected]>
nodejs · Aug 2, 2024 · 76c8ba9 · 76c8ba9
1 parent 8e1e3a8
commit 76c8ba9
Show file tree

Hide file tree

Showing 4 changed files with 42 additions and 40 deletions.
diff --git a/benchmark/buffers/buffer-copy.js b/benchmark/buffers/buffer-copy.js
@@ -5,12 +5,6 @@ const bench = common.createBenchmark(main, {
   bytes: [0, 8, 128, 32 * 1024],
   partial: ['true', 'false'],
   n: [6e6],
-}, {
-  combinationFilter: (p) => {
-    return (p.partial === 'false' && p.bytes === 0) ||
-           (p.partial !== 'false' && p.bytes !== 0);
-  },
-  test: { partial: 'false', bytes: 0 },
 });
 
 function main({ n, bytes, partial }) {

diff --git a/lib/buffer.js b/lib/buffer.js
@@ -57,6 +57,7 @@ const {
   byteLengthUtf8,
   compare: _compare,
   compareOffset,
+  copy: _copy,
   createFromString,
   fill: bindingFill,
   isAscii: bindingIsAscii,
@@ -200,7 +201,7 @@ function toInteger(n, defaultVal) {
   return defaultVal;
 }
 
-function _copy(source, target, targetStart, sourceStart, sourceEnd) {
+function copyImpl(source, target, targetStart, sourceStart, sourceEnd) {
   if (!isUint8Array(source))
     throw new ERR_INVALID_ARG_TYPE('source', ['Buffer', 'Uint8Array'], source);
   if (!isUint8Array(target))
@@ -245,10 +246,10 @@ function _copyActual(source, target, targetStart, sourceStart, sourceEnd) {
   if (nb > sourceLen)
     nb = sourceLen;
 
-  if (sourceStart !== 0 || sourceEnd < source.length)
-    source = new Uint8Array(source.buffer, source.byteOffset + sourceStart, nb);
+  if (nb <= 0)
+    return 0;
 
-  TypedArrayPrototypeSet(target, source, targetStart);
+  _copy(source, target, targetStart, sourceStart, nb);
 
   return nb;
 }
@@ -802,7 +803,7 @@ ObjectDefineProperty(Buffer.prototype, 'offset', {
 
 Buffer.prototype.copy =
   function copy(target, targetStart, sourceStart, sourceEnd) {
-    return _copy(this, target, targetStart, sourceStart, sourceEnd);
+    return copyImpl(this, target, targetStart, sourceStart, sourceEnd);
   };
 
 // No need to verify that "buf.length <= MAX_UINT32" since it's a read-only

diff --git a/src/node_buffer.cc b/src/node_buffer.cc
@@ -576,44 +576,40 @@ void StringSlice(const FunctionCallbackInfo<Value>& args) {
   args.GetReturnValue().Set(ret);
 }
 
-// bytesCopied = copy(buffer, target[, targetStart][, sourceStart][, sourceEnd])
-void Copy(const FunctionCallbackInfo<Value> &args) {
+// Assume caller has properly validated args.
+void SlowCopy(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
 
-  THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
-  THROW_AND_RETURN_UNLESS_BUFFER(env, args[1]);
   ArrayBufferViewContents<char> source(args[0]);
-  Local<Object> target_obj = args[1].As<Object>();
-  SPREAD_BUFFER_ARG(target_obj, target);
+  SPREAD_BUFFER_ARG(args[1].As<Object>(), target);
 
-  size_t target_start = 0;
-  size_t source_start = 0;
-  size_t source_end = 0;
-
-  THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[2], 0, &target_start));
-  THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[3], 0, &source_start));
-  THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[4], source.length(),
-                                          &source_end));
+  const auto target_start = args[2]->Uint32Value(env->context()).ToChecked();
+  const auto source_start = args[3]->Uint32Value(env->context()).ToChecked();
+  const auto to_copy = args[4]->Uint32Value(env->context()).ToChecked();
 
-  // Copy 0 bytes; we're done
-  if (target_start >= target_length || source_start >= source_end)
-    return args.GetReturnValue().Set(0);
+  memmove(target_data + target_start, source.data() + source_start, to_copy);
+  args.GetReturnValue().Set(to_copy);
+}
 
-  if (source_start > source.length())
-    return THROW_ERR_OUT_OF_RANGE(
-        env, "The value of \"sourceStart\" is out of range.");
+// Assume caller has properly validated args.
+uint32_t FastCopy(Local<Value> receiver,
+                  const v8::FastApiTypedArray<uint8_t>& source,
+                  const v8::FastApiTypedArray<uint8_t>& target,
+                  uint32_t target_start,
+                  uint32_t source_start,
+                  uint32_t to_copy) {
+  uint8_t* source_data;
+  CHECK(source.getStorageIfAligned(&source_data));
 
-  if (source_end - source_start > target_length - target_start)
-    source_end = source_start + target_length - target_start;
+  uint8_t* target_data;
+  CHECK(target.getStorageIfAligned(&target_data));
 
-  uint32_t to_copy = std::min(
-      std::min(source_end - source_start, target_length - target_start),
-      source.length() - source_start);
+  memmove(target_data + target_start, source_data + source_start, to_copy);
 
-  memmove(target_data + target_start, source.data() + source_start, to_copy);
-  args.GetReturnValue().Set(to_copy);
+  return to_copy;
 }
 
+static v8::CFunction fast_copy(v8::CFunction::Make(FastCopy));
 
 void Fill(const FunctionCallbackInfo<Value>& args) {
   Environment* env = Environment::GetCurrent(args);
@@ -1447,7 +1443,7 @@ void Initialize(Local<Object> target,
                             "byteLengthUtf8",
                             SlowByteLengthUtf8,
                             &fast_byte_length_utf8);
-  SetMethod(context, target, "copy", Copy);
+  SetFastMethod(context, target, "copy", SlowCopy, &fast_copy);
   SetFastMethodNoSideEffect(context, target, "compare", Compare, &fast_compare);
   SetMethodNoSideEffect(context, target, "compareOffset", CompareOffset);
   SetMethod(context, target, "fill", Fill);
@@ -1510,7 +1506,9 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
   registry->Register(SlowByteLengthUtf8);
   registry->Register(fast_byte_length_utf8.GetTypeInfo());
   registry->Register(FastByteLengthUtf8);
-  registry->Register(Copy);
+  registry->Register(SlowCopy);
+  registry->Register(fast_copy.GetTypeInfo());
+  registry->Register(FastCopy);
   registry->Register(Compare);
   registry->Register(FastCompare);
   registry->Register(fast_compare.GetTypeInfo());

diff --git a/src/node_external_reference.h b/src/node_external_reference.h
@@ -56,6 +56,14 @@ using CFunctionWithInt64Fallback = void (*)(v8::Local<v8::Value>,
                                             v8::FastApiCallbackOptions&);
 using CFunctionWithBool = void (*)(v8::Local<v8::Value>, bool);
 
+using CFunctionBufferCopy =
+    uint32_t (*)(v8::Local<v8::Value> receiver,
+                 const v8::FastApiTypedArray<uint8_t>& source,
+                 const v8::FastApiTypedArray<uint8_t>& target,
+                 uint32_t target_start,
+                 uint32_t source_start,
+                 uint32_t to_copy);
+
 // This class manages the external references from the V8 heap
 // to the C++ addresses in Node.js.
 class ExternalReferenceRegistry {
@@ -79,6 +87,7 @@ class ExternalReferenceRegistry {
   V(CFunctionWithDoubleReturnDouble)                                           \
   V(CFunctionWithInt64Fallback)                                                \
   V(CFunctionWithBool)                                                         \
+  V(CFunctionBufferCopy)                                                       \
   V(const v8::CFunctionInfo*)                                                  \
   V(v8::FunctionCallback)                                                      \
   V(v8::AccessorNameGetterCallback)                                            \