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.
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
Potential 5x perf improvement for stylesheet updates by switching to CSSStyleSheet.replaceSync
#2501
Comments
|
Thank you for the issue - TIL about constructable stylesheets (I knew there was such a thing for Shadow DOM but never occurred to me that I could use it for the document itself). Some additional problems that I'm seeing with:
I wonder if maybe there are some perf gains to be had by keeping Note that this whole thing only matters for Global styles since those are the only ones that are "replaceable". I wonder if there are really any real-world perf gains here or if this is just more of a theoretical problem when looking at this with the perspective of a stress test. |
|
FYI there is a polyfill for constructable stylesheets: https://www.npmjs.com/package/construct-style-sheets-polyfill |
This feels like the best impact to effort ratio thing to try
What about:
I don't have a good answer for this What if it used many small constructable stylesheets with
I agree, the API for this is cumbersome – here's some code that does part of thatlet count = 0;
let match: CSSHMRInsertionPoint = null;
const adoptedStyles = document.adoptedStyleSheets;
if (this.updateMethod === CSSUpdateMethod.cssObjectModel) {
if (adoptedStyles.length > 0) {
count = adoptedStyles.length;
for (let i = 0; i < count && match === null; i++) {
let cssRules: CSSRuleList;
let sheet: CSSStyleSheet;
let ruleCount = 0;
// Non-same origin stylesheets will potentially throw "Security error"
// We will ignore those stylesheets and look at others.
try {
sheet = adoptedStyles[i];
cssRules = sheet.rules;
ruleCount = sheet.rules.length;
} catch (exception) {
continue;
}
if (sheet.disabled || sheet.rules.length === 0) {
continue;
}
const bundleIdRule = cssRules[0] as CSSSupportsRule;
if (
bundleIdRule.type !== 12 ||
!bundleIdRule.conditionText.startsWith("(hmr-bid:")
) {
continue;
}
const bundleIdEnd = bundleIdRule.conditionText.indexOf(
")",
"(hmr-bid:".length + 1
);
if (bundleIdEnd === -1) continue;
CSSLoader.cssLoadId.bundle_id = parseInt(
bundleIdRule.conditionText.substring("(hmr-bid:".length, bundleIdEnd),
10
);
for (let j = 1; j < ruleCount && match === null; j++) {
match = this.findMatchingSupportsRule(
cssRules[j] as CSSSupportsRule,
id,
sheet
);
}
}
}
}
// later
findMatchingSupportsRule(
rule: CSSSupportsRule,
id: number,
sheet: CSSStyleSheet
): CSSHMRInsertionPoint | null {
switch (rule.type) {
// 12 is result.SUPPORTS_RULE
case 12: {
if (!rule.conditionText.startsWith("(hmr-wid:")) {
return null;
}
const startIndex = "hmr-wid:".length + 1;
const endIDRegion = rule.conditionText.indexOf(")", startIndex);
if (endIDRegion === -1) return null;
const int = parseInt(
rule.conditionText.substring(startIndex, endIDRegion),
10
);
if (int !== id) {
return null;
}
let startFileRegion = rule.conditionText.indexOf(
'(hmr-file:"',
endIDRegion
);
if (startFileRegion === -1) return null;
startFileRegion += '(hmr-file:"'.length + 1;
const endFileRegion = rule.conditionText.indexOf(
'"',
startFileRegion
);
if (endFileRegion === -1) return null;
// Empty file strings are invalid
if (endFileRegion - startFileRegion <= 0) return null;
CSSLoader.cssLoadId.id = int;
CSSLoader.cssLoadId.node = sheet.ownerNode as HTMLStylableElement;
CSSLoader.cssLoadId.sheet = sheet;
CSSLoader.cssLoadId.file = rule.conditionText.substring(
startFileRegion - 1,
endFileRegion
);
return CSSLoader.cssLoadId;
}
default: {
return null;
}
}
} |
The problem is that
That's something I'm not afraid of doing but I'd like to preserve the behavior - so people don't experience different outcomes between production and development builds. |
|
btw FWIW on the original issue about manipulating adoptedStylesheets: the implementation agreed upon across browsers (and shipped in chrome 99) is no longer a FrozenArray but rather an ObservableArray so immutability is not an issue anymore. |
|
Sorry for commenting on an old issue, but is the benchmark source code available? I've not yet seen a case where style calculation costs can be affected by the choice of stylesheet format (e.g. |
The problem
Currently, hot module reloading pages that use Emotion with React is about 8x slower than using
CSSStyleSheet.replaceSyncto update styles.Here are two Chrome profiles where the same CSS is updated on disk 1024 times, sleeping for 32ms between each update. In the first case, it's a
<Global>React component, and in the second case, it's a<link>tag being hot reloadedwith-emotion-react.json.gz - this one is using Emotion and the React Component is being re-imported each time. Note the difference in time spent on Rendering between the two profiles.
with-replaceSync.json.gz - this one is using
CSSStyleSheet.replaceSyncThis is the CSS:
This is the React component using Emotion:
Proposed solution
Detect if
CSSStyleSheet.replaceSyncis supported in the current browser and use that to update the existing stylesheet (rather than creating a new one). This would work for both development and production (in production for dynamic styles).Drawbacks:
@importis not supported withCSSStyleSheet.replaceSyncAlternative solutions
Additional context
replaceSynchas some cost, but it's not so bad:Versus:
Incase opening the profiles are a pain, here are screenshots.
Emotion:

replaceSync:

The text was updated successfully, but these errors were encountered: