Skip to content
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

Bug: stylesheets that use precedence are significantly slower to render at scale #32806

Open
RJWadley opened this issue Apr 2, 2025 · 3 comments
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@RJWadley
Copy link

RJWadley commented Apr 2, 2025

React version: 19.0.0, 19.1.0

Steps To Reproduce

  1. Render at least two precedence levels, for example 'a' and 'b' so that react can insert our stylesheets into the head rather than inline.
  2. Render lots of style elements into 'a' (your first precedence level, the important part is that react inserts our stylesheets into the head) and observe performance. Compare this to rendering without any precedence at all.

Link to code example:

Here's a codesandbox demonstrating the basic issue:
https://codesandbox.io/p/sandbox/react-style-perf-t7w9qr
Or a github repository, if you prefer:
https://github.com/RJWadley/react-style-perf

The current behavior

Seems like each style tag is handled independently with very little optimization - which is probably fine if you only load one or two stylesheets but doesn't work well for larger amounts of styling. In the attached example, you should also notice that every time we toggle our styles with precedence our performance gets worse and worse.

Here's a real example of render performance on my M1 mac pro. This isn't from the attached example, but is the same issue just at a larger scale: Image

Zooming in, most of the performance hit seems to come from query selectors and from actually inserting each stylesheet.
Image

The expected behavior

Stylesheets should ideally render in similar time regardless of precedence.

Related: #30739

@RJWadley RJWadley added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Apr 2, 2025
@eps1lon
Copy link
Collaborator

eps1lon commented Apr 3, 2025

Is this flamegraph from dev or prod? If it's only dev, could you also compare performance in prod?

Could you also include timing comparisons between precedence and no precedence?

@brijeshb42
Copy link

We've (at @mui) have also come to a similar conclusion that the current implementation of of style hoisting (mainly on the client side) is rather slow pertaining to the dom querying that occurs to hoist each style tag.
I was able to improve the perf on the client side with this patch by a factor of 17 in a test of rendering 10000 different components with a style tag having both href and precedence.
React v19 took 3659.1043ms to append the dom items vs 215.73ms with the patch. Note that the patch is fully client side and does not consider hydration or RSC. So I am sure this isn't a proper solution, just something in the right direction.

@RJWadley
Copy link
Author

RJWadley commented Apr 3, 2025

@eps1lon the attached flamegraph is from a production next.js build with mangling disabled.

Some timings:

On A Production Site

The above patch does seem to help. It seems to have some issues with SSR though.
stable react: 4.5s
patched react: 97.88ms

When I remove precedence the site gets much slower, likely due to the massively increased DOM size. Still,
stable react, no precedence: 698.64 ms
patched react, no precedence: 904.13 ms

Within Codesandbox Reproduction

In the attached codesandbox example with the letters, I've observed this with mid-tier mobile CPU throttling:
No precedence: 1.01s
With precedence, first render: 5.65s

Because we're adding more and more to the DOM our performance will decrease over time. For example, when rerunning the letters example with precedence, a second render took 15.31s and our third took 26.88s

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug
Projects
None yet
Development

No branches or pull requests

3 participants