[Flight] Pre-walk rendered model instead of using JSON.stringify replacer#36181
Open
unstubbable wants to merge 1 commit intofacebook:mainfrom
Open
[Flight] Pre-walk rendered model instead of using JSON.stringify replacer#36181unstubbable wants to merge 1 commit intofacebook:mainfrom
JSON.stringify replacer#36181unstubbable wants to merge 1 commit intofacebook:mainfrom
Conversation
Co-authored-by: Michael Hart <mhart@cloudflare.com>
github-actions
bot
added
the
React Core Team
unstubbable
changed the title
JSON.stringify replacer
|
You could've asked me to just pull this out from my PR into a new one – I would've happily done it 😉 |
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
3 participants
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.
This PR replaces the
JSON.stringifyreplacer callback (task.toJSON) with a two-step approach: a recursiveresolveModel()pre-walk followed by a plainJSON.stringify()call with no replacer. This is the server-side counterpart to the client-sideJSON.parsereviver removal in #35776.Based on the approach from #36053 by @mhart.
Problem
When serializing a Flight chunk,
emitChunkcurrently callsJSON.stringify(value, task.toJSON). Thetask.toJSONreplacer is called for every key-value pair in the serialized JSON. While the logic inside the replacer is lightweight, the C++ to JavaScript boundary crossing on every node adds up — V8'sJSON.stringifyis implemented in C++, and calling back into JavaScript for every property incurs overhead that scales with the number of keys in the output.Change
Replace the replacer with a two-step process:
resolveModel()recursively walks the rendered value, callingrenderModel()on each child — doing the same transformation the replacer used to do, but entirely in JavaScript without C++ boundary crossings.JSON.stringify()is called with no replacer, staying entirely in C++.The
resolveModelwalk also replicatesJSON.stringify'stoJSONsemantics forDateobjects.Results
Measured using the Flight SSR benchmark fixture (#36180) on a dashboard app with ~25 components, 200 product rows (~325KB Flight payload). Tested across Node 20, 22, and 24.
bench:bare(in-process, no script injection): Flight+Fizz sync median improves by ~4-5% consistently across all three Node versions.bench:server(HTTP, c=1): Flight+Fizz sync throughput improves by ~3-6% across Node versions. Async results vary between runs but trend positive.Future opportunity
While the immediate performance improvement is moderate, this change also sets up a potential future optimization: a Flight mode that renders to an object instead of a stream (#36143 (comment)). Since
resolveModel()already produces a plain JS object tree beforeJSON.stringifyis called, this intermediate representation could potentially be passed to the SSR client without the serialization-deserialization roundtrip that the current stream-based approach requires.