perf(runtime): reuse the marshalled UTF-8 buffer for repeated C string arguments

NativeScript/runtime/Interop.mm

@@ -270,18 +270,32 @@ inline bool isBool() {
       if (strArg->IsExternalOneByte()) {
         const v8::String::ExternalOneByteStringResource* resource =
             strArg->GetExternalOneByteStringResource();
-        value = resource->data();
-      } else {
+        if (OneByteStringResource::Owns(resource)) {
+          value = resource->data();
+        }
+      }
+
+      if (value == nullptr) {
         v8::String::Utf8Value utf8Value(isolate, arg);
-        value = strdup(*utf8Value);
-        // The external string only ties the strdup'd buffer's lifetime to the
-        // GC; it is never read as a string, and its one-byte (Latin-1) content
-        // matches the UTF-8 buffer only for ASCII. Length is the buffer's byte
-        // count, excluding the NUL.
-        OneByteStringResource* resource =
-            new OneByteStringResource(value, (size_t)utf8Value.length());
-        bool success = v8::String::NewExternalOneByte(isolate, resource).ToLocal(&arg);
-        tns::Assert(success, isolate);
+        char* buffer = strdup(*utf8Value);
+        size_t byteLength = (size_t)utf8Value.length();
+        value = buffer;
+        OneByteStringResource* resource = new OneByteStringResource(buffer, byteLength);
+
+        // A one-byte external string's content is Latin-1, which coincides
+        // with the UTF-8 buffer exactly when every character is ASCII - i.e.
+        // when the byte count equals the UTF-16 length. Externalizing the
+        // argument itself lets later marshals of the same string take the
+        // reuse path above.
+        bool externalized = byteLength == (size_t)strArg->Length() &&
+                            strArg->CanMakeExternal(v8::String::ONE_BYTE_ENCODING) &&
+                            strArg->MakeExternal(isolate, resource);
+        if (!externalized) {
+          // The external string only ties the buffer's lifetime to the GC; it
+          // is never read as a string.
+          bool success = v8::String::NewExternalOneByte(isolate, resource).ToLocal(&arg);
+          tns::Assert(success, isolate);
+        }
       }
       Interop::SetValue(dest, value);
     } else {

NativeScript/runtime/OneByteStringResource.cpp

@@ -1,16 +1,35 @@
 #include "OneByteStringResource.h"
 
 #include <cstdlib>
+#include <mutex>
+#include <unordered_set>
 
 using namespace v8;
 
+namespace {
+
+std::mutex registryMutex;
+
+std::unordered_set<const void*>& Registry() {
+  static auto* registry = new std::unordered_set<const void*>();
+  return *registry;
+}
+
+}  // namespace
+
 namespace tns {
 
 OneByteStringResource::OneByteStringResource(const char* data, size_t length):
     data_(data), length_(length) {
+  std::lock_guard<std::mutex> lock(registryMutex);
+  Registry().insert(this);
 }
 
 OneByteStringResource::~OneByteStringResource() {
+  {
+    std::lock_guard<std::mutex> lock(registryMutex);
+    Registry().erase(this);
+  }
   // data_ comes from strdup (see Interop::WriteValue's CStringEncoding path).
   std::free(const_cast<char*>(this->data_));
 }
@@ -23,4 +42,9 @@ size_t OneByteStringResource::length() const {
     return this->length_;
 }
 
+bool OneByteStringResource::Owns(
+    const v8::String::ExternalOneByteStringResource* resource) {
+  std::lock_guard<std::mutex> lock(registryMutex);
+  return Registry().count(resource) != 0;
+}
 }

NativeScript/runtime/OneByteStringResource.h

@@ -11,7 +11,13 @@ class OneByteStringResource : public v8::String::ExternalOneByteStringResource {
     ~OneByteStringResource() override;
     const char* data() const override;
     size_t length() const override;
-private:
+
+    // Whether this runtime created the resource. Only such resources are known
+    // to hold NUL-terminated UTF-8; V8's contract makes a foreign resource
+    // Latin-1 with no terminator guarantee.
+    static bool Owns(const v8::String::ExternalOneByteStringResource* resource);
+
+   private:
     const char* data_;
     size_t length_;
 };

TestRunner/app/tests/Marshalling/ReferenceTests.js

@@ -337,6 +337,37 @@ describe(module.id, function () {
         interop.free(ptr);
     });
 
+    it("reuses one marshalled buffer for repeated ASCII CString arguments", function () {
+        // Built at runtime so the string is a plain sequential string rather
+        // than an internalized literal, which V8 may refuse to externalize.
+        // V8 also refuses to externalize young-generation strings, so promote
+        // it to old space first - mirroring the real-world shape of a
+        // long-lived string marshalled repeatedly.
+        var str = Array(65).join("a");
+        __collect();
+        __collect();
+
+        // functionWithCharPtr echoes its argument pointer, exposing the
+        // address of the marshalled buffer: after the first call externalizes
+        // the string, later calls must reuse the same buffer instead of
+        // copying again.
+        var first = functionWithCharPtr(str);
+        var second = functionWithCharPtr(str);
+        expect(interop.handleof(first).toNumber()).toBe(interop.handleof(second).toNumber());
+        expect(NSString.stringWithUTF8String(first).toString()).toBe(str);
+    });
+
+    it("marshals non-ASCII CString arguments as UTF-8 on every call", function () {
+        // Non-ASCII strings cannot be externalized as one-byte (Latin-1), so
+        // each marshal takes the copying path; the content must round-trip as
+        // UTF-8 both times.
+        var str = ["héllo", "wörld", "🙂"].join(" ");
+        var first = functionWithCharPtr(str);
+        expect(NSString.stringWithUTF8String(first).toString()).toBe(str);
+        var second = functionWithCharPtr(str);
+        expect(NSString.stringWithUTF8String(second).toString()).toBe(str);
+    });
+
     it("interops string from CString", function () {
         const str = "test";
         const ptr = interop.alloc((str.length + 1) * interop.sizeof(interop.types.uint8));