-
Notifications
You must be signed in to change notification settings - Fork 48.2k
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
Adjust SuspenseList CPU bound heuristic #17455
Adjust SuspenseList CPU bound heuristic #17455
Conversation
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 1c7a133:
|
Details of bundled changes.Comparing: b64938e...1c7a133 react-test-renderer
react-native-renderer
react-dom
react-art
react-reconciler
ReactDOM: size: 0.0%, gzip: -0.0% Size changes (experimental) |
Details of bundled changes.Comparing: b64938e...1c7a133 react-reconciler
react-dom
react-art
react-test-renderer
react-native-renderer
ReactDOM: size: 🔺+0.1%, gzip: 🔺+0.1% Size changes (stable) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test before landing please
@@ -1114,7 +1114,8 @@ function completeWork( | |||
return null; | |||
} | |||
} else if ( | |||
now() > renderState.tailExpiration && | |||
now() * 2 - renderState.renderingStartTime > |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Inline comment that translates this into English plz
"The time it took to render last row is greater than time until the expiration"
In SuspenseList we switch to rendering fallbacks (or stop rendering further rows in the case of tail="collapsed/hidden") if it takes more than 500ms to render the list. The limit of 500ms is similar to the train model and designed to be short enough to be in the not noticeable range. This works well if each row is small because we time the 500ms range well. However, if we have a few large rows then we're likely to exceed the limit by a lot. E.g. two 480ms rows hits almost a second instead of 500ms. This PR adjusts the heuristic to instead compute whether something has expired based on the render time of the last row. I.e. if we think rendering one more row would exceed the timeout, then we don't attempt. This still works well for small rows and bails earlier for large rows. The expiration is still based on the start of the list rather than the start of the render. It should probably be based on the start of the render but that's a bigger change and needs some thought.
cc31b2b
to
1c7a133
Compare
* Adjust SuspenseList CPU bound heuristic In SuspenseList we switch to rendering fallbacks (or stop rendering further rows in the case of tail="collapsed/hidden") if it takes more than 500ms to render the list. The limit of 500ms is similar to the train model and designed to be short enough to be in the not noticeable range. This works well if each row is small because we time the 500ms range well. However, if we have a few large rows then we're likely to exceed the limit by a lot. E.g. two 480ms rows hits almost a second instead of 500ms. This PR adjusts the heuristic to instead compute whether something has expired based on the render time of the last row. I.e. if we think rendering one more row would exceed the timeout, then we don't attempt. This still works well for small rows and bails earlier for large rows. The expiration is still based on the start of the list rather than the start of the render. It should probably be based on the start of the render but that's a bigger change and needs some thought. * Comment
* Adjust SuspenseList CPU bound heuristic In SuspenseList we switch to rendering fallbacks (or stop rendering further rows in the case of tail="collapsed/hidden") if it takes more than 500ms to render the list. The limit of 500ms is similar to the train model and designed to be short enough to be in the not noticeable range. This works well if each row is small because we time the 500ms range well. However, if we have a few large rows then we're likely to exceed the limit by a lot. E.g. two 480ms rows hits almost a second instead of 500ms. This PR adjusts the heuristic to instead compute whether something has expired based on the render time of the last row. I.e. if we think rendering one more row would exceed the timeout, then we don't attempt. This still works well for small rows and bails earlier for large rows. The expiration is still based on the start of the list rather than the start of the render. It should probably be based on the start of the render but that's a bigger change and needs some thought. * Comment
In SuspenseList we switch to rendering fallbacks (or stop rendering further rows in the case of tail="collapsed/hidden") if it takes more than 500ms to render the list. The limit of 500ms is similar to the train model and designed to be short enough to be in the not noticeable range.
This works well if each row is small because we time the 500ms range well. However, if we have a few large rows then we're likely to exceed the limit by a lot. E.g. two 480ms rows hits almost a second instead of 500ms.
This PR adjusts the heuristic to instead compute whether something has expired based on the render time of the last row. I.e. if we think rendering one more row would exceed the timeout, then we don't attempt.
This still works well for small rows and bails earlier for large rows.
The expiration is still based on the start of the list rather than the start of the render. It should probably be based on the start of the render but that's a bigger change and needs some thought.