From 7ec0dcb28045845bafcfdc27b8479962f849b876 Mon Sep 17 00:00:00 2001 From: Joyee Cheung Date: Sat, 11 Feb 2023 19:25:00 +0100 Subject: [PATCH] src: 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. PR-URL: https://github.com/nodejs/node/pull/46658 Reviewed-By: Darshan Sen --- 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 0f54990907549b..7674579f88701e 100644 --- a/src/encoding_binding.cc +++ b/src/encoding_binding.cc @@ -20,15 +20,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; @@ -47,19 +62,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(); @@ -68,12 +83,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, @@ -81,8 +90,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). @@ -175,9 +185,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 472b6bb4ad03ad..51e006982b9363 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