-
Notifications
You must be signed in to change notification settings - Fork 45.7k
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
[Fiber] clarify entry condition for suspensey commit recursion #29222
[Fiber] clarify entry condition for suspensey commit recursion #29222
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
a409ff2
to
30d249b
Compare
Comparing: 3ac551e...09f87eb 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
|
@@ -1185,7 +1185,7 @@ function commitRootWhenReady( | |||
) { | |||
// TODO: Combine retry throttling with Suspensey commits. Right now they run | |||
// one after the other. | |||
if (finishedWork.subtreeFlags & ShouldSuspendCommit) { | |||
if (finishedWork.subtreeFlags & (ShouldSuspendCommit | Visibility)) { |
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.
To make this even more specific, if ShouldSuspendCommit is not present, it should check if both Visibility and MaySuspendCommit are set. If there's no MaySuspendCommit flag it can bail out.
So something like:
const BothShouldSuspendCommitAndVisibility = ShouldSuspendCommit | Visibility;
if (
subtreeFlags & SoundSuspendCommit ||
((subtreeFlags & BothShouldSuspendCommitAndVisibility) === BothShouldSuspendCommitAndVisibility)
Previously Suspensey recursion would only trigger if the ShouldSuspendCommit flag was true. However there is a dependence on the Visibility flag embedded in this logic because these flags share a bit. To make it clear that the semantics of Suspensey resources require considering both flags I've added it to the condition even though this extra or-ing is a noop when the bit is shared
30d249b
to
09f87eb
Compare
Previously Suspensey recursion would only trigger if the ShouldSuspendCommit flag was true. However there is a dependence on the Visibility flag embedded in this logic because these flags share a bit. To make it clear that the semantics of Suspensey resources require considering both flags I've added it to the condition even though this extra or-ing is a noop when the bit is shared DiffTrain build for commit f55d172.
Previously Suspensey recursion would only trigger if the ShouldSuspendCommit flag was true. However there is a dependence on the Visibility flag embedded in this logic because these flags share a bit. To make it clear that the semantics of Suspensey resources require considering both flags I've added it to the condition even though this extra or-ing is a noop when the bit is shared DiffTrain build for [f55d172](f55d172)
Previously Suspensey recursion would only trigger if the ShouldSuspendCommit flag was true. However there is a dependence on the Visibility flag embedded in this logic because these flags share a bit. To make it clear that the semantics of Suspensey resources require considering both flags I've added it to the condition even though this extra or-ing is a noop when the bit is shared