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: #46658 Reviewed-By: Darshan Sen <[email protected]>
nodejs · Mar 13, 2023 · 7ec0dcb · 7ec0dcb
1 parent db4fa93
commit 7ec0dcb
Show file tree

Hide file tree

Showing 3 changed files with 41 additions and 26 deletions.
diff --git 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
@@ -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> 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<v8::Object> 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> 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> 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<BindingData>(context, holder);
+  BindingData* binding = realm->AddBindingData<BindingData>(context, holder);
   CHECK_NOT_NULL(binding);
 }
 
 void BindingData::EncodeInto(const FunctionCallbackInfo<Value>& 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<BindingData>(args);
 
   Local<String> source = args[0].As<String>();
 
@@ -68,21 +83,16 @@ void BindingData::EncodeInto(const FunctionCallbackInfo<Value>& args) {
   char* write_result = static_cast<char*>(buf->Data()) + dest->ByteOffset();
   size_t dest_length = dest->ByteLength();
 
-  // results = [ read, written ]
-  Local<Uint32Array> result_arr = args[2].As<Uint32Array>();
-  uint32_t* results = reinterpret_cast<uint32_t*>(
-      static_cast<char*>(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;
+
+  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<Object> target,
                              Local<Value> unused,
                              Local<Context> context,
                              void* priv) {
-  Environment* env = Environment::GetCurrent(context);
+  Realm* realm = Realm::GetCurrent(context);
   BindingData* const binding_data =
-      env->AddBindingData<BindingData>(context, target);
+      realm->AddBindingData<BindingData>(context, target);
   if (binding_data == nullptr) return;
 
   SetMethod(context, target, "encodeInto", EncodeInto);

diff --git 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<v8::Object> obj);
+  BindingData(Realm* realm, v8::Local<v8::Object> 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