From abe0cd34c606c69f83092158f7b0815d821e52d6 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 18 Sep 2020 15:02:26 -0500 Subject: [PATCH] Use Passive flag to schedule onPostCommit (#19862) Instead of calling `onPostCommit` in a separate phase, we can fire them during the same traversal as the rest of the passive effects. This works because effects are executed depth-first. So by the time we reach a Profiler node, we'll have already executed all the effects in its subtree. --- .../src/ReactFiberBeginWork.new.js | 7 +- .../src/ReactFiberCommitWork.new.js | 100 +++++++++--------- .../src/ReactFiberWorkLoop.new.js | 33 +----- 3 files changed, 60 insertions(+), 80 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index fa27d3dcad74e..ef5b69c051322 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -61,6 +61,7 @@ import { ContentReset, DidCapture, Update, + Passive, Ref, Deletion, ForceUpdateForLegacySuspense, @@ -674,7 +675,8 @@ function updateProfiler( renderLanes: Lanes, ) { if (enableProfilerTimer) { - workInProgress.flags |= Update; + // TODO: Only call onRender et al if subtree has effects + workInProgress.flags |= Update | Passive; // Reset effect durations for the next eventual effect phase. // These are reset during render to allow the DevTools commit hook a chance to read them, @@ -3116,12 +3118,13 @@ function beginWork( case Profiler: if (enableProfilerTimer) { // Profiler should only call onRender when one of its descendants actually rendered. + // TODO: Only call onRender et al if subtree has effects const hasChildWork = includesSomeLane( renderLanes, workInProgress.childLanes, ); if (hasChildWork) { - workInProgress.flags |= Update; + workInProgress.flags |= Passive | Update; } // Reset effect durations for the next eventual effect phase. diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index 1943b7ea2dc74..0027250803638 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -121,7 +121,6 @@ import { captureCommitPhaseError, resolveRetryWakeable, markCommitTimeOfFallback, - enqueuePendingPassiveProfilerEffect, schedulePassiveEffectCallback, } from './ReactFiberWorkLoop.new'; import { @@ -507,57 +506,55 @@ function commitHookEffectListMount2(fiber: Fiber): void { } } -export function commitPassiveEffectDurations( +function commitProfilerPassiveEffect( finishedRoot: FiberRoot, finishedWork: Fiber, ): void { if (enableProfilerTimer && enableProfilerCommitHooks) { - // Only Profilers with work in their subtree will have an Update effect scheduled. - if ((finishedWork.flags & Update) !== NoFlags) { - switch (finishedWork.tag) { - case Profiler: { - const {passiveEffectDuration} = finishedWork.stateNode; - const {id, onPostCommit} = finishedWork.memoizedProps; - - // This value will still reflect the previous commit phase. - // It does not get reset until the start of the next commit phase. - const commitTime = getCommitTime(); - - if (typeof onPostCommit === 'function') { - if (enableSchedulerTracing) { - onPostCommit( - id, - finishedWork.alternate === null ? 'mount' : 'update', - passiveEffectDuration, - commitTime, - finishedRoot.memoizedInteractions, - ); - } else { - onPostCommit( - id, - finishedWork.alternate === null ? 'mount' : 'update', - passiveEffectDuration, - commitTime, - ); - } + switch (finishedWork.tag) { + case Profiler: { + const {passiveEffectDuration} = finishedWork.stateNode; + const {id, onPostCommit} = finishedWork.memoizedProps; + + // This value will still reflect the previous commit phase. + // It does not get reset until the start of the next commit phase. + const commitTime = getCommitTime(); + + if (typeof onPostCommit === 'function') { + if (enableSchedulerTracing) { + onPostCommit( + id, + finishedWork.alternate === null ? 'mount' : 'update', + passiveEffectDuration, + commitTime, + finishedRoot.memoizedInteractions, + ); + } else { + onPostCommit( + id, + finishedWork.alternate === null ? 'mount' : 'update', + passiveEffectDuration, + commitTime, + ); } + } - // Bubble times to the next nearest ancestor Profiler. - // After we process that Profiler, we'll bubble further up. - let parentFiber = finishedWork.return; - while (parentFiber !== null) { - if (parentFiber.tag === Profiler) { - const parentStateNode = parentFiber.stateNode; - parentStateNode.passiveEffectDuration += passiveEffectDuration; - break; - } - parentFiber = parentFiber.return; + // Bubble times to the next nearest ancestor Profiler. + // After we process that Profiler, we'll bubble further up. + // TODO: Use JS Stack instead + let parentFiber = finishedWork.return; + while (parentFiber !== null) { + if (parentFiber.tag === Profiler) { + const parentStateNode = parentFiber.stateNode; + parentStateNode.passiveEffectDuration += passiveEffectDuration; + break; } - break; + parentFiber = parentFiber.return; } - default: - break; + break; } + default: + break; } } } @@ -841,13 +838,9 @@ function commitLifeCycles( } } - // Schedule a passive effect for this Profiler to call onPostCommit hooks. - // This effect should be scheduled even if there is no onPostCommit callback for this Profiler, - // because the effect is also where times bubble to parent Profilers. - enqueuePendingPassiveProfilerEffect(finishedWork); - // Propagate layout effect durations to the next nearest Profiler ancestor. // Do not reset these values until the next render so DevTools has a chance to read them first. + // TODO: Use JS Stack instead let parentFiber = finishedWork.return; while (parentFiber !== null) { if (parentFiber.tag === Profiler) { @@ -1912,6 +1905,7 @@ function commitPassiveWork(finishedWork: Fiber): void { finishedWork, finishedWork.return, ); + break; } } } @@ -1933,13 +1927,21 @@ function commitPassiveUnmount( } } -function commitPassiveLifeCycles(finishedWork: Fiber): void { +function commitPassiveLifeCycles( + finishedRoot: FiberRoot, + finishedWork: Fiber, +): void { switch (finishedWork.tag) { case FunctionComponent: case ForwardRef: case SimpleMemoComponent: case Block: { commitHookEffectListMount2(finishedWork); + break; + } + case Profiler: { + commitProfilerPassiveEffect(finishedRoot, finishedWork); + break; } } } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 361824de35e9e..6192fc7c422ac 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -21,7 +21,6 @@ import { enableSuspenseServerRenderer, replayFailedUnitOfWorkWithInvokeGuardedCallback, enableProfilerTimer, - enableProfilerCommitHooks, enableSchedulerTracing, warnAboutUnmockedScheduler, deferRenderPhaseUpdateToNextBatch, @@ -197,7 +196,6 @@ import { commitPassiveLifeCycles as commitPassiveEffectOnFiber, commitDetachRef, commitAttachRef, - commitPassiveEffectDurations, commitResetTextContent, isSuspenseBoundaryBeingHidden, } from './ReactFiberCommitWork.new'; @@ -336,7 +334,6 @@ let rootDoesHavePassiveEffects: boolean = false; let rootWithPendingPassiveEffects: FiberRoot | null = null; let pendingPassiveEffectsRenderPriority: ReactPriorityLevel = NoSchedulerPriority; let pendingPassiveEffectsLanes: Lanes = NoLanes; -let pendingPassiveProfilerEffects: Array = []; let rootsWithPendingDiscreteUpdates: Set | null = null; @@ -2451,31 +2448,18 @@ export function flushPassiveEffects(): boolean { return false; } -export function enqueuePendingPassiveProfilerEffect(fiber: Fiber): void { - if (enableProfilerTimer && enableProfilerCommitHooks) { - pendingPassiveProfilerEffects.push(fiber); - if (!rootDoesHavePassiveEffects) { - rootDoesHavePassiveEffects = true; - scheduleCallback(NormalSchedulerPriority, () => { - flushPassiveEffects(); - return null; - }); - } - } -} - -function flushPassiveMountEffects(firstChild: Fiber): void { +function flushPassiveMountEffects(root, firstChild: Fiber): void { let fiber = firstChild; while (fiber !== null) { const primarySubtreeFlags = fiber.subtreeFlags & PassiveMask; if (fiber.child !== null && primarySubtreeFlags !== NoFlags) { - flushPassiveMountEffects(fiber.child); + flushPassiveMountEffects(root, fiber.child); } if ((fiber.flags & Passive) !== NoFlags) { setCurrentDebugFiberInDEV(fiber); - commitPassiveEffectOnFiber(fiber); + commitPassiveEffectOnFiber(root, fiber); resetCurrentDebugFiberInDEV(); } @@ -2586,16 +2570,7 @@ function flushPassiveEffectsImpl() { // value set by a create function in another component. // Layout effects have the same constraint. flushPassiveUnmountEffects(root.current); - flushPassiveMountEffects(root.current); - - if (enableProfilerTimer && enableProfilerCommitHooks) { - const profilerEffects = pendingPassiveProfilerEffects; - pendingPassiveProfilerEffects = []; - for (let i = 0; i < profilerEffects.length; i++) { - const fiber = ((profilerEffects[i]: any): Fiber); - commitPassiveEffectDurations(root, fiber); - } - } + flushPassiveMountEffects(root, root.current); if (enableSchedulerTracing) { popInteractions(((prevInteractions: any): Set));