Repository navigation
Conversation
Tony133
marked this pull request as ready for review
October 22, 2026 08:35
Member
|
I'd wait for another review |
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposal:
Follow-up to #900 and #903.
Problem
#900 gave plain
numberandintegerarrays their own measured cutoffs for the copy-then-JSON.stringifypath (128 / 512 on Node 22+), but plainstringarrays are still delegated only at or abovelargeArraySize(default 20000). Below that, every element goes through the generated loop andasString, which escapes in JS and is far slower than the native serializer. In practice atags: string[]with 10-100 elements, which is the common case, never hits the fast path.With #903 in place the copy actually reaches V8's fast path on Node 25+, which makes the gap even wider.
Measurement
Crossover (array length from which the copy beats the generated loop), median of 5 interleaved rounds, output consumed with
Buffer.byteLength,mainafter #903. Strings of 8, 40 and 120 chars, with and without characters to escape.On Node 22+ the copy wins from 2 elements everywhere, and is 2-6x faster from 8 elements on. On Node 20 the native serializer is slower on long strings, so the safe cutoff is 128.
Change
Add
stringtoPRIMITIVE_FAST_PATH_MIN_LENGTHwith the same two-tier structure asnumber/integer: 8 on Node 22+, 128 on Node 20.largeArraySizenow only driveslargeArrayMechanism: 'json-stringify', which was its original purpose. Output is byte-identical; the miss/resume semantics ofasPlainStringArrayare unchanged. README and thelargeArraySizeJSDoc updated accordingly.Below the cutoff nothing changes.
largeArrayMechanism: 'json-stringify'is unaffected.Benchmark (before =
mainwith #903, after = this PR)Median of 5 interleaved rounds, output consumed with
Buffer.byteLength.nativeis plainJSON.stringify;beforeandafterare fast-json-stringify onmainand on this branch.tags[n]is{ id, name, tags: string[n] }, the typical Fastify response shape.Node 26.12.0 (V8 14.6)
Node 24.19.0 (V8 13.6)
Node 22.23.1 (V8 12.4)
Node 20.20.2 (V8 11.3) — cutoff 128, so arrays below it keep the loop:
Arrays below the cutoff and the
json-stringifymechanism are unchanged on every runtime (within ±3%). Above the cutoff, string arrays go from 2-5x slower than native to within 5-30%.Tests
test/string-array-cutoff.test.js: the generated code uses the string cutoff and ignoreslargeArraySize; output is byte-identical toJSON.stringifyaround the cutoff (−1, 0, +1, ×10); nested string arrays use the same cutoff; escapes, non-strings,null,DateandRegExpelements above the cutoff keep the per-element coercion semantics;largeArraySizestill drives thejson-stringifymechanism. Full suite green withc8 --100on Node 22, 24 and 26.Semver
The
largeArraySizeoption no longer affects plain string arrays under the default mechanism. Output is unchanged, but the documented behaviour of an option changes, so this is probablysemver-minor.