From 788e064767f3d8be520869a3bfad9699e89a380a Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 18 Sep 2020 15:02:03 -0500 Subject: [PATCH] Don't call onCommit et al if there are no effects Checks `subtreeFlags` before scheduling an effect on the Profiler. --- .../src/ReactFiberBeginWork.new.js | 13 ----- .../src/ReactFiberCommitWork.new.js | 15 ++++- .../src/ReactFiberCompleteWork.new.js | 55 ++++++++++++++++++- 3 files changed, 67 insertions(+), 16 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index ef5b69c051322..30601e5a9cdbc 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -675,9 +675,6 @@ function updateProfiler( renderLanes: Lanes, ) { if (enableProfilerTimer) { - // 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, const stateNode = workInProgress.stateNode; @@ -3117,16 +3114,6 @@ 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 |= Passive | Update; - } - // 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, const stateNode = workInProgress.stateNode; diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index 0027250803638..0fd280b8f5556 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -71,6 +71,7 @@ import { Placement, Snapshot, Update, + Callback, PassiveMask, } from './ReactFiberFlags'; import getComponentName from 'shared/getComponentName'; @@ -792,10 +793,17 @@ function commitLifeCycles( if (enableProfilerTimer) { const {onCommit, onRender} = finishedWork.memoizedProps; const {effectDuration} = finishedWork.stateNode; + const flags = finishedWork.flags; const commitTime = getCommitTime(); - if (typeof onRender === 'function') { + const OnRenderFlag = Update; + const OnCommitFlag = Callback; + + if ( + (flags & OnRenderFlag) !== NoFlags && + typeof onRender === 'function' + ) { if (enableSchedulerTracing) { onRender( finishedWork.memoizedProps.id, @@ -819,7 +827,10 @@ function commitLifeCycles( } if (enableProfilerCommitHooks) { - if (typeof onCommit === 'function') { + if ( + (flags & OnCommitFlag) !== NoFlags && + typeof onCommit === 'function' + ) { if (enableSchedulerTracing) { onCommit( finishedWork.memoizedProps.id, diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js index b0e74e84ead40..f4b8380deaa3d 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js @@ -67,10 +67,15 @@ import { import { Ref, Update, + Callback, + Passive, + Deletion, NoFlags, DidCapture, Snapshot, MutationMask, + LayoutMask, + PassiveMask, StaticMask, } from './ReactFiberFlags'; import invariant from 'shared/invariant'; @@ -787,6 +792,8 @@ function bubbleProperties(completedWork: Fiber) { } completedWork.childLanes = newChildLanes; + + return didBailout; } function completeWork( @@ -804,7 +811,6 @@ function completeWork( case ForwardRef: case Fragment: case Mode: - case Profiler: case ContextConsumer: case MemoComponent: bubbleProperties(workInProgress); @@ -966,6 +972,53 @@ function completeWork( bubbleProperties(workInProgress); return null; } + case Profiler: { + const didBailout = bubbleProperties(workInProgress); + if (!didBailout) { + // Use subtreeFlags to determine which commit callbacks should fire. + // TODO: Move this logic to the commit phase, since we already check if + // a fiber's subtree contains effects. Refactor the commit phase's + // depth-first traversal so that we can put work tag-specific logic + // before or after committing a subtree's effects. + const OnRenderFlag = Update; + const OnCommitFlag = Callback; + const OnPostCommitFlag = Passive; + const subtreeFlags = workInProgress.subtreeFlags; + const flags = workInProgress.flags; + let newFlags = flags; + + // Call onRender any time this fiber or its subtree are worked on, even + // if there are no effects + newFlags |= OnRenderFlag; + + // Call onCommit only if the subtree contains layout work, or if it + // contains deletions, since those might result in unmount work, which + // we include in the same measure. + // TODO: Can optimize by using a static flag to track whether a tree + // contains layout effects, like we do for passive effects. + if ( + (flags & (LayoutMask | Deletion)) !== NoFlags || + (subtreeFlags & (LayoutMask | Deletion)) !== NoFlags + ) { + newFlags |= OnCommitFlag; + } + + // Call onPostCommit only if the subtree contains passive work. + // Don't have to check for deletions, because Deletion is already + // a passive flag. + if ( + (flags & PassiveMask) !== NoFlags || + (subtreeFlags & PassiveMask) !== NoFlags + ) { + newFlags |= OnPostCommitFlag; + } + workInProgress.flags = newFlags; + } else { + // This fiber and its subtree bailed out, so don't fire any callbacks. + } + + return null; + } case SuspenseComponent: { popSuspenseContext(workInProgress); const nextState: null | SuspenseState = workInProgress.memoizedState;