Skip to content

Commit

Permalink
[Fiber] Optimize enableProfilerCommitHooks by Collecting Elapsed Effe…
Browse files Browse the repository at this point in the history
…ct Duration in Module Scope (#30981)

Stacked on #30979.

The problem with the previous approach is that it recursively walked the
tree up to propagate the resulting time from recording a layout effect.

Instead, we keep a running count of the effect duration on the module
scope. Then we reset it when entering a nested Profiler and then we add
its elapsed count when we exit the Profiler.

This also fixes a bug where we weren't previously including unmount
times for some detached trees since they couldn't bubble up to find the
profiler.
  • Loading branch information
sebmarkbage authored Sep 17, 2024
1 parent 7b56a54 commit 4549be0
Show file tree
Hide file tree
Showing 7 changed files with 272 additions and 375 deletions.
8 changes: 4 additions & 4 deletions packages/react-reconciler/src/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -1033,8 +1033,8 @@ function updateProfiler(
// 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;
stateNode.effectDuration = 0;
stateNode.passiveEffectDuration = 0;
stateNode.effectDuration = -0;
stateNode.passiveEffectDuration = -0;
}
}
const nextProps = workInProgress.pendingProps;
Expand Down Expand Up @@ -3711,8 +3711,8 @@ function attemptEarlyBailoutIfNoScheduledUpdate(
// 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;
stateNode.effectDuration = 0;
stateNode.passiveEffectDuration = 0;
stateNode.effectDuration = -0;
stateNode.passiveEffectDuration = -0;
}
}
break;
Expand Down
71 changes: 48 additions & 23 deletions packages/react-reconciler/src/ReactFiberCommitEffects.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,8 @@ import {NoFlags} from './ReactFiberFlags';
import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber';
import {resolveClassComponentProps} from './ReactFiberClassComponent';
import {
recordLayoutEffectDuration,
startLayoutEffectTimer,
recordPassiveEffectDuration,
startPassiveEffectTimer,
recordEffectDuration,
startEffectTimer,
isCurrentUpdateNested,
} from './ReactProfilerTimer';
import {NoMode, ProfileMode} from './ReactTypeOfMode';
Expand Down Expand Up @@ -91,14 +89,41 @@ export function commitHookLayoutEffects(
// e.g. a destroy function in one component should never override a ref set
// by a create function in another component during the same commit.
if (shouldProfile(finishedWork)) {
startLayoutEffectTimer();
startEffectTimer();
commitHookEffectListMount(hookFlags, finishedWork);
recordLayoutEffectDuration(finishedWork);
recordEffectDuration(finishedWork);
} else {
commitHookEffectListMount(hookFlags, finishedWork);
}
}

export function commitHookLayoutUnmountEffects(
finishedWork: Fiber,
nearestMountedAncestor: null | Fiber,
hookFlags: HookFlags,
) {
// Layout effects are destroyed during the mutation phase so that all
// destroy functions for all fibers are called before any create functions.
// This prevents sibling component effects from interfering with each other,
// e.g. a destroy function in one component should never override a ref set
// by a create function in another component during the same commit.
if (shouldProfile(finishedWork)) {
startEffectTimer();
commitHookEffectListUnmount(
hookFlags,
finishedWork,
nearestMountedAncestor,
);
recordEffectDuration(finishedWork);
} else {
commitHookEffectListUnmount(
hookFlags,
finishedWork,
nearestMountedAncestor,
);
}
}

export function commitHookEffectListMount(
flags: HookFlags,
finishedWork: Fiber,
Expand Down Expand Up @@ -265,9 +290,9 @@ export function commitHookPassiveMountEffects(
hookFlags: HookFlags,
) {
if (shouldProfile(finishedWork)) {
startPassiveEffectTimer();
startEffectTimer();
commitHookEffectListMount(hookFlags, finishedWork);
recordPassiveEffectDuration(finishedWork);
recordEffectDuration(finishedWork);
} else {
commitHookEffectListMount(hookFlags, finishedWork);
}
Expand All @@ -279,13 +304,13 @@ export function commitHookPassiveUnmountEffects(
hookFlags: HookFlags,
) {
if (shouldProfile(finishedWork)) {
startPassiveEffectTimer();
startEffectTimer();
commitHookEffectListUnmount(
hookFlags,
finishedWork,
nearestMountedAncestor,
);
recordPassiveEffectDuration(finishedWork);
recordEffectDuration(finishedWork);
} else {
commitHookEffectListUnmount(
hookFlags,
Expand Down Expand Up @@ -333,7 +358,7 @@ export function commitClassLayoutLifecycles(
}
}
if (shouldProfile(finishedWork)) {
startLayoutEffectTimer();
startEffectTimer();
if (__DEV__) {
runWithFiberInDEV(
finishedWork,
Expand All @@ -348,7 +373,7 @@ export function commitClassLayoutLifecycles(
captureCommitPhaseError(finishedWork, finishedWork.return, error);
}
}
recordLayoutEffectDuration(finishedWork);
recordEffectDuration(finishedWork);
} else {
if (__DEV__) {
runWithFiberInDEV(
Expand Down Expand Up @@ -404,7 +429,7 @@ export function commitClassLayoutLifecycles(
}
}
if (shouldProfile(finishedWork)) {
startLayoutEffectTimer();
startEffectTimer();
if (__DEV__) {
runWithFiberInDEV(
finishedWork,
Expand All @@ -426,7 +451,7 @@ export function commitClassLayoutLifecycles(
captureCommitPhaseError(finishedWork, finishedWork.return, error);
}
}
recordLayoutEffectDuration(finishedWork);
recordEffectDuration(finishedWork);
} else {
if (__DEV__) {
runWithFiberInDEV(
Expand Down Expand Up @@ -679,7 +704,7 @@ export function safelyCallComponentWillUnmount(
);
instance.state = current.memoizedState;
if (shouldProfile(current)) {
startLayoutEffectTimer();
startEffectTimer();
if (__DEV__) {
runWithFiberInDEV(
current,
Expand All @@ -695,7 +720,7 @@ export function safelyCallComponentWillUnmount(
captureCommitPhaseError(current, nearestMountedAncestor, error);
}
}
recordLayoutEffectDuration(current);
recordEffectDuration(current);
} else {
if (__DEV__) {
runWithFiberInDEV(
Expand Down Expand Up @@ -736,10 +761,10 @@ function commitAttachRef(finishedWork: Fiber) {
if (typeof ref === 'function') {
if (shouldProfile(finishedWork)) {
try {
startLayoutEffectTimer();
startEffectTimer();
finishedWork.refCleanup = ref(instanceToUse);
} finally {
recordLayoutEffectDuration(finishedWork);
recordEffectDuration(finishedWork);
}
} else {
finishedWork.refCleanup = ref(instanceToUse);
Expand Down Expand Up @@ -793,14 +818,14 @@ export function safelyDetachRef(
try {
if (shouldProfile(current)) {
try {
startLayoutEffectTimer();
startEffectTimer();
if (__DEV__) {
runWithFiberInDEV(current, refCleanup);
} else {
refCleanup();
}
} finally {
recordLayoutEffectDuration(current);
recordEffectDuration(current);
}
} else {
if (__DEV__) {
Expand All @@ -823,14 +848,14 @@ export function safelyDetachRef(
try {
if (shouldProfile(current)) {
try {
startLayoutEffectTimer();
startEffectTimer();
if (__DEV__) {
(runWithFiberInDEV(current, ref, null): void);
} else {
ref(null);
}
} finally {
recordLayoutEffectDuration(current);
recordEffectDuration(current);
}
} else {
if (__DEV__) {
Expand All @@ -849,7 +874,7 @@ export function safelyDetachRef(
}
}

export function safelyCallDestroy(
function safelyCallDestroy(
current: Fiber,
nearestMountedAncestor: Fiber | null,
destroy: () => void,
Expand Down
Loading

0 comments on commit 4549be0

Please sign in to comment.