[edusperoni]

Stacked on #524 (base fix/cstring-arg-lifetime); review that first.

Motivation

Every JS string passed to a char* parameter took three allocations and two copies before the call:

1. String::Utf8Value: Utf8LengthV2, allocation, WriteUtf8V2
2. strdup of that buffer: second allocation and copy, then the first is freed
3. a separately newed OneByteStringResource

Changes

OneByteStringResource::NewUtf8Copy sizes one malloc block for the resource plus the UTF-8 bytes and has V8 write straight into it, with the same flags Utf8Value uses (kNullTerminate | kReplaceInvalidUtf8, so lone surrogates still become U+FFFD). Dispose() destroys the resource and frees the block. The destructor is private, so the resource can only be released through Dispose().

The external string that ties the copy's lifetime to the GC is unchanged, so a pointer into the argument is valid exactly as long as before. Returned aliases (strchr, strstr), strtol's endptr and strtok's saved state behave the same.

Benchmarks

Release build, iPhone 16 Pro simulator, median of 5 alternating launches, each the best of 7 timed trials (ns per call). strlen keeps the native side negligible, so the deltas are marshalling cost.

case	main	#524	this PR	no V8 string*
labs(-1) (no string, baseline)	153	153	148	152
strlen("")	199	206	185	161
strlen(8 ASCII)	210	209	192	172
strlen(64 ASCII)	220	221	187	172
strlen(64 non-ASCII)	243	243	209	198
strlen(4 KB ASCII)	812	823	662	643

* Not part of this PR: this PR with the GC-tie external string removed (the copy leaks), built only to measure what that string still costs. About 15–25 ns per call, or 8–10% of a short-string call. That's the most a call-owned copy (freed when the call returns) could still save, and it would shorten pointer lifetimes, which can't be done compatibly. Not pursued.

Benchmark spec

Run with the Release A/B harness (run_bench.sh build / run / compare_bench.py); the spec only needs strlen and labs from the SDK metadata.

describe("CStringBench", function () {
    var now = (typeof performance !== "undefined" && performance.now) ? function () { return performance.now(); } : function () { return Date.now(); };

    function bench(name, fn, iters) {
        for (var w = 0; w < 5000; w++) fn();
        var best = Infinity;
        for (var trial = 0; trial < 7; trial++) {
            var t0 = now();
            for (var i = 0; i < iters; i++) fn();
            var ns = (now() - t0) * 1e6 / iters;
            if (ns < best) best = ns;
        }
        console.log("BENCH-JS " + name + ": " + best.toFixed(1) + " ns/op");
    }

    function repeat(s, n) { var out = ""; for (var i = 0; i < n; i++) out += s; return out; }

    it("times C-string argument marshalling", function () {
        var N = 200000;
        var sink = 0;
        var short8 = "abcdefgh";
        var medium64 = repeat("abcdefgh", 8);
        var long4k = repeat("abcdefgh", 512);
        var nonAscii64 = repeat("héllo wö", 8);
        var empty = "";
        // Flatten the concatenations once so every case marshals a flat string.
        sink += medium64.charCodeAt(0) + long4k.charCodeAt(0) + nonAscii64.charCodeAt(0);

        bench("baseline: labs(-1) (no string)", function () { sink += labs(-1); }, N);
        bench("strlen(\"\")", function () { sink += strlen(empty); }, N);
        bench("strlen(8 ASCII)", function () { sink += strlen(short8); }, N);
        bench("strlen(64 ASCII)", function () { sink += strlen(medium64); }, N);
        bench("strlen(64 non-ASCII)", function () { sink += strlen(nonAscii64); }, N);
        bench("strlen(4 KB ASCII)", function () { sink += strlen(long4k); }, N / 10);

        console.log("BENCH-JS done sink=" + sink);
    });
});

Tests

No new specs: the output is byte-for-byte the same, so the existing C-string specs (including #524's "" and external Latin-1 cases) cover it.

• Full Debug suite: 1837 tests, 0 failures.
• Full suite under ASan (-a): 1837 tests, 0 failures, no ASan reports.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: bd9b2815-9050-465a-b877-a15d171c1d4f

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}