[Fizz] Reduce chunk push overhead in HTML serialization#36139
[Fizz] Reduce chunk push overhead in HTML serialization#36139switz wants to merge 1 commit intofacebook:mainfrom
Conversation
Batch attribute and style serialization into single string pushes instead of 3-5 separate push calls per attribute. This reduces array operations, chunk count in segment buffers, and downstream Buffer.byteLength and writeChunk/encodeInto call volume. Changes: - pushStringAttribute: 5 pushes → 1 concatenated string push - pushBooleanAttribute: 3 pushes → 1 push - pushStyleAttribute: 4N+1 pushes → 1 push (build full style string) - pushAttribute default/URL/enumerated/numeric cases: 5 pushes → 1 push - processStyleName: returns plain string instead of PrecomputedChunk - pushStartGenericElement/pushStartAnchor: merge close tag with text children into single push when no dangerouslySetInnerHTML - byteLengthOfChunk: use string.length instead of Buffer.byteLength for heuristic byte sizing (outlining threshold, progressive chunks). For ASCII (99%+ of HTML output), length === byte length. Benchmark (Node v24, Apple M1 Max, 1000 iterations, 100 warmup): | Tree | Before | After | Δ | |-----------------------------|-----------|-----------|----------| | Small (10 el) | 37,959 | 48,257 | +27.1% | | Medium (300 el, Suspense) | 2,309 | 3,518 | +52.4% | | Medium (300 el, no Suspense)| 2,404 | 3,725 | +54.9% | | Large (2500+ el, Suspense) | 429 | 626 | +45.9% |
|
Hi @switz! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks! |
I spent some time yesterday digging into RSC performance with Claude. I think there are two much bigger issues to tackle than this (one being web vs. node streams and the other being an architectural bottleneck), but this PR did provide some nice wins in a variety of benchmarks.
I have a separate improvement to Flight on another branch, but I'm a tad more skeptical of that PR. Plus, its gains were more modest.
Benchmark (Node v24, Apple M1 Max, 1000 iterations, 100 warmup):
This is just one small part of the pipeline, so it's not representative of major e2e gains.
Below are Claude's notes–feel free to turn down this PR if you find its efficacy lacking. The
byteLengthOfChunkchange I'm a touch more nervous about, but if it's only used for heuristics this feels okay.Summary
Batches attribute and style serialization into single string pushes instead of 3-5 separate
target.push()calls per attribute. This reduces array operations, chunk count in segment buffers, anddownstream
Buffer.byteLengthandwriteChunk/encodeIntocall volume.Motivation
Profiling
renderToPipeableStreamwith V8's--profshowed thatpushAttributeandpushStartGenericElementwere the top JS hot paths, with the dominant cost being the sheer number oftarget.push()calls per element. A<div>with 6 attributes and a 6-property style object was doing~54 array pushes for attributes alone. Each push adds an item to the segment's chunk array, which then
has to be iterated by
finishedSegmentfor byte sizing and byflushSubtreefor encoding.Changes
Attribute push batching (
ReactFizzConfigDOM.js):pushStringAttribute: 5 pushes (attributeSeparator,name,attributeAssign,escapedValue,attributeEnd) → 1 push with' ' + name + '="' + escapeTextForBrowser(value) + '"'pushBooleanAttribute: 3 pushes → 1 pushpushStyleAttribute: 4N+1 pushes (N = number of style properties) → 1 push. Builds the entirestyle="..."string before pushing.pushAttributedefault case (data-, aria-, etc.), URL attributes, enumerated attributes, numericattributes, overloaded booleans: all consolidated from 3-5 pushes to 1.
processStyleName: returns plain string instead ofPrecomputedChunkto enable string concatenationin style building.
Text child merging (
ReactFizzConfigDOM.js):pushStartGenericElementandpushStartAnchor: when children is a string and there's nodangerouslySetInnerHTML, merge the>with the escaped text into a single string push.Byte length fast path (
ReactServerStreamConfigNode.js):byteLengthOfChunk: usestring.lengthinstead ofBuffer.byteLength(chunk, 'utf8'). For ASCIIcontent (99%+ of HTML attribute/tag output),
.length === byte length. The slight underestimate formulti-byte characters is acceptable since
byteSizeis only used for heuristic decisions (outliningthreshold at 500 bytes, progressive chunk sizing).
Results
Benchmarked with
renderToPipeableStreamon Node v24.14.0, Apple M1 Max. Renders piped to abyte-counting Writable (no I/O). 100 warmup, 1000 measured iterations, <1% coefficient of variation.
Output HTML is byte-identical before and after.
How did you test this change?
All existing Fizz and Server integration tests pass (4,088 tests across 150 suites). Verified HTML
output identity for elements with className, id, style objects, data-* attributes, escaped text content,
void elements, and SVG namespace attributes.