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: performance deteriorates when using ReactDOM.createRoot instead of ReactDom.render for virtual-table. #27524

Open
atersolis opened this issue Oct 15, 2023 · 3 comments

Comments

@atersolis
Copy link

While experimenting with react-window, I encountered a performance disparity in my project compared to the smooth user experience demonstrated in the react-window documentation examples. Even after building for production, I noticed white flashes when scrolling quickly in my project, whereas the documentation's examples remained consistently smooth.

After conducting several experiments, I successfully identified the root cause: the performance significantly deteriorates when using ReactDOM.createRoot compared to the now deprecated ReactDOM.render.

Here is a video to illustrate the issue:
https://github.com/facebook/react/assets/7544804/0be9ba6f-47e8-4d61-bb6c-e8fef0787776

This issue is not exclusive to react-window; in fact, I first encountered it while developing a toy version of it using function components and hooks (react-window itself is currently implemented using class components).

React version:
Tried with react/react-dom 18.2.0 and react/react-dom 18.3.0-canary-09fbee89d-20231013

Link to code example:
Here are two CodeSandbox examples of the same scenario with only one distinction:

Additionally, I've created two CodeSandbox using my own simplified version of a virtual table, the performance issue is also noticeable in this scenario:

The current behavior

The performance appears to be worse when using ReactDom.createRoot compared to ReactDom.render

The expected behavior

Using ReactDom.createRoot should give the same performance or better performance than ReactDom.render

@atersolis atersolis added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Oct 15, 2023
@sophiebits
Copy link
Collaborator

I can reproduce that when dragging the scrollbar in your simplified codesandboxes, there is more visual lag for the boxes to appear in the createRoot version, like in your video.

In the createRoot version, I tried replacing the forceRender call with

flushSync(() => {  // imported from react-dom
  forceRender({});
});

which forces React to perform that one update synchronously and that seems to make it behave similarly to the ReactDOM.render call, which you may be able to use as a workaround. (For example, you can use flushSync even once ReactDOM.render is removed – although it may not be fully compatible with new features like the use() Hook.) Note that the state update is performed immediately and cannot be batched with any state updates enqueued after the flushSync call, so in your current setup you need to edit the ref beforehand, and you need to be careful that you don't have other scroll event handlers that might rerender the same grid (or measure layout in an effect) to avoid possible performance issues.

Internally in React, when you trigger an update within a scroll event, the update is assigned the ContinuousEventPriority level, which is mapped to UserBlockingPriority in our scheduler. Unlike some other event types like click, React does not force this to flush immediately, the idea being that continuing to respond to other user interaction (if any occurs) is more important than rerendering based on a scroll event. So React may return control to the browser (vs for a click event it will finish rerendering even if it means causing the page to freeze for a sec). However in my testing the rerender is not coming close to hitting the deadline; I tried adding an effect to Grid that logs the current time for every render and generally one happens every 16ms.

@acdlite Do you have any other insight to share on why this lags or what to do? (Is there a supported way to opt into DiscreteEventPriority from a scroll event?)

@sophiebits sophiebits added Type: Needs Investigation and removed Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug labels Oct 19, 2023
@timwiegand
Copy link

timwiegand commented Nov 6, 2023

I have the same problem. I used the Chrome performance profiler on my own simple sample and zoomed in on a single frame. The results confirm the theory that React is returning control to the browser after SetState is called. The browser paints before the render is scheduled. The end result is that while scrolling, the browser paints using the DOM rendered for the previous frame. When using react-window, the control goes blank when you scroll fast because each frame you've scrolled past all the content react-window rendered for the previous frame.

image

The entire sequence shown here takes less than 5ms. The rest of the frame is idle. I think the browser is coalescing input events until just before the paint is due, then raising the scroll event. At this point anything scheduled for a callback will run after the paint. It doesn't matter how quick the render is, because it doesn't start until after the paint has happened. The same thing will happen for updates triggered using requestAnimationFrame.

Would it be possible to start processing UserBlockingPriority tasks before returning control to the event loop, while still allowing them to be interrupted if it takes more than 5ms? This is in keeping with the philosophy of not blocking renders while allowing fast updates to complete before the browser paints.

@piecyk
Copy link

piecyk commented Apr 17, 2024

bump, any insides how we can improve this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants