From a7899392189f52f5cf464404457af87d1950d9f0 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Tue, 8 Sep 2020 10:38:39 -0400 Subject: [PATCH 1/4] Moved resetChildLanes into complete work This enabled us to remove a few hot path tag-type checks, but did not otherwise change any functionality. --- .../src/ReactFiberCompleteWork.new.js | 199 +++++++++++++++++- .../src/ReactFiberWorkLoop.new.js | 152 +------------ 2 files changed, 195 insertions(+), 156 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js index 3cb08b48eea5..25226a6561d1 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js @@ -8,7 +8,7 @@ */ import type {Fiber} from './ReactInternalTypes'; -import type {Lanes} from './ReactFiberLane'; +import type {Lanes, Lane} from './ReactFiberLane'; import type { ReactFundamentalComponentInstance, ReactScopeInstance, @@ -58,7 +58,12 @@ import { OffscreenComponent, LegacyHiddenComponent, } from './ReactWorkTags'; -import {NoMode, BlockingMode, ProfileMode} from './ReactTypeOfMode'; +import { + NoMode, + BlockingMode, + ConcurrentMode, + ProfileMode, +} from './ReactTypeOfMode'; import { Ref, Update, @@ -66,6 +71,7 @@ import { DidCapture, Snapshot, MutationMask, + StaticMask, } from './ReactFiberFlags'; import invariant from 'shared/invariant'; @@ -137,9 +143,16 @@ import { renderHasNotSuspendedYet, popRenderLanes, getRenderTargetTime, + subtreeRenderLanes, } from './ReactFiberWorkLoop.new'; import {createFundamentalStateInstance} from './ReactFiberFundamental.new'; -import {OffscreenLane, SomeRetryLane} from './ReactFiberLane'; +import { + OffscreenLane, + SomeRetryLane, + NoLanes, + includesSomeLane, + mergeLanes, +} from './ReactFiberLane'; import {resetChildFibers} from './ReactChildFiber.new'; import {createScopeInstance} from './ReactFiberScope.new'; import {transferActualDuration} from './ReactProfilerTimer.new'; @@ -668,6 +681,114 @@ function cutOffTailIfNeeded( } } +function bubbleProperties(completedWork: Fiber) { + const didBailout = + completedWork.alternate !== null && + completedWork.alternate.child === completedWork.child; + + let newChildLanes = NoLanes; + let subtreeFlags = NoFlags; + + if (!didBailout) { + // Bubble up the earliest expiration time. + if (enableProfilerTimer && (completedWork.mode & ProfileMode) !== NoMode) { + // In profiling mode, resetChildExpirationTime is also used to reset + // profiler durations. + let actualDuration = completedWork.actualDuration; + let treeBaseDuration = ((completedWork.selfBaseDuration: any): number); + + let child = completedWork.child; + while (child !== null) { + newChildLanes = mergeLanes( + newChildLanes, + mergeLanes(child.lanes, child.childLanes), + ); + + subtreeFlags |= child.subtreeFlags; + subtreeFlags |= child.flags; + + // When a fiber is cloned, its actualDuration is reset to 0. This value will + // only be updated if work is done on the fiber (i.e. it doesn't bailout). + // When work is done, it should bubble to the parent's actualDuration. If + // the fiber has not been cloned though, (meaning no work was done), then + // this value will reflect the amount of time spent working on a previous + // render. In that case it should not bubble. We determine whether it was + // cloned by comparing the child pointer. + actualDuration += child.actualDuration; + + treeBaseDuration += child.treeBaseDuration; + child = child.sibling; + } + + completedWork.actualDuration = actualDuration; + completedWork.treeBaseDuration = treeBaseDuration; + } else { + let child = completedWork.child; + while (child !== null) { + newChildLanes = mergeLanes( + newChildLanes, + mergeLanes(child.lanes, child.childLanes), + ); + + subtreeFlags |= child.subtreeFlags; + subtreeFlags |= child.flags; + + child = child.sibling; + } + } + + completedWork.subtreeFlags |= subtreeFlags; + } else { + // Bubble up the earliest expiration time. + if (enableProfilerTimer && (completedWork.mode & ProfileMode) !== NoMode) { + // In profiling mode, resetChildExpirationTime is also used to reset + // profiler durations. + let treeBaseDuration = ((completedWork.selfBaseDuration: any): number); + + let child = completedWork.child; + while (child !== null) { + newChildLanes = mergeLanes( + newChildLanes, + mergeLanes(child.lanes, child.childLanes), + ); + + // "Static" flags share the lifetime of the fiber/hook they belong to, + // so we should bubble those up even during a bailout. All the other + // flags have a lifetime only of a single render + commit, so we should + // ignore them. + subtreeFlags |= child.subtreeFlags & StaticMask; + subtreeFlags |= child.flags & StaticMask; + + treeBaseDuration += child.treeBaseDuration; + child = child.sibling; + } + + completedWork.treeBaseDuration = treeBaseDuration; + } else { + let child = completedWork.child; + while (child !== null) { + newChildLanes = mergeLanes( + newChildLanes, + mergeLanes(child.lanes, child.childLanes), + ); + + // "Static" flags share the lifetime of the fiber/hook they belong to, + // so we should bubble those up even during a bailout. All the other + // flags have a lifetime only of a single render + commit, so we should + // ignore them. + subtreeFlags |= child.subtreeFlags & StaticMask; + subtreeFlags |= child.flags & StaticMask; + + child = child.sibling; + } + } + + completedWork.subtreeFlags |= subtreeFlags; + } + + completedWork.childLanes = newChildLanes; +} + function completeWork( current: Fiber | null, workInProgress: Fiber, @@ -686,12 +807,14 @@ function completeWork( case Profiler: case ContextConsumer: case MemoComponent: + bubbleProperties(workInProgress); return null; case ClassComponent: { const Component = workInProgress.type; if (isLegacyContextProvider(Component)) { popLegacyContext(workInProgress); } + bubbleProperties(workInProgress); return null; } case HostRoot: { @@ -720,6 +843,7 @@ function completeWork( } } updateHostContainer(current, workInProgress); + bubbleProperties(workInProgress); return null; } case HostComponent: { @@ -746,6 +870,7 @@ function completeWork( 'caused by a bug in React. Please file an issue.', ); // This can happen when we abort work. + bubbleProperties(workInProgress); return null; } @@ -803,6 +928,7 @@ function completeWork( markRef(workInProgress); } } + bubbleProperties(workInProgress); return null; } case HostText: { @@ -837,6 +963,7 @@ function completeWork( ); } } + bubbleProperties(workInProgress); return null; } case SuspenseComponent: { @@ -856,6 +983,20 @@ function completeWork( if (enableSchedulerTracing) { markSpawnedWork(OffscreenLane); } + bubbleProperties(workInProgress); + if (enableProfilerTimer) { + if ((workInProgress.mode & ProfileMode) !== NoMode) { + const isTimedOutSuspense = nextState !== null; + if (isTimedOutSuspense) { + // Don't count time spent in a timed out Suspense subtree as part of the base duration. + const primaryChildFragment = workInProgress.child; + if (primaryChildFragment !== null) { + // $FlowFixMe Flow doens't support type casting in combiation with the -= operator + workInProgress.treeBaseDuration -= ((primaryChildFragment.treeBaseDuration: any): number); + } + } + } + } return null; } else { // We should never have been in a hydration state if we didn't have a current. @@ -872,6 +1013,20 @@ function completeWork( // If something suspended, schedule an effect to attach retry listeners. // So we might as well always mark this. workInProgress.flags |= Update; + bubbleProperties(workInProgress); + if (enableProfilerTimer) { + if ((workInProgress.mode & ProfileMode) !== NoMode) { + const isTimedOutSuspense = nextState !== null; + if (isTimedOutSuspense) { + // Don't count time spent in a timed out Suspense subtree as part of the base duration. + const primaryChildFragment = workInProgress.child; + if (primaryChildFragment !== null) { + // $FlowFixMe Flow doens't support type casting in combiation with the -= operator + workInProgress.treeBaseDuration -= ((primaryChildFragment.treeBaseDuration: any): number); + } + } + } + } return null; } } @@ -964,6 +1119,19 @@ function completeWork( // Always notify the callback workInProgress.flags |= Update; } + bubbleProperties(workInProgress); + if (enableProfilerTimer) { + if ((workInProgress.mode & ProfileMode) !== NoMode) { + if (nextDidTimeout) { + // Don't count time spent in a timed out Suspense subtree as part of the base duration. + const primaryChildFragment = workInProgress.child; + if (primaryChildFragment !== null) { + // $FlowFixMe Flow doens't support type casting in combiation with the -= operator + workInProgress.treeBaseDuration -= ((primaryChildFragment.treeBaseDuration: any): number); + } + } + } + } return null; } case HostPortal: @@ -972,10 +1140,12 @@ function completeWork( if (current === null) { preparePortalMount(workInProgress.stateNode.containerInfo); } + bubbleProperties(workInProgress); return null; case ContextProvider: // Pop provider fiber popProvider(workInProgress); + bubbleProperties(workInProgress); return null; case IncompleteClassComponent: { // Same as class component case. I put it down here so that the tags are @@ -984,6 +1154,7 @@ function completeWork( if (isLegacyContextProvider(Component)) { popLegacyContext(workInProgress); } + bubbleProperties(workInProgress); return null; } case SuspenseListComponent: { @@ -995,6 +1166,7 @@ function completeWork( if (renderState === null) { // We're running in the default, "independent" mode. // We don't do anything in this mode. + bubbleProperties(workInProgress); return null; } @@ -1117,6 +1289,7 @@ function completeWork( !getIsHydrating() // We don't cut it if we're hydrating. ) { // We're done. + bubbleProperties(workInProgress); return null; } } else if ( @@ -1190,6 +1363,7 @@ function completeWork( // Do a pass over the next row. return next; } + bubbleProperties(workInProgress); return null; } case FundamentalComponent: { @@ -1217,6 +1391,7 @@ function completeWork( ): any): Instance); fundamentalInstance.instance = instance; if (fundamentalImpl.reconcileChildren === false) { + bubbleProperties(workInProgress); return null; } appendAllChildren(instance, workInProgress, false, false); @@ -1239,6 +1414,7 @@ function completeWork( markUpdate(workInProgress); } } + bubbleProperties(workInProgress); return null; } break; @@ -1261,24 +1437,27 @@ function completeWork( markRef(workInProgress); } } + bubbleProperties(workInProgress); return null; } break; } case Block: if (enableBlocksAPI) { + bubbleProperties(workInProgress); return null; } break; case OffscreenComponent: case LegacyHiddenComponent: { popRenderLanes(workInProgress); + const nextState: OffscreenState | null = workInProgress.memoizedState; + const nextIsHidden = nextState !== null; + if (current !== null) { - const nextState: OffscreenState | null = workInProgress.memoizedState; const prevState: OffscreenState | null = current.memoizedState; const prevIsHidden = prevState !== null; - const nextIsHidden = nextState !== null; if ( prevIsHidden !== nextIsHidden && newProps.mode !== 'unstable-defer-without-hiding' @@ -1286,6 +1465,16 @@ function completeWork( workInProgress.flags |= Update; } } + + // Don't bubble properties for hidden children. + if ( + !nextIsHidden || + includesSomeLane(subtreeRenderLanes, (OffscreenLane: Lane)) || + (workInProgress.mode & ConcurrentMode) === NoLanes + ) { + bubbleProperties(workInProgress); + } + return null; } } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 1e386c330954..8471fa13865e 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -115,8 +115,6 @@ import { MemoComponent, SimpleMemoComponent, Block, - OffscreenComponent, - LegacyHiddenComponent, ScopeComponent, } from './ReactWorkTags'; import {LegacyRoot} from './ReactRootTags'; @@ -139,7 +137,6 @@ import { MutationMask, LayoutMask, PassiveMask, - StaticMask, } from './ReactFiberFlags'; import { NoLanePriority, @@ -151,7 +148,6 @@ import { NoLane, SyncLane, SyncBatchedLane, - OffscreenLane, NoTimestamp, findUpdateLane, findTransitionLane, @@ -290,7 +286,7 @@ let workInProgressRootRenderLanes: Lanes = NoLanes; // // Most things in the work loop should deal with workInProgressRootRenderLanes. // Most things in begin/complete phases should deal with subtreeRenderLanes. -let subtreeRenderLanes: Lanes = NoLanes; +export let subtreeRenderLanes: Lanes = NoLanes; const subtreeRenderLanesCursor: StackCursor = createCursor(NoLanes); // Whether to root completed, errored, suspended, etc. @@ -1719,8 +1715,6 @@ function completeUnitOfWork(unitOfWork: Fiber): void { workInProgress = next; return; } - - resetChildLanes(completedWork); } else { // This fiber did not complete because something threw. Pop values off // the stack without entering the complete phase. If this is a boundary, @@ -1782,150 +1776,6 @@ function completeUnitOfWork(unitOfWork: Fiber): void { } } -function resetChildLanes(completedWork: Fiber) { - if ( - // TODO: Move this check out of the hot path by moving `resetChildLanes` - // to switch statement in `completeWork`. - (completedWork.tag === LegacyHiddenComponent || - completedWork.tag === OffscreenComponent) && - completedWork.memoizedState !== null && - !includesSomeLane(subtreeRenderLanes, (OffscreenLane: Lane)) && - (completedWork.mode & ConcurrentMode) !== NoLanes - ) { - // The children of this component are hidden. Don't bubble their - // expiration times. - return; - } - - const didBailout = - completedWork.alternate !== null && - completedWork.alternate.child === completedWork.child; - - let newChildLanes = NoLanes; - let subtreeFlags = NoFlags; - - if (!didBailout) { - // Bubble up the earliest expiration time. - if (enableProfilerTimer && (completedWork.mode & ProfileMode) !== NoMode) { - // In profiling mode, resetChildExpirationTime is also used to reset - // profiler durations. - let actualDuration = completedWork.actualDuration; - let treeBaseDuration = ((completedWork.selfBaseDuration: any): number); - - let child = completedWork.child; - while (child !== null) { - newChildLanes = mergeLanes( - newChildLanes, - mergeLanes(child.lanes, child.childLanes), - ); - - subtreeFlags |= child.subtreeFlags; - subtreeFlags |= child.flags; - - // When a fiber is cloned, its actualDuration is reset to 0. This value will - // only be updated if work is done on the fiber (i.e. it doesn't bailout). - // When work is done, it should bubble to the parent's actualDuration. If - // the fiber has not been cloned though, (meaning no work was done), then - // this value will reflect the amount of time spent working on a previous - // render. In that case it should not bubble. We determine whether it was - // cloned by comparing the child pointer. - actualDuration += child.actualDuration; - - treeBaseDuration += child.treeBaseDuration; - child = child.sibling; - } - - const isTimedOutSuspense = - completedWork.tag === SuspenseComponent && - completedWork.memoizedState !== null; - if (isTimedOutSuspense) { - // Don't count time spent in a timed out Suspense subtree as part of the base duration. - const primaryChildFragment = completedWork.child; - if (primaryChildFragment !== null) { - treeBaseDuration -= ((primaryChildFragment.treeBaseDuration: any): number); - } - } - - completedWork.actualDuration = actualDuration; - completedWork.treeBaseDuration = treeBaseDuration; - } else { - let child = completedWork.child; - while (child !== null) { - newChildLanes = mergeLanes( - newChildLanes, - mergeLanes(child.lanes, child.childLanes), - ); - - subtreeFlags |= child.subtreeFlags; - subtreeFlags |= child.flags; - - child = child.sibling; - } - } - - completedWork.subtreeFlags |= subtreeFlags; - } else { - // Bubble up the earliest expiration time. - if (enableProfilerTimer && (completedWork.mode & ProfileMode) !== NoMode) { - // In profiling mode, resetChildExpirationTime is also used to reset - // profiler durations. - let treeBaseDuration = ((completedWork.selfBaseDuration: any): number); - - let child = completedWork.child; - while (child !== null) { - newChildLanes = mergeLanes( - newChildLanes, - mergeLanes(child.lanes, child.childLanes), - ); - - // "Static" flags share the lifetime of the fiber/hook they belong to, - // so we should bubble those up even during a bailout. All the other - // flags have a lifetime only of a single render + commit, so we should - // ignore them. - subtreeFlags |= child.subtreeFlags & StaticMask; - subtreeFlags |= child.flags & StaticMask; - - treeBaseDuration += child.treeBaseDuration; - child = child.sibling; - } - - const isTimedOutSuspense = - completedWork.tag === SuspenseComponent && - completedWork.memoizedState !== null; - if (isTimedOutSuspense) { - // Don't count time spent in a timed out Suspense subtree as part of the base duration. - const primaryChildFragment = completedWork.child; - if (primaryChildFragment !== null) { - treeBaseDuration -= ((primaryChildFragment.treeBaseDuration: any): number); - } - } - - completedWork.treeBaseDuration = treeBaseDuration; - } else { - let child = completedWork.child; - while (child !== null) { - newChildLanes = mergeLanes( - newChildLanes, - mergeLanes(child.lanes, child.childLanes), - ); - - // "Static" flags share the lifetime of the fiber/hook they belong to, - // so we should bubble those up even during a bailout. All the other - // flags have a lifetime only of a single render + commit, so we should - // ignore them. - subtreeFlags |= child.subtreeFlags & StaticMask; - subtreeFlags |= child.flags & StaticMask; - - child = child.sibling; - } - } - - completedWork.subtreeFlags |= subtreeFlags; - } - - completedWork.childLanes = newChildLanes; -} - function commitRoot(root) { const renderPriorityLevel = getCurrentPriorityLevel(); runWithPriority( From 0247a8a4e1b36b1a6361e6ac5ac0bb13e2fa977e Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 9 Sep 2020 11:44:38 -0400 Subject: [PATCH 2/4] Temporarily split lanes, flags, and profiler durations into separate bubble methods so I can refactor each in isolation --- .../src/ReactFiberCompleteWork.new.js | 125 ++++++++---------- 1 file changed, 55 insertions(+), 70 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js index 25226a6561d1..d5d328a2b31a 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js @@ -681,32 +681,72 @@ function cutOffTailIfNeeded( } } -function bubbleProperties(completedWork: Fiber) { +function bubbleProperties(completedWork: Fiber): void { + bubbleProfilerDurationsFromChildren(completedWork); + bubbleLanesFromChildren(completedWork); + bubbleFlagsFromChildren(completedWork); +} + +// TODO (effects) Temorary method; merge this into bubbleProperties once refactor is done. +function bubbleFlagsFromChildren(completedWork: Fiber): void { const didBailout = completedWork.alternate !== null && completedWork.alternate.child === completedWork.child; - let newChildLanes = NoLanes; let subtreeFlags = NoFlags; if (!didBailout) { - // Bubble up the earliest expiration time. - if (enableProfilerTimer && (completedWork.mode & ProfileMode) !== NoMode) { - // In profiling mode, resetChildExpirationTime is also used to reset - // profiler durations. + let child = completedWork.child; + while (child !== null) { + subtreeFlags |= child.subtreeFlags; + subtreeFlags |= child.flags; + + child = child.sibling; + } + } else { + let child = completedWork.child; + while (child !== null) { + // "Static" flags share the lifetime of the fiber/hook they belong to, + // so we should bubble those up even during a bailout. All the other + // flags have a lifetime only of a single render + commit, so we should + // ignore them. + subtreeFlags |= child.subtreeFlags & StaticMask; + subtreeFlags |= child.flags & StaticMask; + + child = child.sibling; + } + } + + completedWork.subtreeFlags |= subtreeFlags; +} + +// TODO (effects) Temorary method; merge this into bubbleProperties once refactor is done. +function bubbleLanesFromChildren(completedWork: Fiber): void { + let newChildLanes = NoLanes; + let child = completedWork.child; + while (child !== null) { + newChildLanes = mergeLanes( + newChildLanes, + mergeLanes(child.lanes, child.childLanes), + ); + + child = child.sibling; + } + + completedWork.childLanes = newChildLanes; +} + +// TODO (effects) Temorary method; merge this into bubbleProperties once refactor is done. +function bubbleProfilerDurationsFromChildren(completedWork: Fiber): void { + if (enableProfilerTimer && (completedWork.mode & ProfileMode) !== NoMode) { + const didBailout = + completedWork.alternate !== null && + completedWork.alternate.child === completedWork.child; + if (!didBailout) { let actualDuration = completedWork.actualDuration; let treeBaseDuration = ((completedWork.selfBaseDuration: any): number); - let child = completedWork.child; while (child !== null) { - newChildLanes = mergeLanes( - newChildLanes, - mergeLanes(child.lanes, child.childLanes), - ); - - subtreeFlags |= child.subtreeFlags; - subtreeFlags |= child.flags; - // When a fiber is cloned, its actualDuration is reset to 0. This value will // only be updated if work is done on the fiber (i.e. it doesn't bailout). // When work is done, it should bubble to the parent's actualDuration. If @@ -715,7 +755,6 @@ function bubbleProperties(completedWork: Fiber) { // render. In that case it should not bubble. We determine whether it was // cloned by comparing the child pointer. actualDuration += child.actualDuration; - treeBaseDuration += child.treeBaseDuration; child = child.sibling; } @@ -723,70 +762,16 @@ function bubbleProperties(completedWork: Fiber) { completedWork.actualDuration = actualDuration; completedWork.treeBaseDuration = treeBaseDuration; } else { - let child = completedWork.child; - while (child !== null) { - newChildLanes = mergeLanes( - newChildLanes, - mergeLanes(child.lanes, child.childLanes), - ); - - subtreeFlags |= child.subtreeFlags; - subtreeFlags |= child.flags; - - child = child.sibling; - } - } - - completedWork.subtreeFlags |= subtreeFlags; - } else { - // Bubble up the earliest expiration time. - if (enableProfilerTimer && (completedWork.mode & ProfileMode) !== NoMode) { - // In profiling mode, resetChildExpirationTime is also used to reset - // profiler durations. let treeBaseDuration = ((completedWork.selfBaseDuration: any): number); - let child = completedWork.child; while (child !== null) { - newChildLanes = mergeLanes( - newChildLanes, - mergeLanes(child.lanes, child.childLanes), - ); - - // "Static" flags share the lifetime of the fiber/hook they belong to, - // so we should bubble those up even during a bailout. All the other - // flags have a lifetime only of a single render + commit, so we should - // ignore them. - subtreeFlags |= child.subtreeFlags & StaticMask; - subtreeFlags |= child.flags & StaticMask; - treeBaseDuration += child.treeBaseDuration; child = child.sibling; } completedWork.treeBaseDuration = treeBaseDuration; - } else { - let child = completedWork.child; - while (child !== null) { - newChildLanes = mergeLanes( - newChildLanes, - mergeLanes(child.lanes, child.childLanes), - ); - - // "Static" flags share the lifetime of the fiber/hook they belong to, - // so we should bubble those up even during a bailout. All the other - // flags have a lifetime only of a single render + commit, so we should - // ignore them. - subtreeFlags |= child.subtreeFlags & StaticMask; - subtreeFlags |= child.flags & StaticMask; - - child = child.sibling; - } } - - completedWork.subtreeFlags |= subtreeFlags; } - - completedWork.childLanes = newChildLanes; } function completeWork( From 2b767a438881029a02e826880552e62d35131193 Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Wed, 9 Sep 2020 14:29:09 -0400 Subject: [PATCH 3/4] Refactored flag bubbling to be from child to parent --- .../react-reconciler/src/ReactFiber.new.js | 2 +- .../src/ReactFiberCompleteWork.new.js | 45 +++++++------------ .../src/ReactFiberWorkLoop.new.js | 3 +- 3 files changed, 19 insertions(+), 31 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiber.new.js b/packages/react-reconciler/src/ReactFiber.new.js index eca759ceaba6..5d005c2017a3 100644 --- a/packages/react-reconciler/src/ReactFiber.new.js +++ b/packages/react-reconciler/src/ReactFiber.new.js @@ -283,7 +283,6 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber { workInProgress.type = current.type; // We already have an alternate. - workInProgress.subtreeFlags = NoFlags; workInProgress.deletions = null; if (enableProfilerTimer) { @@ -299,6 +298,7 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber { // Reset all effects except static ones. // Static effects are not specific to a render. workInProgress.flags = current.flags & StaticMask; + workInProgress.subtreeFlags = current.subtreeFlags & StaticMask; workInProgress.childLanes = current.childLanes; workInProgress.lanes = current.lanes; diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js index d5d328a2b31a..f72c82e7de51 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js @@ -72,6 +72,7 @@ import { Snapshot, MutationMask, StaticMask, + PerformedWork, } from './ReactFiberFlags'; import invariant from 'shared/invariant'; @@ -684,40 +685,26 @@ function cutOffTailIfNeeded( function bubbleProperties(completedWork: Fiber): void { bubbleProfilerDurationsFromChildren(completedWork); bubbleLanesFromChildren(completedWork); - bubbleFlagsFromChildren(completedWork); + bubbleFlagsToParent(completedWork); } // TODO (effects) Temorary method; merge this into bubbleProperties once refactor is done. -function bubbleFlagsFromChildren(completedWork: Fiber): void { - const didBailout = - completedWork.alternate !== null && - completedWork.alternate.child === completedWork.child; - - let subtreeFlags = NoFlags; - - if (!didBailout) { - let child = completedWork.child; - while (child !== null) { - subtreeFlags |= child.subtreeFlags; - subtreeFlags |= child.flags; - - child = child.sibling; - } - } else { - let child = completedWork.child; - while (child !== null) { +function bubbleFlagsToParent(completedWork: Fiber): void { + const parent = completedWork.return; + if (parent !== null) { + const didBailout = (completeWork.flags & PerformedWork) !== NoFlags; + if (!didBailout) { + parent.subtreeFlags |= completedWork.subtreeFlags; + parent.subtreeFlags |= completedWork.flags; + } else { // "Static" flags share the lifetime of the fiber/hook they belong to, - // so we should bubble those up even during a bailout. All the other - // flags have a lifetime only of a single render + commit, so we should - // ignore them. - subtreeFlags |= child.subtreeFlags & StaticMask; - subtreeFlags |= child.flags & StaticMask; - - child = child.sibling; + // so we should bubble those up even during a bailout. + // All the other flags have a lifetime only of a single render + commit, + // so we should ignore them. + parent.subtreeFlags |= completedWork.subtreeFlags & StaticMask; + parent.subtreeFlags |= completedWork.flags & StaticMask; } } - - completedWork.subtreeFlags |= subtreeFlags; } // TODO (effects) Temorary method; merge this into bubbleProperties once refactor is done. @@ -1205,7 +1192,7 @@ function completeWork( // Rerender the whole list, but this time, we'll force fallbacks // to stay in place. // Reset the child fibers to their original state. - workInProgress.subtreeFlags = NoFlags; + workInProgress.subtreeFlags &= StaticMask; resetChildFibers(workInProgress, renderLanes); // Set up the Suspense Context to force suspense and immediately diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 8471fa13865e..61d2b5eb206a 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -137,6 +137,7 @@ import { MutationMask, LayoutMask, PassiveMask, + StaticMask, } from './ReactFiberFlags'; import { NoLanePriority, @@ -1753,7 +1754,7 @@ function completeUnitOfWork(unitOfWork: Fiber): void { if (returnFiber !== null) { // Mark the parent fiber as incomplete returnFiber.flags |= Incomplete; - returnFiber.subtreeFlags = NoFlags; + returnFiber.subtreeFlags &= StaticMask; returnFiber.deletions = null; } } From 7f575fafebcfed0ed545ee2cc93c263552bb9f9d Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Mon, 14 Sep 2020 10:21:26 -0400 Subject: [PATCH 4/4] Attempted refactoring of childLanes to be from workInProgress to parent (rather than from children to workInProgress). This attempt isn't quite right though. Has some test failures that requires additional investigation. --- .../react-reconciler/src/ReactFiber.new.js | 4 +- .../src/ReactFiberBeginWork.new.js | 32 ++++++++++++--- .../src/ReactFiberCompleteWork.new.js | 40 ++++++++++--------- .../src/ReactFiberWorkLoop.new.js | 1 + 4 files changed, 51 insertions(+), 26 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiber.new.js b/packages/react-reconciler/src/ReactFiber.new.js index 5d005c2017a3..80af950e5213 100644 --- a/packages/react-reconciler/src/ReactFiber.new.js +++ b/packages/react-reconciler/src/ReactFiber.new.js @@ -299,7 +299,7 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber { // Static effects are not specific to a render. workInProgress.flags = current.flags & StaticMask; workInProgress.subtreeFlags = current.subtreeFlags & StaticMask; - workInProgress.childLanes = current.childLanes; + workInProgress.childLanes = NoLanes; workInProgress.lanes = current.lanes; workInProgress.child = current.child; @@ -388,7 +388,7 @@ export function resetWorkInProgress(workInProgress: Fiber, renderLanes: Lanes) { } } else { // Reset to the cloned values that createWorkInProgress would've. - workInProgress.childLanes = current.childLanes; + workInProgress.childLanes = NoLanes; workInProgress.lanes = current.lanes; workInProgress.child = current.child; diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index fa27d3dcad74..80f4ba8e5178 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -1883,7 +1883,7 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) { prevOffscreenState === null ? mountSuspenseOffscreenState(renderLanes) : updateSuspenseOffscreenState(prevOffscreenState, renderLanes); - primaryChildFragment.childLanes = getRemainingWorkInPrimaryTree( + primaryChildFragment.childLanes = workInProgress.childLanes = getRemainingWorkInPrimaryTree( current, renderLanes, ); @@ -1920,7 +1920,8 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) { prevOffscreenState === null ? mountSuspenseOffscreenState(renderLanes) : updateSuspenseOffscreenState(prevOffscreenState, renderLanes); - primaryChildFragment.childLanes = getRemainingWorkInPrimaryTree( + // TODO (effects) Document + primaryChildFragment.childLanes = workInProgress.childLanes = getRemainingWorkInPrimaryTree( current, renderLanes, ); @@ -1989,6 +1990,18 @@ function mountSuspenseFallbackChildren( primaryChildFragment.childLanes = NoLanes; primaryChildFragment.pendingProps = primaryChildProps; + // TODO (effects) This is tricky because by the time we are resetting the childLanes here + // they have already bubbled to the parent SuspenseComponent and its parent also + // (during the previous complete phase bubbling back up to Suspense). + // + // Compare this to the old fork, where we will restart work on the Suspense node + // before bubbling higher (and letting the values leak out). + // + // We could reset childLanes for the parent SuspenseComponent safely, + // but we can't reset childLanes for its parent without potentially erasing sibling updates. + // Instead we need to prevent the childLanes from bubbling in the first place during a suspend. + workInProgress.childLanes = NoLanes; + if (enableProfilerTimer && workInProgress.mode & ProfileMode) { // Reset the durations from the first pass so they aren't included in the // final amounts. This seems counterintuitive, since we're intentionally @@ -2110,6 +2123,9 @@ function updateSuspenseFallbackChildren( primaryChildFragment.childLanes = NoLanes; primaryChildFragment.pendingProps = primaryChildProps; + // TODO (effects) + workInProgress.childLanes = NoLanes; + if (enableProfilerTimer && workInProgress.mode & ProfileMode) { // Reset the durations from the first pass so they aren't included in the // final amounts. This seems counterintuitive, since we're intentionally @@ -2969,7 +2985,7 @@ function bailoutOnAlreadyFinishedWork( markSkippedUpdateLanes(workInProgress.lanes); // Check if the children have any pending work. - if (!includesSomeLane(renderLanes, workInProgress.childLanes)) { + if (!includesSomeLane(renderLanes, current.childLanes)) { // The children don't have any work either. We can skip them. // TODO: Once we add back resuming, we should check if the children are // a work-in-progress set. If so, we need to transfer their effects. @@ -3118,7 +3134,7 @@ function beginWork( // Profiler should only call onRender when one of its descendants actually rendered. const hasChildWork = includesSomeLane( renderLanes, - workInProgress.childLanes, + current.childLanes, ); if (hasChildWork) { workInProgress.flags |= Update; @@ -3197,9 +3213,15 @@ function beginWork( case SuspenseListComponent: { const didSuspendBefore = (current.flags & DidCapture) !== NoFlags; +// TODO current.childLanes is right; workInProgress.childLanes is wrong. +// Presumably we bubbled something up that we shouldn't have then. +// Maybet his is a variation of the Suspense thing earlier? +// Either we need to reset (or not copy)? +// Or maybe we need to *set* or update in this case? const hasChildWork = includesSomeLane( renderLanes, - workInProgress.childLanes, + // workInProgress.childLanes, + current.childLanes, // This causes a loop for some tests ); if (didSuspendBefore) { diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js index f72c82e7de51..b1dbbc8b5ff2 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js @@ -684,7 +684,7 @@ function cutOffTailIfNeeded( function bubbleProperties(completedWork: Fiber): void { bubbleProfilerDurationsFromChildren(completedWork); - bubbleLanesFromChildren(completedWork); + bubbleLanesToParent(completedWork); bubbleFlagsToParent(completedWork); } @@ -707,20 +707,18 @@ function bubbleFlagsToParent(completedWork: Fiber): void { } } +// TODO (effects) Broken Suspense problem; +// We aren't calling bubbleLanesToParent() for the Suspended components which means OffscreenLane doesn't bubble up. + // TODO (effects) Temorary method; merge this into bubbleProperties once refactor is done. -function bubbleLanesFromChildren(completedWork: Fiber): void { - let newChildLanes = NoLanes; - let child = completedWork.child; - while (child !== null) { - newChildLanes = mergeLanes( - newChildLanes, - mergeLanes(child.lanes, child.childLanes), +function bubbleLanesToParent(completedWork: Fiber): void { + const parent = completedWork.return; + if (parent !== null) { + parent.childLanes = mergeLanes( + parent.childLanes, + mergeLanes(completedWork.lanes, completedWork.childLanes), ); - - child = child.sibling; } - - completedWork.childLanes = newChildLanes; } // TODO (effects) Temorary method; merge this into bubbleProperties once refactor is done. @@ -1014,6 +1012,8 @@ function completeWork( ) { transferActualDuration(workInProgress); } + // TODO (effects) Comment about why + // bubbleProperties(workInProgress); return workInProgress; } @@ -1204,6 +1204,7 @@ function completeWork( ForceSuspenseFallback, ), ); + bubbleProperties(workInProgress); return workInProgress.child; } row = row.sibling; @@ -1332,6 +1333,7 @@ function completeWork( suspenseContext = setDefaultShallowSuspenseContext(suspenseContext); } pushSuspenseContext(workInProgress, suspenseContext); + bubbleProperties(next); // Do a pass over the next row. return next; } @@ -1439,13 +1441,13 @@ function completeWork( } // Don't bubble properties for hidden children. - if ( - !nextIsHidden || - includesSomeLane(subtreeRenderLanes, (OffscreenLane: Lane)) || - (workInProgress.mode & ConcurrentMode) === NoLanes - ) { - bubbleProperties(workInProgress); - } + // if ( + // !nextIsHidden || + // includesSomeLane(subtreeRenderLanes, (OffscreenLane: Lane)) || + // (workInProgress.mode & ConcurrentMode) === NoLanes + // ) { + bubbleProperties(workInProgress); + // } return null; } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 61d2b5eb206a..b8133211cf4b 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -150,6 +150,7 @@ import { SyncLane, SyncBatchedLane, NoTimestamp, + OffscreenLane, findUpdateLane, findTransitionLane, findRetryLane,