Skip to content

[Fiber] Optimize enableProfilerCommitHooks by Collecting Elapsed Effect Duration in Module Scope #30981

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 8 commits into from
Sep 17, 2024
Merged

[Fiber] Optimize enableProfilerCommitHooks by Collecting Elapsed Effect Duration in Module Scope #30981

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
ba3af2f
Move layout hook unmounting to helper
sebmarkbage Sep 16, 2024
bc36701
Unify Hook Deletion on Unmount
sebmarkbage Sep 16, 2024
fdd833f
Similarly this should use the commitHookLayoutUnmountEffects
sebmarkbage Sep 16, 2024
f5775d7
Layout effect profiling has now successfully moved into CommitEffects.
sebmarkbage Sep 16, 2024
2c9b1ee
Track the currently computing effect durations on module scope
sebmarkbage Sep 16, 2024
aa36862
Collect timings for unmount layout effects and unmount passive effect…
sebmarkbage Sep 16, 2024
202fdec
Update tests to reflect that we're now including unmount times for de…
sebmarkbage Sep 16, 2024
a41c35f
Unify startEffectTimer and recordEffectDuration
sebmarkbage Sep 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions packages/react-reconciler/src/ReactFiberBeginWork.js
View file
Open in desktop
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
View file
Open in desktop
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
Loading