emotion-js / emotion

👩‍🎤 CSS-in-JS library designed for high performance style composition
https://emotion.sh/
MIT License
17.41k stars 1.11k forks source link

Potential 5x perf improvement for stylesheet updates by switching to `CSSStyleSheet.replaceSync` #2501

Open Jarred-Sumner opened 2 years ago

Jarred-Sumner commented 2 years ago

The problem

Currently, hot module reloading pages that use Emotion with React is about 8x slower than using CSSStyleSheet.replaceSync to 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 reloaded

with-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.

CleanShot 2021-10-07 at 23 35 22@2x

with-replaceSync.json.gz - this one is using CSSStyleSheet.replaceSync

CleanShot 2021-10-07 at 23 35 11@2x

This is the CSS:

:root {
  --timestamp: "16336741341477";
  --interval: "32";
  --progress-bar: 56.889%;
  --spinner-1-muted: rgb(179, 6, 202);
  --spinner-1-primary: rgb(224, 8, 253);
  --spinner-2-muted: rgb(22, 188, 124);
  --spinner-2-primary: rgb(27, 235, 155);
  --spinner-3-muted: rgb(89, 72, 0);
  --spinner-3-primary: rgb(111, 90, 0);
  --spinner-4-muted: rgb(18, 84, 202);
  --spinner-4-primary: rgb(23, 105, 253);
  --spinner-rotate: 304deg;
}

This is the React component using Emotion:


import { Global } from "@emotion/react";
export function CSSInJSStyles() {
  return (
    <Global
      styles={`
:root {
  --timestamp: "16336721342556";
  --interval: "32";
  --progress-bar: 56.889%;
  --spinner-1-muted: rgb(179, 6, 202);
  --spinner-1-primary: rgb(224, 8, 253);
  --spinner-2-muted: rgb(22, 188, 124);
  --spinner-2-primary: rgb(27, 235, 155);
  --spinner-3-muted: rgb(89, 72, 0);
  --spinner-3-primary: rgb(111, 90, 0);
  --spinner-4-muted: rgb(18, 84, 202);
  --spinner-4-primary: rgb(23, 105, 253);
  --spinner-rotate: 304deg;
}  `}
    />
  );
}

Proposed solution

Detect if CSSStyleSheet.replaceSync is 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:

Alternative solutions

Additional context

replaceSync has some cost, but it's not so bad:

CleanShot 2021-10-07 at 23 58 24@2x

Versus:

CleanShot 2021-10-07 at 23 58 53@2x

Incase opening the profiles are a pain, here are screenshots.

Emotion:

CleanShot 2021-10-08 at 00 09 54@2x

replaceSync:

CleanShot 2021-10-08 at 00 08 16@2x
Andarist commented 2 years ago

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 style elements and only swapping the rules contained in them.

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.

web-padawan commented 2 years ago

FYI there is a polyfill for constructable stylesheets: https://www.npmjs.com/package/construct-style-sheets-polyfill If you need some insight about internal details, feel free to reach out to @Lodin and @calebdwilliams.

Jarred-Sumner commented 2 years ago

I wonder if maybe there are some perf gains to be had by keeping style elements and only swapping the rules contained in them.

This feels like the best impact to effort ratio thing to try

Note that this whole thing only matters for Global styles since those are the only ones that are "replaceable"

What about:

The problem is that our global styles are always "grouped" with our "scoped" styles - if people only would use Emotion-based styles then using this would not be a problem at all. However, people tend to use Emotion in combination with other styles and this would have an impact on which styles are applied in the end when dealing with conflicting rules or rules with the same specificity

I don't have a good answer for this

What if it used many small constructable stylesheets with adoptedStyleSheets (sort of like how CSS Module Scripts appear to work -- it returns a CSSStyleSheet object)? Then, any modifications to dynamic styles (or re-renders that change styles for the component) only update the constructable stylesheet for the flattened css in-place? I don't know what the perf looks like for many small stylesheets versus a couple large ones.

manipulating document.adoptedStyleSheets seems to be really cumbersome - before each manipulation, we'd have to find our style sheet in that and create the appropriate new version of that in an immutable way. Not that big of a problem implementation-wise but it's a little bit quirky that we can't just manipulate our stuff without caring about what other scripts do on the page 😢

I agree, the API for this is cumbersome – here's some code that does part of that ```tsx let 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; } } } ```
Andarist commented 2 years ago

When updating a React component that has css passed through props which have dynamic content (i.e. not flattened by babel)?

The problem is that css styles are never removed - unless you call flush manually. The reasoning behind is that styles are not owned by a component - styles are associated with the styles that they contain. So many different components can use the very same styles without generating many class names. Even if we consider that the same component type could be rendered multiple times on a page this becomes problematic (if we'd like to clean up our styles). To handle this we'd have to implement ref counting but given how React reserves its right to call render multiple times etc this is rather hard to pull off. Maybe with the addition of the useInsertionEffect (that has landed as an experimental thing for the upcoming React 18) this would become more possible. But even with that in place, I'm unsure if it would be worth it - I've seen some discussions that removing styles can actually be costly and that just keeping them around can be a perf win. The part of the idea is that the already injected styles will actually get used again if the user stays in the app and navigate back etc. The only exception to this is the global styles.

In development, when hot reloading a React component?

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.

e111077 commented 2 years ago

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.

nolanlawson commented 1 year ago

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. <style> vs constructable stylesheet), so I'm curious to know where this perf difference is coming from.