Skip to content

Commit

Permalink
Use Passive flag to schedule onPostCommit (#19862)
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@acdlite
acdlite authored Sep 18, 2020
1 parent 50d9451 commit 8b2d378
Show file tree
Hide file tree
Showing 3 changed files with 60 additions and 80 deletions.
7 changes: 5 additions & 2 deletions packages/react-reconciler/src/ReactFiberBeginWork.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ import {
ContentReset,
DidCapture,
Update,
Passive,
Ref,
Deletion,
ForceUpdateForLegacySuspense,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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.
Expand Down
100 changes: 51 additions & 49 deletions packages/react-reconciler/src/ReactFiberCommitWork.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,6 @@ import {
captureCommitPhaseError,
resolveRetryWakeable,
markCommitTimeOfFallback,
enqueuePendingPassiveProfilerEffect,
schedulePassiveEffectCallback,
} from './ReactFiberWorkLoop.new';
import {
Expand Down Expand Up @@ -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;
}
}
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -1912,6 +1905,7 @@ function commitPassiveWork(finishedWork: Fiber): void {
finishedWork,
finishedWork.return,
);
break;
}
}
}
Expand All @@ -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;
}
}
}
Expand Down
33 changes: 4 additions & 29 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import {
enableSuspenseServerRenderer,
replayFailedUnitOfWorkWithInvokeGuardedCallback,
enableProfilerTimer,
enableProfilerCommitHooks,
enableSchedulerTracing,
warnAboutUnmockedScheduler,
deferRenderPhaseUpdateToNextBatch,
Expand Down Expand Up @@ -197,7 +196,6 @@ import {
commitPassiveLifeCycles as commitPassiveEffectOnFiber,
commitDetachRef,
commitAttachRef,
commitPassiveEffectDurations,
commitResetTextContent,
isSuspenseBoundaryBeingHidden,
} from './ReactFiberCommitWork.new';
Expand Down Expand Up @@ -336,7 +334,6 @@ let rootDoesHavePassiveEffects: boolean = false;
let rootWithPendingPassiveEffects: FiberRoot | null = null;
let pendingPassiveEffectsRenderPriority: ReactPriorityLevel = NoSchedulerPriority;
let pendingPassiveEffectsLanes: Lanes = NoLanes;
let pendingPassiveProfilerEffects: Array<Fiber> = [];

let rootsWithPendingDiscreteUpdates: Set<FiberRoot> | null = null;

Expand Down Expand Up @@ -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();
}

Expand Down Expand Up @@ -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<Interaction>));
Expand Down

0 comments on commit 8b2d378

Please sign in to comment.