[Fiber] Yield every other frame for Transition/Retry work #31828
+36
−3
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
|
Comparing: 6a4b46c...222a6c9 Critical size changesIncludes critical production bundles, as well as any change greater than 2%:
Significant size changesIncludes any change greater than 0.2%: Expand to show
|
sebmarkbage
commented
Dec 18, 2024
| // is to reduce the framerate of animations to 30 frames per second. | ||
| // For Idle work we yield every 5ms to keep animations going smooth. | ||
| if (workInProgress !== null) { | ||
| const yieldAfter = now() + (nonIdle ? 25 : 5); |
There was a problem hiding this comment.
Since this is now snapshotted deeper, it doesn't account for the overhead in React to get to this point. We could move this outside but also that overhead should ideally be pretty small, fixed and known.
facebook-github-bot
added
CLA Signed
React Core Team
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This flag first moves the
shouldYield()logic into React itself. We need this forpostTaskcompatibility anyway since this logic is no longer a concern of the scheduler. This means that there can also be no globalrequestPaint()that asks for painting earlier. So this is best rolled out withenableAlwaysYieldScheduler(and ideallyenableYieldingBeforePassive) instead ofenableRequestPaint.Once in React we can change the yield timing heuristics. This uses the previous 5ms for Idle work to keep everything responsive while doing background work. However, for Transitions and Retries we have seen that same thread animations (like loading states animating, or constant animations like cool Three.js stuff) can take CPU time away from the Transition that causes moving into new content to slow down. Therefore we only yield every 25ms.
The purpose of this yield is not to avoid the overhead of yielding, which is very low, but rather to intentionally block any frequently occurring other main thread work like animations from starving our work. If we could we could just tell everyone else to throttle their stuff for ideal scheduling but that's not quite realistic. In other words, the purpose of this is to reduce the frame rate of animations to 30 fps and we achieve this by not yielding. We still do yield to allow the animations to not just stall. This seems like a good balance.
The 5ms of Idle is because we don't really need to yield less often since the overhead is low. We keep it low to allow 120 fps animations to run if necessary and our work may not be the only work within a frame so we need to yield early enough to leave enough time left.
Similarly we choose 25ms rather than say 35ms to ensure that we push long enough to guarantee to half the frame rate but low enough that there's plenty of time left for a rAF to power each animation every other frame. It's also low enough that if something else interrupts the work like a new interaction, we can still be responsive to that within 50ms or so. We also need to yield in case there's I/O work that needs to get bounced through the main thread.
This flag is currently off everywhere since we have so many other scheduling flags but that means there's some urgency to roll those out fully so we can test this one. There's also some tests to update since this doesn't go through the Mock scheduler anymore for yields.