From 29ef3b06f75545177cc7f7040fb256970489339e Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sat, 11 Feb 2023 23:08:41 +0100 Subject: [PATCH 1/5] benchmark: stablize encode benchmark - Increase the number of iteration to 1e6 to reduce flakes. 1e4 can introduce flakes even when comparing the main branch against itself - Replace the 1024 * 32 length test with 1024 * 8 since it would otherwise take too long to complete. Remove the 16 length test since it's not too different from 32. - Check the results of the encoding methods at the end. --- benchmark/util/text-encoder.js | 39 +++++++++++++++++++--------------- 1 file changed, 22 insertions(+), 17 deletions(-) diff --git a/benchmark/util/text-encoder.js b/benchmark/util/text-encoder.js index 1429710e9cbef5..3150a6fbc884b0 100644 --- a/benchmark/util/text-encoder.js +++ b/benchmark/util/text-encoder.js @@ -1,10 +1,11 @@ 'use strict'; const common = require('../common.js'); +const assert = require('assert'); const bench = common.createBenchmark(main, { - len: [16, 32, 256, 1024, 1024 * 32], - n: [1e4], + len: [32, 256, 1024, 1024 * 8], + n: [1e6], type: ['one-byte-string', 'two-byte-string', 'ascii'], op: ['encode', 'encodeInto'], }); @@ -26,20 +27,24 @@ function main({ n, op, len, type }) { } const input = base.repeat(len); - const subarray = new Uint8Array(len); - - bench.start(); - switch (op) { - case 'encode': { - for (let i = 0; i < n; i++) - encoder.encode(input); - break; - } - case 'encodeInto': { - for (let i = 0; i < n; i++) - encoder.encodeInto(input, subarray); - break; - } + if (op === 'encode') { + const expected = encoder.encode(input); + let result; + bench.start(); + for (let i = 0; i < n; i++) + result = encoder.encode(input); + bench.end(n); + assert.deepStrictEqual(result, expected); + } else { + const expected = new Uint8Array(len); + const subarray = new Uint8Array(len); + const expectedStats = encoder.encodeInto(input, expected); + let result; + bench.start(); + for (let i = 0; i < n; i++) + result = encoder.encodeInto(input, subarray); + bench.end(n); + assert.deepStrictEqual(subarray, expected); + assert.deepStrictEqual(result, expectedStats); } - bench.end(n); } From 57bb2177405cbb7f5216d31351b59eaf39f567f9 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sat, 11 Feb 2023 18:54:35 +0100 Subject: [PATCH 2/5] src: move encoding bindings to a new binding Move the bindings used by TextEncoder to a new binding for more self-contained code. --- lib/internal/encoding.js | 2 +- node.gyp | 2 + src/base_object_types.h | 1 + src/encoding_binding.cc | 203 ++++++++++++++++++++++++ src/encoding_binding.h | 46 ++++++ src/node_binding.cc | 1 + src/node_buffer.cc | 128 --------------- src/node_external_reference.h | 1 + src/node_snapshotable.cc | 1 + test/parallel/test-bootstrap-modules.js | 1 + 10 files changed, 257 insertions(+), 129 deletions(-) create mode 100644 src/encoding_binding.cc create mode 100644 src/encoding_binding.h diff --git a/lib/internal/encoding.js b/lib/internal/encoding.js index 5feec0552870be..2ee569c737d18c 100644 --- a/lib/internal/encoding.js +++ b/lib/internal/encoding.js @@ -54,7 +54,7 @@ const { encodeInto, encodeUtf8String, decodeUTF8, -} = internalBinding('buffer'); +} = internalBinding('encoding_binding'); const { Buffer } = require('buffer'); diff --git a/node.gyp b/node.gyp index cb46307ef62012..36490af56b77f5 100644 --- a/node.gyp +++ b/node.gyp @@ -479,6 +479,7 @@ 'src/connection_wrap.cc', 'src/dataqueue/queue.cc', 'src/debug_utils.cc', + 'src/encoding_binding.cc', 'src/env.cc', 'src/fs_event_wrap.cc', 'src/handle_wrap.cc', @@ -589,6 +590,7 @@ 'src/dataqueue/queue.h', 'src/debug_utils.h', 'src/debug_utils-inl.h', + 'src/encoding_binding.h', 'src/env_properties.h', 'src/env.h', 'src/env-inl.h', diff --git a/src/base_object_types.h b/src/base_object_types.h index f4c70a89177975..482c928882f99b 100644 --- a/src/base_object_types.h +++ b/src/base_object_types.h @@ -10,6 +10,7 @@ namespace node { // what the class passes to SET_BINDING_ID(), the second argument should match // the C++ class name. #define SERIALIZABLE_BINDING_TYPES(V) \ + V(encoding_binding_data, encoding_binding::BindingData) \ V(fs_binding_data, fs::BindingData) \ V(v8_binding_data, v8_utils::BindingData) \ V(blob_binding_data, BlobBindingData) \ diff --git a/src/encoding_binding.cc b/src/encoding_binding.cc new file mode 100644 index 00000000000000..e9bc048a510a3c --- /dev/null +++ b/src/encoding_binding.cc @@ -0,0 +1,203 @@ +#include "encoding_binding.h" +#include "env-inl.h" +#include "node_errors.h" +#include "node_external_reference.h" +#include "simdutf.h" +#include "string_bytes.h" +#include "v8.h" + +#include + +namespace node { +namespace encoding_binding { + +using v8::ArrayBuffer; +using v8::BackingStore; +using v8::Context; +using v8::Function; +using v8::FunctionCallbackInfo; +using v8::Isolate; +using v8::Local; +using v8::MaybeLocal; +using v8::Object; +using v8::String; +using v8::Uint8Array; +using v8::Uint32Array; +using v8::Value; + +BindingData::BindingData(Environment* env, Local object) + : SnapshotableObject(env, object, type_int) {} + +bool BindingData::PrepareForSerialization(Local context, + v8::SnapshotCreator* creator) { + // Return true because we need to maintain the reference to the binding from + // JS land. + return true; +} + +InternalFieldInfoBase* BindingData::Serialize(int index) { + DCHECK_EQ(index, BaseObject::kEmbedderType); + InternalFieldInfo* info = + InternalFieldInfoBase::New(type()); + return info; +} + +void BindingData::Deserialize(Local context, + Local holder, + int index, + InternalFieldInfoBase* info) { + DCHECK_EQ(index, BaseObject::kEmbedderType); + v8::HandleScope scope(context->GetIsolate()); + Environment* env = Environment::GetCurrent(context); + // Recreate the buffer in the constructor. + BindingData* binding = env->AddBindingData(context, holder); + CHECK_NOT_NULL(binding); +} + +void BindingData::EncodeInto(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + Isolate* isolate = env->isolate(); + CHECK_GE(args.Length(), 3); + CHECK(args[0]->IsString()); + CHECK(args[1]->IsUint8Array()); + CHECK(args[2]->IsUint32Array()); + + Local source = args[0].As(); + + Local dest = args[1].As(); + Local buf = dest->Buffer(); + char* write_result = static_cast(buf->Data()) + dest->ByteOffset(); + size_t dest_length = dest->ByteLength(); + + // results = [ read, written ] + Local result_arr = args[2].As(); + uint32_t* results = reinterpret_cast( + static_cast(result_arr->Buffer()->Data()) + + result_arr->ByteOffset()); + + int nchars; + int written = source->WriteUtf8( + isolate, + write_result, + dest_length, + &nchars, + String::NO_NULL_TERMINATION | String::REPLACE_INVALID_UTF8); + results[0] = nchars; + results[1] = written; +} + +// Encode a single string to a UTF-8 Uint8Array (not Buffer). +// Used in TextEncoder.prototype.encode. +void BindingData::EncodeUtf8String(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); + Isolate* isolate = env->isolate(); + CHECK_GE(args.Length(), 1); + CHECK(args[0]->IsString()); + + Local str = args[0].As(); + size_t length = str->Utf8Length(isolate); + + Local ab; + { + NoArrayBufferZeroFillScope no_zero_fill_scope(env->isolate_data()); + std::unique_ptr bs = + ArrayBuffer::NewBackingStore(isolate, length); + + CHECK(bs); + + str->WriteUtf8(isolate, + static_cast(bs->Data()), + -1, // We are certain that `data` is sufficiently large + nullptr, + String::NO_NULL_TERMINATION | String::REPLACE_INVALID_UTF8); + + ab = ArrayBuffer::New(isolate, std::move(bs)); + } + + auto array = Uint8Array::New(ab, 0, length); + args.GetReturnValue().Set(array); +} + +// Convert the input into an encoded string +void BindingData::DecodeUTF8(const FunctionCallbackInfo& args) { + Environment* env = Environment::GetCurrent(args); // list, flags + + CHECK_GE(args.Length(), 1); + + if (!(args[0]->IsArrayBuffer() || args[0]->IsSharedArrayBuffer() || + args[0]->IsArrayBufferView())) { + return node::THROW_ERR_INVALID_ARG_TYPE( + env->isolate(), + "The \"list\" argument must be an instance of SharedArrayBuffer, " + "ArrayBuffer or ArrayBufferView."); + } + + ArrayBufferViewContents buffer(args[0]); + + bool ignore_bom = args[1]->IsTrue(); + bool has_fatal = args[2]->IsTrue(); + + const char* data = buffer.data(); + size_t length = buffer.length(); + + if (has_fatal) { + auto result = simdutf::validate_utf8_with_errors(data, length); + + if (result.error) { + return node::THROW_ERR_ENCODING_INVALID_ENCODED_DATA( + env->isolate(), "The encoded data was not valid for encoding utf-8"); + } + } + + if (!ignore_bom && length >= 3) { + if (memcmp(data, "\xEF\xBB\xBF", 3) == 0) { + data += 3; + length -= 3; + } + } + + if (length == 0) return args.GetReturnValue().SetEmptyString(); + + Local error; + MaybeLocal maybe_ret = + StringBytes::Encode(env->isolate(), data, length, UTF8, &error); + Local ret; + + if (!maybe_ret.ToLocal(&ret)) { + CHECK(!error.IsEmpty()); + env->isolate()->ThrowException(error); + return; + } + + args.GetReturnValue().Set(ret); +} + +void BindingData::Initialize(Local target, + Local unused, + Local context, + void* priv) { + Environment* env = Environment::GetCurrent(context); + BindingData* const binding_data = + env->AddBindingData(context, target); + if (binding_data == nullptr) return; + + SetMethod(context, target, "encodeInto", EncodeInto); + SetMethodNoSideEffect(context, target, "encodeUtf8String", EncodeUtf8String); + SetMethodNoSideEffect(context, target, "decodeUTF8", DecodeUTF8); +} + +void BindingData::RegisterTimerExternalReferences( + ExternalReferenceRegistry* registry) { + registry->Register(EncodeInto); + registry->Register(EncodeUtf8String); + registry->Register(DecodeUTF8); +} + +} // namespace encoding_binding +} // namespace node + +NODE_BINDING_CONTEXT_AWARE_INTERNAL( + encoding_binding, node::encoding_binding::BindingData::Initialize) +NODE_BINDING_EXTERNAL_REFERENCE( + encoding_binding, + node::encoding_binding::BindingData::RegisterTimerExternalReferences) diff --git a/src/encoding_binding.h b/src/encoding_binding.h new file mode 100644 index 00000000000000..0fd2a467061177 --- /dev/null +++ b/src/encoding_binding.h @@ -0,0 +1,46 @@ +#ifndef SRC_ENCODING_BINDING_H_ +#define SRC_ENCODING_BINDING_H_ + +#if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#include +#include "aliased_buffer.h" +#include "node_snapshotable.h" +#include "v8-fast-api-calls.h" + +namespace node { +class ExternalReferenceRegistry; + +namespace encoding_binding { +class BindingData : public SnapshotableObject { + public: + BindingData(Environment* env, v8::Local obj); + + using InternalFieldInfo = InternalFieldInfoBase; + + SERIALIZABLE_OBJECT_METHODS() + SET_BINDING_ID(encoding_binding_data) + + SET_NO_MEMORY_INFO() + SET_SELF_SIZE(BindingData) + SET_MEMORY_INFO_NAME(BindingData) + + static void EncodeInto(const v8::FunctionCallbackInfo& args); + static void EncodeUtf8String(const v8::FunctionCallbackInfo& args); + static void DecodeUTF8(const v8::FunctionCallbackInfo& args); + + static void Initialize(v8::Local target, + v8::Local unused, + v8::Local context, + void* priv); + static void RegisterTimerExternalReferences( + ExternalReferenceRegistry* registry); +}; + +} // namespace encoding_binding + +} // namespace node + +#endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS + +#endif // SRC_ENCODING_BINDING_H_ \ No newline at end of file diff --git a/src/node_binding.cc b/src/node_binding.cc index 3c97c73964d207..48825171a43feb 100644 --- a/src/node_binding.cc +++ b/src/node_binding.cc @@ -42,6 +42,7 @@ V(config) \ V(contextify) \ V(credentials) \ + V(encoding_binding) \ V(errors) \ V(fs) \ V(fs_dir) \ diff --git a/src/node_buffer.cc b/src/node_buffer.cc index fec1c96634aaec..62651f4656b1b9 100644 --- a/src/node_buffer.cc +++ b/src/node_buffer.cc @@ -568,60 +568,6 @@ void StringSlice(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(ret); } -// Convert the input into an encoded string -void DecodeUTF8(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); // list, flags - - CHECK_GE(args.Length(), 1); - - if (!(args[0]->IsArrayBuffer() || args[0]->IsSharedArrayBuffer() || - args[0]->IsArrayBufferView())) { - return node::THROW_ERR_INVALID_ARG_TYPE( - env->isolate(), - "The \"list\" argument must be an instance of SharedArrayBuffer, " - "ArrayBuffer or ArrayBufferView."); - } - - ArrayBufferViewContents buffer(args[0]); - - bool ignore_bom = args[1]->IsTrue(); - bool has_fatal = args[2]->IsTrue(); - - const char* data = buffer.data(); - size_t length = buffer.length(); - - if (has_fatal) { - auto result = simdutf::validate_utf8_with_errors(data, length); - - if (result.error) { - return node::THROW_ERR_ENCODING_INVALID_ENCODED_DATA( - env->isolate(), "The encoded data was not valid for encoding utf-8"); - } - } - - if (!ignore_bom && length >= 3) { - if (memcmp(data, "\xEF\xBB\xBF", 3) == 0) { - data += 3; - length -= 3; - } - } - - if (length == 0) return args.GetReturnValue().SetEmptyString(); - - Local error; - MaybeLocal maybe_ret = - StringBytes::Encode(env->isolate(), data, length, UTF8, &error); - Local ret; - - if (!maybe_ret.ToLocal(&ret)) { - CHECK(!error.IsEmpty()); - env->isolate()->ThrowException(error); - return; - } - - args.GetReturnValue().Set(ret); -} - // bytesCopied = copy(buffer, target[, targetStart][, sourceStart][, sourceEnd]) void Copy(const FunctionCallbackInfo &args) { Environment* env = Environment::GetCurrent(args); @@ -1173,72 +1119,6 @@ void Swap64(const FunctionCallbackInfo& args) { args.GetReturnValue().Set(args[0]); } - -// Encode a single string to a UTF-8 Uint8Array (not Buffer). -// Used in TextEncoder.prototype.encode. -static void EncodeUtf8String(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - Isolate* isolate = env->isolate(); - CHECK_GE(args.Length(), 1); - CHECK(args[0]->IsString()); - - Local str = args[0].As(); - size_t length = str->Utf8Length(isolate); - - Local ab; - { - NoArrayBufferZeroFillScope no_zero_fill_scope(env->isolate_data()); - std::unique_ptr bs = - ArrayBuffer::NewBackingStore(isolate, length); - - CHECK(bs); - - str->WriteUtf8(isolate, - static_cast(bs->Data()), - -1, // We are certain that `data` is sufficiently large - nullptr, - String::NO_NULL_TERMINATION | String::REPLACE_INVALID_UTF8); - - ab = ArrayBuffer::New(isolate, std::move(bs)); - } - - auto array = Uint8Array::New(ab, 0, length); - args.GetReturnValue().Set(array); -} - - -static void EncodeInto(const FunctionCallbackInfo& args) { - Environment* env = Environment::GetCurrent(args); - Isolate* isolate = env->isolate(); - CHECK_GE(args.Length(), 3); - CHECK(args[0]->IsString()); - CHECK(args[1]->IsUint8Array()); - CHECK(args[2]->IsUint32Array()); - - Local source = args[0].As(); - - Local dest = args[1].As(); - Local buf = dest->Buffer(); - char* write_result = static_cast(buf->Data()) + dest->ByteOffset(); - size_t dest_length = dest->ByteLength(); - - // results = [ read, written ] - Local result_arr = args[2].As(); - uint32_t* results = reinterpret_cast( - static_cast(result_arr->Buffer()->Data()) + - result_arr->ByteOffset()); - - int nchars; - int written = source->WriteUtf8( - isolate, - write_result, - dest_length, - &nchars, - String::NO_NULL_TERMINATION | String::REPLACE_INVALID_UTF8); - results[0] = nchars; - results[1] = written; -} - static void IsUtf8(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); CHECK_EQ(args.Length(), 1); @@ -1382,7 +1262,6 @@ void Initialize(Local target, SetMethod(context, target, "setBufferPrototype", SetBufferPrototype); SetMethodNoSideEffect(context, target, "createFromString", CreateFromString); - SetMethodNoSideEffect(context, target, "decodeUTF8", DecodeUTF8); SetFastMethodNoSideEffect(context, target, @@ -1404,9 +1283,6 @@ void Initialize(Local target, SetMethod(context, target, "swap32", Swap32); SetMethod(context, target, "swap64", Swap64); - SetMethod(context, target, "encodeInto", EncodeInto); - SetMethodNoSideEffect(context, target, "encodeUtf8String", EncodeUtf8String); - SetMethodNoSideEffect(context, target, "isUtf8", IsUtf8); SetMethodNoSideEffect(context, target, "isAscii", IsAscii); @@ -1447,7 +1323,6 @@ void Initialize(Local target, void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(SetBufferPrototype); registry->Register(CreateFromString); - registry->Register(DecodeUTF8); registry->Register(SlowByteLengthUtf8); registry->Register(fast_byte_length_utf8.GetTypeInfo()); @@ -1464,9 +1339,6 @@ void RegisterExternalReferences(ExternalReferenceRegistry* registry) { registry->Register(Swap32); registry->Register(Swap64); - registry->Register(EncodeInto); - registry->Register(EncodeUtf8String); - registry->Register(IsUtf8); registry->Register(IsAscii); diff --git a/src/node_external_reference.h b/src/node_external_reference.h index 2eb3e3bf3cd458..1df9ebec6da581 100644 --- a/src/node_external_reference.h +++ b/src/node_external_reference.h @@ -68,6 +68,7 @@ class ExternalReferenceRegistry { V(cares_wrap) \ V(contextify) \ V(credentials) \ + V(encoding_binding) \ V(env_var) \ V(errors) \ V(fs) \ diff --git a/src/node_snapshotable.cc b/src/node_snapshotable.cc index d9d4f162e9cfd2..9b684ca83270e2 100644 --- a/src/node_snapshotable.cc +++ b/src/node_snapshotable.cc @@ -5,6 +5,7 @@ #include #include "base_object-inl.h" #include "debug_utils-inl.h" +#include "encoding_binding.h" #include "env-inl.h" #include "node_blob.h" #include "node_builtins.h" diff --git a/test/parallel/test-bootstrap-modules.js b/test/parallel/test-bootstrap-modules.js index 6efb806e205009..3a8cbfd0b69d28 100644 --- a/test/parallel/test-bootstrap-modules.js +++ b/test/parallel/test-bootstrap-modules.js @@ -10,6 +10,7 @@ const assert = require('assert'); const expectedModules = new Set([ 'Internal Binding builtins', + 'Internal Binding encoding_binding', 'Internal Binding errors', 'Internal Binding util', 'NativeModule internal/errors', From 9b54d805cea39244fdfb9c1d6a3542d2f076782a Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sat, 11 Feb 2023 19:25:00 +0100 Subject: [PATCH 3/5] encoding: use AliasedUint32Array for encodeInto results Getting the buffer from a TypedArray created from the JS land incurs a copy. For encodeInto() results we can just use an AliasedArray and let the binding always own the store. --- lib/internal/encoding.js | 15 +++++++------- src/encoding_binding.cc | 44 ++++++++++++++++++++++++---------------- src/encoding_binding.h | 8 ++++++-- 3 files changed, 41 insertions(+), 26 deletions(-) diff --git a/lib/internal/encoding.js b/lib/internal/encoding.js index 2ee569c737d18c..a6759d4266ae84 100644 --- a/lib/internal/encoding.js +++ b/lib/internal/encoding.js @@ -13,7 +13,6 @@ const { StringPrototypeSlice, Symbol, SymbolToStringTag, - Uint32Array, Uint8Array, } = primordials; @@ -49,12 +48,12 @@ const { validateString, validateObject, } = require('internal/validators'); - +const binding = internalBinding('encoding_binding'); const { encodeInto, encodeUtf8String, decodeUTF8, -} = internalBinding('encoding_binding'); +} = binding; const { Buffer } = require('buffer'); @@ -318,8 +317,6 @@ function getEncodingFromLabel(label) { return encodings.get(trimAsciiWhitespace(label.toLowerCase())); } -const encodeIntoResults = new Uint32Array(2); - class TextEncoder { constructor() { this[kEncoder] = true; @@ -340,8 +337,12 @@ class TextEncoder { validateString(src, 'src'); if (!dest || !isUint8Array(dest)) throw new ERR_INVALID_ARG_TYPE('dest', 'Uint8Array', dest); - encodeInto(src, dest, encodeIntoResults); - return { read: encodeIntoResults[0], written: encodeIntoResults[1] }; + + encodeInto(src, dest); + // We need to read from the binding here since the buffer gets refreshed + // from the snapshot. + const { 0: read, 1: written } = binding.encodeIntoResults; + return { read, written }; } [inspect](depth, opts) { diff --git a/src/encoding_binding.cc b/src/encoding_binding.cc index e9bc048a510a3c..c33d77845b13ab 100644 --- a/src/encoding_binding.cc +++ b/src/encoding_binding.cc @@ -21,15 +21,30 @@ using v8::Local; using v8::MaybeLocal; using v8::Object; using v8::String; -using v8::Uint8Array; using v8::Uint32Array; +using v8::Uint8Array; using v8::Value; -BindingData::BindingData(Environment* env, Local object) - : SnapshotableObject(env, object, type_int) {} +void BindingData::MemoryInfo(MemoryTracker* tracker) const { + tracker->TrackField("encode_into_results_buffer", + encode_into_results_buffer_); +} + +BindingData::BindingData(Realm* realm, v8::Local object) + : SnapshotableObject(realm, object, type_int), + encode_into_results_buffer_(realm->isolate(), kEncodeIntoResultsLength) { + object + ->Set(realm->context(), + FIXED_ONE_BYTE_STRING(realm->isolate(), "encodeIntoResults"), + encode_into_results_buffer_.GetJSArray()) + .Check(); +} bool BindingData::PrepareForSerialization(Local context, v8::SnapshotCreator* creator) { + // We'll just re-initialize the buffers in the constructor since their + // contents can be thrown away once consumed in the previous call. + encode_into_results_buffer_.Release(); // Return true because we need to maintain the reference to the binding from // JS land. return true; @@ -48,19 +63,19 @@ void BindingData::Deserialize(Local context, InternalFieldInfoBase* info) { DCHECK_EQ(index, BaseObject::kEmbedderType); v8::HandleScope scope(context->GetIsolate()); - Environment* env = Environment::GetCurrent(context); + Realm* realm = Realm::GetCurrent(context); // Recreate the buffer in the constructor. - BindingData* binding = env->AddBindingData(context, holder); + BindingData* binding = realm->AddBindingData(context, holder); CHECK_NOT_NULL(binding); } void BindingData::EncodeInto(const FunctionCallbackInfo& args) { Environment* env = Environment::GetCurrent(args); Isolate* isolate = env->isolate(); - CHECK_GE(args.Length(), 3); + CHECK_GE(args.Length(), 2); CHECK(args[0]->IsString()); CHECK(args[1]->IsUint8Array()); - CHECK(args[2]->IsUint32Array()); + BindingData* binding_data = Realm::GetBindingData(args); Local source = args[0].As(); @@ -69,12 +84,6 @@ void BindingData::EncodeInto(const FunctionCallbackInfo& args) { char* write_result = static_cast(buf->Data()) + dest->ByteOffset(); size_t dest_length = dest->ByteLength(); - // results = [ read, written ] - Local result_arr = args[2].As(); - uint32_t* results = reinterpret_cast( - static_cast(result_arr->Buffer()->Data()) + - result_arr->ByteOffset()); - int nchars; int written = source->WriteUtf8( isolate, @@ -82,8 +91,9 @@ void BindingData::EncodeInto(const FunctionCallbackInfo& args) { dest_length, &nchars, String::NO_NULL_TERMINATION | String::REPLACE_INVALID_UTF8); - results[0] = nchars; - results[1] = written; + + binding_data->encode_into_results_buffer_[0] = nchars; + binding_data->encode_into_results_buffer_[1] = written; } // Encode a single string to a UTF-8 Uint8Array (not Buffer). @@ -176,9 +186,9 @@ void BindingData::Initialize(Local target, Local unused, Local context, void* priv) { - Environment* env = Environment::GetCurrent(context); + Realm* realm = Realm::GetCurrent(context); BindingData* const binding_data = - env->AddBindingData(context, target); + realm->AddBindingData(context, target); if (binding_data == nullptr) return; SetMethod(context, target, "encodeInto", EncodeInto); diff --git a/src/encoding_binding.h b/src/encoding_binding.h index 0fd2a467061177..14b0dc84f27d13 100644 --- a/src/encoding_binding.h +++ b/src/encoding_binding.h @@ -14,14 +14,14 @@ class ExternalReferenceRegistry; namespace encoding_binding { class BindingData : public SnapshotableObject { public: - BindingData(Environment* env, v8::Local obj); + BindingData(Realm* realm, v8::Local obj); using InternalFieldInfo = InternalFieldInfoBase; SERIALIZABLE_OBJECT_METHODS() SET_BINDING_ID(encoding_binding_data) - SET_NO_MEMORY_INFO() + void MemoryInfo(MemoryTracker* tracker) const override; SET_SELF_SIZE(BindingData) SET_MEMORY_INFO_NAME(BindingData) @@ -35,6 +35,10 @@ class BindingData : public SnapshotableObject { void* priv); static void RegisterTimerExternalReferences( ExternalReferenceRegistry* registry); + +private: + static constexpr size_t kEncodeIntoResultsLength = 2; + AliasedUint32Array encode_into_results_buffer_; }; } // namespace encoding_binding From 1233e7fef547e676073f87703e48b82705b41fb9 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Tue, 7 Mar 2023 17:56:56 +0100 Subject: [PATCH 4/5] fixup! encoding: use AliasedUint32Array for encodeInto results --- src/encoding_binding.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/encoding_binding.h b/src/encoding_binding.h index 14b0dc84f27d13..13be4f39a688eb 100644 --- a/src/encoding_binding.h +++ b/src/encoding_binding.h @@ -36,7 +36,7 @@ class BindingData : public SnapshotableObject { static void RegisterTimerExternalReferences( ExternalReferenceRegistry* registry); -private: + private: static constexpr size_t kEncodeIntoResultsLength = 2; AliasedUint32Array encode_into_results_buffer_; }; From 4ea7f2e2cdbcf30a18b51c4f31dd2e5c4a4eb54e Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Tue, 7 Mar 2023 17:57:11 +0100 Subject: [PATCH 5/5] fixup! src: move encoding bindings to a new binding --- src/encoding_binding.cc | 2 -- src/encoding_binding.h | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/src/encoding_binding.cc b/src/encoding_binding.cc index c33d77845b13ab..467c4c11efd2bf 100644 --- a/src/encoding_binding.cc +++ b/src/encoding_binding.cc @@ -14,14 +14,12 @@ namespace encoding_binding { using v8::ArrayBuffer; using v8::BackingStore; using v8::Context; -using v8::Function; using v8::FunctionCallbackInfo; using v8::Isolate; using v8::Local; using v8::MaybeLocal; using v8::Object; using v8::String; -using v8::Uint32Array; using v8::Uint8Array; using v8::Value; diff --git a/src/encoding_binding.h b/src/encoding_binding.h index 13be4f39a688eb..51e006982b9363 100644 --- a/src/encoding_binding.h +++ b/src/encoding_binding.h @@ -47,4 +47,4 @@ class BindingData : public SnapshotableObject { #endif // defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS -#endif // SRC_ENCODING_BINDING_H_ \ No newline at end of file +#endif // SRC_ENCODING_BINDING_H_