From 041a0c0d1857186fc25668da9a472815ae19f95d Mon Sep 17 00:00:00 2001 From: Robert Nagy Date: Sun, 28 Jul 2024 23:14:29 +0200 Subject: [PATCH] buffer: use native copy impl PR-URL: https://github.com/nodejs/node/pull/54087 --- benchmark/buffers/buffer-copy.js | 6 ---- lib/buffer.js | 11 ++++--- src/node_buffer.cc | 56 +++++++++++++++----------------- src/node_external_reference.h | 9 +++++ 4 files changed, 42 insertions(+), 40 deletions(-) diff --git a/benchmark/buffers/buffer-copy.js b/benchmark/buffers/buffer-copy.js index a498c08e65276f..a3bf6693b146b5 100644 --- 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 index 823a9249eccc52..6713ddab4b7600 100644 --- a/lib/buffer.js +++ b/lib/buffer.js @@ -56,6 +56,7 @@ const { byteLengthUtf8, compare: _compare, compareOffset, + copy: _copy, createFromString, fill: bindingFill, isAscii: bindingIsAscii, @@ -199,7 +200,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)) @@ -244,10 +245,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; } @@ -801,7 +802,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 index 02a6a79492cf12..6e141b974131cc 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -576,44 +576,40 @@ void StringSlice(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(ret); } -// bytesCopied = copy(buffer, target[, targetStart][, sourceStart][, sourceEnd]) -void Copy(const FunctionCallbackInfo &args) { +// Assume caller has properly validated args. +void SlowCopy(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); - THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]); - THROW_AND_RETURN_UNLESS_BUFFER(env, args[1]); ArrayBufferViewContents source(args[0]); - Local target_obj = args[1].As(); - SPREAD_BUFFER_ARG(target_obj, target); + SPREAD_BUFFER_ARG(args[1].As(), 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 receiver, + const v8::FastApiTypedArray& source, + const v8::FastApiTypedArray& 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& args) { Environment* env = Environment::GetCurrent(args); @@ -1447,7 +1443,7 @@ void Initialize(Local 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 index b80b8727c23fd1..b59a3a9e9c957a 100644 --- a/src/node_external_reference.h +++ b/src/node_external_reference.h @@ -56,6 +56,14 @@ using CFunctionWithInt64Fallback = void (*)(v8::Local, v8::FastApiCallbackOptions&); using CFunctionWithBool = void (*)(v8::Local, bool); +using CFunctionBufferCopy = + uint32_t (*)(v8::Local receiver, + const v8::FastApiTypedArray& source, + const v8::FastApiTypedArray& 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) \