[Fiber] Fix initial mount starvation problems#7448
[Fiber] Fix initial mount starvation problems#7448sebmarkbage wants to merge 1 commit intofacebook:masterfrom
Conversation
The priority level gets reset at the wrong time because I rely on mutating it at the wrong point. This moves it to the end of completed work as a second pass over the children to see what the highest priority is. This is inefficient for large sets but we can try to find a smarter way to do this later. E.g. passing it down the stack. This bug fix revealed another bug that I had flagged before that we're finding work to do in the "current" tree instead of the working tree. For trees that were paused, e.g. childInProgress trees, this won't work since don't have a current tree to search. Therefore I fixed findNextUnitOfWorkAtPriority to use workInProgress instead of current.
ghost
added
the
| // Don't bother drilling further down this tree if there is no child | ||
| // with more content. | ||
| // TODO: Shouldn't this still drill down even though the first | ||
| // shallow level doesn't have anything pending on it. |
There was a problem hiding this comment.
This todo is correct, I think. Once you split the priority fields to distinguish between the fiber's priority and the subtree's priority, there could be a matching update further down the tree. Now that this function operates on the work in progress tree, could you do a recursive call?
| alt.pendingProps = fiber.pendingProps; | ||
| alt.pendingWorkPriority = priorityLevel; | ||
|
|
||
| alt.memoizedProps = fiber.memoizedProps; |
There was a problem hiding this comment.
This turns out to be a bad idea, because it throws away progressed work all the time on the way up. This is what is causing the triangle demo to starve at slower speeds now. Needs a unit test to cover this case. However, not resetting this but resetting the child is also not correct and leads to incorrect results.
sebmarkbage
added
GH Review: needs-revision
and removed
GH Review: review-needed
labels
|
Going to abandon this strategy for now. Will try a different approach. |
3 participants
The priority level gets reset at the wrong time because I rely
on mutating it at the wrong point. This moves it to the end of
completed work as a second pass over the children to see what
the highest priority is. This is inefficient for large sets but
we can try to find a smarter way to do this later. E.g. passing
it down the stack.
This bug fix revealed another bug that I had flagged before that
we're finding work to do in the "current" tree instead of the
working tree. For trees that were paused, e.g. childInProgress
trees, this won't work since don't have a current tree to search.
Therefore I fixed findNextUnitOfWorkAtPriority to use
workInProgress instead of current.