Skip to content

Commit 9f8f26e

Browse files
ronagtargos
authored andcommitted
buffer: use native copy impl
PR-URL: #54087 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: Daniel Lemire <daniel@lemire.me>
1 parent a4aecd2 commit 9f8f26e

File tree

4 files changed

+42
-40
lines changed

4 files changed

+42
-40
lines changed

benchmark/buffers/buffer-copy.js

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,6 @@ const bench = common.createBenchmark(main, {
55
bytes: [0, 8, 128, 32 * 1024],
66
partial: ['true', 'false'],
77
n: [6e6],
8-
}, {
9-
combinationFilter: (p) => {
10-
return (p.partial === 'false' && p.bytes === 0) ||
11-
(p.partial !== 'false' && p.bytes !== 0);
12-
},
13-
test: { partial: 'false', bytes: 0 },
148
});
159

1610
function main({ n, bytes, partial }) {

lib/buffer.js

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ const {
5757
byteLengthUtf8,
5858
compare: _compare,
5959
compareOffset,
60+
copy: _copy,
6061
createFromString,
6162
fill: bindingFill,
6263
isAscii: bindingIsAscii,
@@ -200,7 +201,7 @@ function toInteger(n, defaultVal) {
200201
return defaultVal;
201202
}
202203

203-
function _copy(source, target, targetStart, sourceStart, sourceEnd) {
204+
function copyImpl(source, target, targetStart, sourceStart, sourceEnd) {
204205
if (!isUint8Array(source))
205206
throw new ERR_INVALID_ARG_TYPE('source', ['Buffer', 'Uint8Array'], source);
206207
if (!isUint8Array(target))
@@ -245,10 +246,10 @@ function _copyActual(source, target, targetStart, sourceStart, sourceEnd) {
245246
if (nb > sourceLen)
246247
nb = sourceLen;
247248

248-
if (sourceStart !== 0 || sourceEnd < source.length)
249-
source = new Uint8Array(source.buffer, source.byteOffset + sourceStart, nb);
249+
if (nb <= 0)
250+
return 0;
250251

251-
TypedArrayPrototypeSet(target, source, targetStart);
252+
_copy(source, target, targetStart, sourceStart, nb);
252253

253254
return nb;
254255
}
@@ -802,7 +803,7 @@ ObjectDefineProperty(Buffer.prototype, 'offset', {
802803

803804
Buffer.prototype.copy =
804805
function copy(target, targetStart, sourceStart, sourceEnd) {
805-
return _copy(this, target, targetStart, sourceStart, sourceEnd);
806+
return copyImpl(this, target, targetStart, sourceStart, sourceEnd);
806807
};
807808

808809
// No need to verify that "buf.length <= MAX_UINT32" since it's a read-only

src/node_buffer.cc

Lines changed: 27 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -576,44 +576,40 @@ void StringSlice(const FunctionCallbackInfo<Value>& args) {
576576
args.GetReturnValue().Set(ret);
577577
}
578578

579-
// bytesCopied = copy(buffer, target[, targetStart][, sourceStart][, sourceEnd])
580-
void Copy(const FunctionCallbackInfo<Value> &args) {
579+
// Assume caller has properly validated args.
580+
void SlowCopy(const FunctionCallbackInfo<Value>& args) {
581581
Environment* env = Environment::GetCurrent(args);
582582

583-
THROW_AND_RETURN_UNLESS_BUFFER(env, args[0]);
584-
THROW_AND_RETURN_UNLESS_BUFFER(env, args[1]);
585583
ArrayBufferViewContents<char> source(args[0]);
586-
Local<Object> target_obj = args[1].As<Object>();
587-
SPREAD_BUFFER_ARG(target_obj, target);
584+
SPREAD_BUFFER_ARG(args[1].As<Object>(), target);
588585

589-
size_t target_start = 0;
590-
size_t source_start = 0;
591-
size_t source_end = 0;
592-
593-
THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[2], 0, &target_start));
594-
THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[3], 0, &source_start));
595-
THROW_AND_RETURN_IF_OOB(ParseArrayIndex(env, args[4], source.length(),
596-
&source_end));
586+
const auto target_start = args[2]->Uint32Value(env->context()).ToChecked();
587+
const auto source_start = args[3]->Uint32Value(env->context()).ToChecked();
588+
const auto to_copy = args[4]->Uint32Value(env->context()).ToChecked();
597589

598-
// Copy 0 bytes; we're done
599-
if (target_start >= target_length || source_start >= source_end)
600-
return args.GetReturnValue().Set(0);
590+
memmove(target_data + target_start, source.data() + source_start, to_copy);
591+
args.GetReturnValue().Set(to_copy);
592+
}
601593

602-
if (source_start > source.length())
603-
return THROW_ERR_OUT_OF_RANGE(
604-
env, "The value of \"sourceStart\" is out of range.");
594+
// Assume caller has properly validated args.
595+
uint32_t FastCopy(Local<Value> receiver,
596+
const v8::FastApiTypedArray<uint8_t>& source,
597+
const v8::FastApiTypedArray<uint8_t>& target,
598+
uint32_t target_start,
599+
uint32_t source_start,
600+
uint32_t to_copy) {
601+
uint8_t* source_data;
602+
CHECK(source.getStorageIfAligned(&source_data));
605603

606-
if (source_end - source_start > target_length - target_start)
607-
source_end = source_start + target_length - target_start;
604+
uint8_t* target_data;
605+
CHECK(target.getStorageIfAligned(&target_data));
608606

609-
uint32_t to_copy = std::min(
610-
std::min(source_end - source_start, target_length - target_start),
611-
source.length() - source_start);
607+
memmove(target_data + target_start, source_data + source_start, to_copy);
612608

613-
memmove(target_data + target_start, source.data() + source_start, to_copy);
614-
args.GetReturnValue().Set(to_copy);
609+
return to_copy;
615610
}
616611

612+
static v8::CFunction fast_copy(v8::CFunction::Make(FastCopy));
617613

618614
void Fill(const FunctionCallbackInfo<Value>& args) {
619615
Environment* env = Environment::GetCurrent(args);
@@ -1447,7 +1443,7 @@ void Initialize(Local<Object> target,
14471443
"byteLengthUtf8",
14481444
SlowByteLengthUtf8,
14491445
&fast_byte_length_utf8);
1450-
SetMethod(context, target, "copy", Copy);
1446+
SetFastMethod(context, target, "copy", SlowCopy, &fast_copy);
14511447
SetFastMethodNoSideEffect(context, target, "compare", Compare, &fast_compare);
14521448
SetMethodNoSideEffect(context, target, "compareOffset", CompareOffset);
14531449
SetMethod(context, target, "fill", Fill);
@@ -1510,7 +1506,9 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) {
15101506
registry->Register(SlowByteLengthUtf8);
15111507
registry->Register(fast_byte_length_utf8.GetTypeInfo());
15121508
registry->Register(FastByteLengthUtf8);
1513-
registry->Register(Copy);
1509+
registry->Register(SlowCopy);
1510+
registry->Register(fast_copy.GetTypeInfo());
1511+
registry->Register(FastCopy);
15141512
registry->Register(Compare);
15151513
registry->Register(FastCompare);
15161514
registry->Register(fast_compare.GetTypeInfo());

src/node_external_reference.h

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,14 @@ using CFunctionWithInt64Fallback = void (*)(v8::Local<v8::Value>,
5656
v8::FastApiCallbackOptions&);
5757
using CFunctionWithBool = void (*)(v8::Local<v8::Value>, bool);
5858

59+
using CFunctionBufferCopy =
60+
uint32_t (*)(v8::Local<v8::Value> receiver,
61+
const v8::FastApiTypedArray<uint8_t>& source,
62+
const v8::FastApiTypedArray<uint8_t>& target,
63+
uint32_t target_start,
64+
uint32_t source_start,
65+
uint32_t to_copy);
66+
5967
// This class manages the external references from the V8 heap
6068
// to the C++ addresses in Node.js.
6169
class ExternalReferenceRegistry {
@@ -79,6 +87,7 @@ class ExternalReferenceRegistry {
7987
V(CFunctionWithDoubleReturnDouble) \
8088
V(CFunctionWithInt64Fallback) \
8189
V(CFunctionWithBool) \
90+
V(CFunctionBufferCopy) \
8291
V(const v8::CFunctionInfo*) \
8392
V(v8::FunctionCallback) \
8493
V(v8::AccessorNameGetterCallback) \

0 commit comments

Comments
 (0)