Skip to content

[Fizz] Reduce chunk push overhead in HTML serialization #36139

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
switz wants to merge 1 commit into
base: main
Choose a base branch
from
+63 −89
Open

[Fizz] Reduce chunk push overhead in HTML serialization #36139

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
145 changes: 58 additions & 87 deletions packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1180,22 +1180,20 @@
}
}

const styleNameCache: Map<string, PrecomputedChunk> = new Map();
function processStyleName(styleName: string): PrecomputedChunk {
const chunk = styleNameCache.get(styleName);
if (chunk !== undefined) {
return chunk;
}
const result = stringToPrecomputedChunk(
escapeTextForBrowser(hyphenateStyleName(styleName)),
);
const styleNameCache: Map<string, string> = new Map();
function processStyleName(styleName: string): string {
const cached = styleNameCache.get(styleName);
if (cached !== undefined) {
return cached;
}
const result = escapeTextForBrowser(hyphenateStyleName(styleName));
styleNameCache.set(styleName, result);
return result;
}

const styleAttributeStart = stringToPrecomputedChunk(' style="');

Check failure on line 1194 in packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js

View workflow job for this annotation

GitHub Actions / Run eslint 

'styleAttributeStart' is assigned a value but never used
const styleAssign = stringToPrecomputedChunk(':');

Check failure on line 1195 in packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js

View workflow job for this annotation

GitHub Actions / Run eslint 

'styleAssign' is assigned a value but never used
const styleSeparator = stringToPrecomputedChunk(';');

Check failure on line 1196 in packages/react-dom-bindings/src/server/ReactFizzConfigDOM.js

View workflow job for this annotation

GitHub Actions / Run eslint 

'styleSeparator' is assigned a value but never used

function pushStyleAttribute(
target: Array<Chunk | PrecomputedChunk>,
Expand All @@ -1209,7 +1207,7 @@
);
}

let isFirst = true;
let styleString = '';
for (const styleName in style) {
if (!hasOwnProperty.call(style, styleName)) {
continue;
Expand All @@ -1231,48 +1229,42 @@
continue;
}

let nameChunk;
let valueChunk;
let nameStr;
let valueStr;
const isCustomProperty = styleName.indexOf('--') === 0;
if (isCustomProperty) {
nameChunk = stringToChunk(escapeTextForBrowser(styleName));
nameStr = escapeTextForBrowser(styleName);
if (__DEV__) {
checkCSSPropertyStringCoercion(styleValue, styleName);
}
valueChunk = stringToChunk(
escapeTextForBrowser(('' + styleValue).trim()),
);
valueStr = escapeTextForBrowser(('' + styleValue).trim());
} else {
if (__DEV__) {
warnValidStyle(styleName, styleValue);
}

nameChunk = processStyleName(styleName);
nameStr = processStyleName(styleName);
if (typeof styleValue === 'number') {
if (styleValue !== 0 && !isUnitlessNumber(styleName)) {
valueChunk = stringToChunk(styleValue + 'px'); // Presumes implicit 'px' suffix for unitless numbers
valueStr = styleValue + 'px'; // Presumes implicit 'px' suffix for unitless numbers
} else {
valueChunk = stringToChunk('' + styleValue);
valueStr = '' + styleValue;
}
} else {
if (__DEV__) {
checkCSSPropertyStringCoercion(styleValue, styleName);
}
valueChunk = stringToChunk(
escapeTextForBrowser(('' + styleValue).trim()),
);
valueStr = escapeTextForBrowser(('' + styleValue).trim());
}
}
if (isFirst) {
isFirst = false;
// If it's first, we don't need any separators prefixed.
target.push(styleAttributeStart, nameChunk, styleAssign, valueChunk);
if (styleString === '') {
styleString = nameStr + ':' + valueStr;
} else {
target.push(styleSeparator, nameChunk, styleAssign, valueChunk);
styleString += ';' + nameStr + ':' + valueStr;
}
}
if (!isFirst) {
target.push(attributeEnd);
if (styleString !== '') {
target.push(stringToChunk(' style="' + styleString + '"'));
}
}

Expand All @@ -1287,7 +1279,7 @@
value: string | boolean | number | Function | Object, // not null or undefined
): void {
if (value && typeof value !== 'function' && typeof value !== 'symbol') {
target.push(attributeSeparator, stringToChunk(name), attributeEmptyString);
target.push(stringToChunk(' ' + name + '=""'));
}
}

Expand All @@ -1302,11 +1294,7 @@
typeof value !== 'boolean'
) {
target.push(
attributeSeparator,
stringToChunk(name),
attributeAssign,
stringToChunk(escapeTextForBrowser(value)),
attributeEnd,
stringToChunk(' ' + name + '="' + escapeTextForBrowser(value) + '"'),
);
}
}
Expand Down Expand Up @@ -1610,11 +1598,9 @@
}
const sanitizedValue = sanitizeURL('' + value);
target.push(
attributeSeparator,
stringToChunk(name),
attributeAssign,
stringToChunk(escapeTextForBrowser(sanitizedValue)),
attributeEnd,
stringToChunk(
' ' + name + '="' + escapeTextForBrowser(sanitizedValue) + '"',
),
);
return;
}
Expand Down Expand Up @@ -1645,11 +1631,9 @@
}
const sanitizedValue = sanitizeURL('' + value);
target.push(
attributeSeparator,
stringToChunk('xlink:href'),
attributeAssign,
stringToChunk(escapeTextForBrowser(sanitizedValue)),
attributeEnd,
stringToChunk(
' xlink:href="' + escapeTextForBrowser(sanitizedValue) + '"',
),
);
return;
}
Expand All @@ -1667,11 +1651,9 @@
// these aren't boolean attributes (they are coerced to strings).
if (typeof value !== 'function' && typeof value !== 'symbol') {
target.push(
attributeSeparator,
stringToChunk(name),
attributeAssign,
stringToChunk(escapeTextForBrowser(value)),
attributeEnd,
stringToChunk(
' ' + name + '="' + escapeTextForBrowser(value) + '"',
),
);
}
return;
Expand Down Expand Up @@ -1727,20 +1709,14 @@
case 'download': {
// Overloaded Boolean
if (value === true) {
target.push(
attributeSeparator,
stringToChunk(name),
attributeEmptyString,
);
target.push(stringToChunk(' ' + name + '=""'));
} else if (value === false) {
// Ignored
} else if (typeof value !== 'function' && typeof value !== 'symbol') {
target.push(
attributeSeparator,
stringToChunk(name),
attributeAssign,
stringToChunk(escapeTextForBrowser(value)),
attributeEnd,
stringToChunk(
' ' + name + '="' + escapeTextForBrowser(value) + '"',
),
);
}
return;
Expand All @@ -1757,11 +1733,9 @@
(value: any) >= 1
) {
target.push(
attributeSeparator,
stringToChunk(name),
attributeAssign,
stringToChunk(escapeTextForBrowser(value)),
attributeEnd,
stringToChunk(
' ' + name + '="' + escapeTextForBrowser(value) + '"',
),
);
}
return;
Expand All @@ -1775,11 +1749,9 @@
!isNaN(value)
) {
target.push(
attributeSeparator,
stringToChunk(name),
attributeAssign,
stringToChunk(escapeTextForBrowser(value)),
attributeEnd,
stringToChunk(
' ' + name + '="' + escapeTextForBrowser(value) + '"',
),
);
}
return;
Expand Down Expand Up @@ -1837,11 +1809,13 @@
}
}
target.push(
attributeSeparator,
stringToChunk(attributeName),
attributeAssign,
stringToChunk(escapeTextForBrowser(value)),
attributeEnd,
stringToChunk(
' ' +
attributeName +
'="' +
escapeTextForBrowser(value) +
'"',
),
);
}
}
Expand Down Expand Up @@ -1956,14 +1930,12 @@

pushViewTransitionAttributes(target, formatContext);

target.push(endOfStartTag);
pushInnerHTML(target, innerHTML, children);
if (typeof children === 'string') {
// Special case children as a string to avoid the unnecessary comment.
// TODO: Remove this special case after the general optimization is in place.
target.push(stringToChunk(encodeHTMLTextNode(children)));
if (innerHTML == null && typeof children === 'string') {
target.push(stringToChunk('>' + encodeHTMLTextNode(children)));
return null;
}
target.push(endOfStartTag);
pushInnerHTML(target, innerHTML, children);
return children;
}

Expand Down Expand Up @@ -3964,14 +3936,13 @@

pushViewTransitionAttributes(target, formatContext);

target.push(endOfStartTag);
pushInnerHTML(target, innerHTML, children);
if (typeof children === 'string') {
// Special case children as a string to avoid the unnecessary comment.
// TODO: Remove this special case after the general optimization is in place.
target.push(stringToChunk(encodeHTMLTextNode(children)));
if (innerHTML == null && typeof children === 'string') {
// Fast path: close tag and emit text child in a single push.
target.push(stringToChunk('>' + encodeHTMLTextNode(children)));
Copy link
Copy Markdown
Collaborator

@gnoff gnoff Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason it was written the way it was before is we try to avoid branching on hot paths as much as possible. In this case you are adding an extra if check to every element that has non string children because we check the innerHTML twice instead of once.

There is probably a more dramatic refactor that could avoid the extra conditional and still allow for the avoidance of the extra chunk.

return null;
}
target.push(endOfStartTag);
pushInnerHTML(target, innerHTML, children);
return children;
}

Expand Down
7 changes: 5 additions & 2 deletions packages/react-server/src/ReactServerStreamConfigNode.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -222,8 +222,11 @@ export function typedArrayToBinaryChunk(

export function byteLengthOfChunk(chunk: Chunk | PrecomputedChunk): number {
return typeof chunk === 'string'
? Buffer.byteLength(chunk, 'utf8')
: chunk.byteLength;
? chunk.length // Fast path: .length === byte length for ASCII (99%+ of HTML output).
: // For multi-byte chars this slightly underestimates, which is fine
// because byteSize is only used for heuristic decisions (outlining
// threshold and progressive chunk sizing).
chunk.byteLength;
Comment on lines +225 to +229
Copy link
Copy Markdown
Collaborator

@gnoff gnoff Mar 30, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change should have broken something in ReactFlightServer. Suggests that we have some untested serialization path with large string serialization.

Even if the rationale was true though we can't land this change because the layering implies you get back an accurate byteLength. We'd have to rename to getApproximateByteLength etc...

Unless this is like the entirety of the perf win it doesn't seem worth the effort to rewrite the parts that use this length for approximating heuristics so let's just drop it.

If you want to see what would be broken by this check out emitTextChunk in ReactFlightServer

}

export function byteLengthOfBinaryChunk(chunk: BinaryChunk): number {
Expand Down
Loading