[core] Regression test is 5 times slower after React 19 upgrade

[DiegoAndai]

From #44672 (comment)

Tests speed increased by x2.4 https://app.circleci.com/insights/github/mui/material-ui/workflows/pipeline/overview?reporting-window=last-60-days after December 1st.

---

It seems to be this PR: #44672. The time it takes to run the visual regression tests:

• Before: 4m https://app.circleci.com/pipelines/github/mui/material-ui/144367/workflows/26011c08-72cf-4022-a276-119f1e755c3b/jobs/777254
• After: 19m https://app.circleci.com/pipelines/github/mui/material-ui/144370/workflows/d5b7164c-53ba-455f-8bc5-95e9eabc7ea4/jobs/777276

---

Some notes:

• The compile time did not increase, it's ~65000 ms in both
• Return times don't seem to have been changed either

Search keywords:

[DiegoAndai]

@oliviertassinari this looks related (reference):

Average CPU usage down to 25% from 75% in around the same dates. Do you know what could cause this?

[DiegoAndai]

It's definitely a React 19 thing:

In this PR #44868:

• Ran with React 19 in 20 minutes
• Ran with React 18 in 4 minutes

[DiegoAndai]

Looks like it's related to the Suspense changes: #44935

[DiegoAndai]

Hey @eps1lon, what might be going on here? After upgrading to React 19, our regressions tests went from 4 minutes to 20. If I remove the Suspense component from the test (see #44935), the test returns to 4 minutes. The Suspense component was added here: #28239.

I couldn't find much information about the impact of the Suspense React 19 changes on React.lazy, so I'm lost as to what could be the cause or possible solution. The tests also take 20 minutes with React's 19.0.0-rc.0, so this doesn't seem related to the latest changes made to Suspense before the stable release.

If you have any ideas or could point me to a resource that might be related, I would greatly appreciate it.

[eps1lon]

After React displays a fallback, React will throttle the reveal of the Suspense boundary by 300ms to reduce jank (see facebook/react#31819). Should be fixed by wrapping the initial render in React.startTransition.

[DiegoAndai]

Thanks for the response @eps1lon. I don't understand what you meant:

Should be fixed by wrapping the initial render in React.startTransition.

To which initial render are you referring to? Where/how should React.startTransition be used?

[eps1lon]

Where you call root.render() i.e. React.startTransition(() => root.render())

[DiegoAndai]

I tried on

material-ui/test/regressions/index.js

Line 318 in f37ca96

viewerReactRoot.current.render(children);

and

material-ui/test/regressions/index.js

Line 465 in f37ca96

reactRoot.render(children);

Sadly, none worked.

---

It's maybe due to something we're doing regarding this comment?

material-ui/test/regressions/TestViewer.js

Lines 12 to 14 in f37ca96

	// We're simulating `act(() => ReactDOM.render(children))`
	// In the end children passive effects should've been flushed.
	// React doesn't have any such guarantee outside of `act()` so we're approximating it.

---

Also, forstartTransition to have an effect, shouldn't it be used "inside" the Suspense barrier?

[eps1lon]

material-ui/test/regressions/index.js

Line 395 in f37ca96

window.muiFixture.navigate = navigate;

is probably also missing a startTransition. Basically any state update that triggers a Suspense fallback in your test needs a startTransition.

[DiegoAndai]

I also tried adding it there, but it doesn't work 😓

The only thing inside the Suspense barrier are the tests, so adding startTransition to all tests looks like overkill.

At this point, I will rework the test workflow without suspense. I don't think we need the fallback anyway.

Thanks fro the replies, @eps1lon, I appreciate it.

[eps1lon]

At this point, I will rework the test workflow without suspense. I don't think we need the fallback anyway.

It doesn't matter whether you use Suspense. Just that you suspend outside of a Transition. That you need to fix to avoid the throttling mechanism. Without Suspense, React just unmounts the whole app.

so adding startTransition to all tests looks like overkill.

I don't think you need to do this as far as I remember. Don't we trigger the same function between each test? Just make sure that is wrapped in startTransition.

[DiegoAndai]

It doesn't matter whether you use Suspense

Then why, by just removing Suspense, do we get back to the original test speed?: #44935

Don't we trigger the same function between each test?

I don't know what function you're referring to. Sorry, I'm not very familiar with this section of the code 😓.

[Janpot]

Looking a bit into this. It looks runtime related. With a small reproduction:

import * as React from 'react';
import * as ReactDOM from 'react-dom/client';

function Fallback() {
  React.useEffect(() => {
    console.time('fallback');
    return () => {
      console.timeEnd('fallback');
    };
  }, []);
  return <div>fallback</div>;
}

const components = Array.from({ length: 1000 }, (_, i) =>
  React.lazy(async () => ({
    default: function Foo() {
      return <div>foo {i}</div>;
    },
  })),
);

function App() {
  const [counter, setCounter] = React.useState(0);
  const Foo = components[counter % components.length];
  return (
    <div>
      <button onClick={() => setCounter((x) => x + 1)}>click</button>
      <React.Suspense fallback={<Fallback />}>
        <div>hello {counter}</div>
        <Foo />
      </React.Suspense>
    </div>
  );
}

ReactDOM.createRoot(document.getElementById('react-root')).render(<App />);

This consistently prints ~300ms on every click in our webpack setup. Trying the same in vite prints <1ms each time. @eps1lon given that I nowhere start a transition, even though it's not desired for our use-case I suppose webpack is behaving as expected here and vite has a bug? However, it doesn't look like adding transitions does anything to the behavior...

Will probably just port our test setup to vite if it lets me within reasonable time. We can get a few more wins with this setup.

[domarmstrong]

If you wrap setCounter in a startTransition it does indeed completely remove the fallback and the transitions are instant.

But, if you are using some external state, such as useSyncExternalStore (url state for example), startTransition doesn't seem to have any impact:

import * as React from 'react';
import * as ReactDOM from 'react-dom/client';

function Fallback() {
  React.useEffect(() => {
    console.time('fallback');
    return () => {
      console.timeEnd('fallback');
    };
  }, []);
  return <div>fallback</div>;
}

const components = Array.from({ length: 1000 }, (_, i) =>
  React.lazy(async () => ({
    default: function Foo() {
      return <div>foo {i}</div>;
    },
  })),
);

const store = {
  counter: 0,
  incrementCounter() {
    this.counter = this.counter + 1;
    this._subscribers.forEach((callback) => callback());
  },

  _subscribers: new Set(),
  subscribe(callback) {
    store._subscribers.add(callback);
    return () => store._subscribers.delete(callback);
  }
};

function useCounter() {
  return React.useSyncExternalStore(store.subscribe, () => store.counter);
}

export function App() {
  const counter = useCounter();
  const Foo = components[counter % components.length];
  return (
    <div>
      <button onClick={() => React.startTransition(() => store.incrementCounter())}>click</button>
      <React.Suspense fallback={<Fallback />}>
        <div>hello {counter}</div>
        <Foo />
      </React.Suspense>
    </div>
  );
}

[Janpot]

If you wrap setCounter in a startTransition it does indeed completely remove the fallback and the transitions are instant.

Transitions are, but initial render stays >300ms. Even when wrapping root.render with React.startTransition().

[github-actions]

This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue.
Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

[DiegoAndai]

We're back to pre-React 19 times 👍🏼