From f2d6be279924685cec5af4151b7947446a95a52a Mon Sep 17 00:00:00 2001 From: Eduardo Speroni Date: Tue, 25 Aug 2026 16:29:02 -0300 Subject: [PATCH] perf(runtime): reuse the marshalled UTF-8 buffer for repeated C string arguments Marshalling a JS string to char* copied it with strdup on every call and parked each copy on a throwaway external string for GC to reclaim; the IsExternalOneByte fast path above it could never fire for runtime-created strings because the argument itself was never externalized. WriteValue now externalizes the argument in place via MakeExternal when the string is pure ASCII (UTF-8 byte count == UTF-16 length, the exact condition under which the buffer equals the string's one-byte content) and V8 permits it, so later marshals of the same string reuse the buffer with no copy. Non-ASCII, young-generation, and otherwise non-externalizable strings keep the previous copy-per-call behavior. The fast path is also now gated on an ownership registry in OneByteStringResource: only buffers this runtime created are known to be NUL-terminated UTF-8, while a foreign resource's data is Latin-1 with no terminator guarantee and can no longer be handed to native code blindly. --- NativeScript/runtime/Interop.mm | 36 +++++++++++++------ .../runtime/OneByteStringResource.cpp | 24 +++++++++++++ NativeScript/runtime/OneByteStringResource.h | 8 ++++- .../app/tests/Marshalling/ReferenceTests.js | 31 ++++++++++++++++ 4 files changed, 87 insertions(+), 12 deletions(-) diff --git a/NativeScript/runtime/Interop.mm b/NativeScript/runtime/Interop.mm index 8a6544b2..4dacbe8c 100644 --- a/NativeScript/runtime/Interop.mm +++ b/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 { diff --git a/NativeScript/runtime/OneByteStringResource.cpp b/NativeScript/runtime/OneByteStringResource.cpp index e10086ca..4100a9ff 100644 --- a/NativeScript/runtime/OneByteStringResource.cpp +++ b/NativeScript/runtime/OneByteStringResource.cpp @@ -1,16 +1,35 @@ #include "OneByteStringResource.h" #include +#include +#include using namespace v8; +namespace { + +std::mutex registryMutex; + +std::unordered_set& Registry() { + static auto* registry = new std::unordered_set(); + return *registry; +} + +} // namespace + namespace tns { OneByteStringResource::OneByteStringResource(const char* data, size_t length): data_(data), length_(length) { + std::lock_guard lock(registryMutex); + Registry().insert(this); } OneByteStringResource::~OneByteStringResource() { + { + std::lock_guard lock(registryMutex); + Registry().erase(this); + } // data_ comes from strdup (see Interop::WriteValue's CStringEncoding path). std::free(const_cast(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 lock(registryMutex); + return Registry().count(resource) != 0; +} } diff --git a/NativeScript/runtime/OneByteStringResource.h b/NativeScript/runtime/OneByteStringResource.h index 0cf0a8fe..b81a5611 100644 --- a/NativeScript/runtime/OneByteStringResource.h +++ b/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_; }; diff --git a/TestRunner/app/tests/Marshalling/ReferenceTests.js b/TestRunner/app/tests/Marshalling/ReferenceTests.js index c1810c3b..229bb1a5 100644 --- a/TestRunner/app/tests/Marshalling/ReferenceTests.js +++ b/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));