Skip to content

Commit 4549be0

Browse files
authored
[Fiber] Optimize enableProfilerCommitHooks by Collecting Elapsed Effect 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.
1 parent 7b56a54 commit 4549be0

7 files changed

+272
-375
lines changed

packages/react-reconciler/src/ReactFiberBeginWork.js

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1033,8 +1033,8 @@ function updateProfiler(
10331033
// Reset effect durations for the next eventual effect phase.
10341034
// These are reset during render to allow the DevTools commit hook a chance to read them,
10351035
const stateNode = workInProgress.stateNode;
1036-
stateNode.effectDuration = 0;
1037-
stateNode.passiveEffectDuration = 0;
1036+
stateNode.effectDuration = -0;
1037+
stateNode.passiveEffectDuration = -0;
10381038
}
10391039
}
10401040
const nextProps = workInProgress.pendingProps;
@@ -3711,8 +3711,8 @@ function attemptEarlyBailoutIfNoScheduledUpdate(
37113711
// Reset effect durations for the next eventual effect phase.
37123712
// These are reset during render to allow the DevTools commit hook a chance to read them,
37133713
const stateNode = workInProgress.stateNode;
3714-
stateNode.effectDuration = 0;
3715-
stateNode.passiveEffectDuration = 0;
3714+
stateNode.effectDuration = -0;
3715+
stateNode.passiveEffectDuration = -0;
37163716
}
37173717
}
37183718
break;

packages/react-reconciler/src/ReactFiberCommitEffects.js

Lines changed: 48 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -31,10 +31,8 @@ import {NoFlags} from './ReactFiberFlags';
3131
import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber';
3232
import {resolveClassComponentProps} from './ReactFiberClassComponent';
3333
import {
34-
recordLayoutEffectDuration,
35-
startLayoutEffectTimer,
36-
recordPassiveEffectDuration,
37-
startPassiveEffectTimer,
34+
recordEffectDuration,
35+
startEffectTimer,
3836
isCurrentUpdateNested,
3937
} from './ReactProfilerTimer';
4038
import {NoMode, ProfileMode} from './ReactTypeOfMode';
@@ -91,14 +89,41 @@ export function commitHookLayoutEffects(
9189
// e.g. a destroy function in one component should never override a ref set
9290
// by a create function in another component during the same commit.
9391
if (shouldProfile(finishedWork)) {
94-
startLayoutEffectTimer();
92+
startEffectTimer();
9593
commitHookEffectListMount(hookFlags, finishedWork);
96-
recordLayoutEffectDuration(finishedWork);
94+
recordEffectDuration(finishedWork);
9795
} else {
9896
commitHookEffectListMount(hookFlags, finishedWork);
9997
}
10098
}
10199

100+
export function commitHookLayoutUnmountEffects(
101+
finishedWork: Fiber,
102+
nearestMountedAncestor: null | Fiber,
103+
hookFlags: HookFlags,
104+
) {
105+
// Layout effects are destroyed during the mutation phase so that all
106+
// destroy functions for all fibers are called before any create functions.
107+
// This prevents sibling component effects from interfering with each other,
108+
// e.g. a destroy function in one component should never override a ref set
109+
// by a create function in another component during the same commit.
110+
if (shouldProfile(finishedWork)) {
111+
startEffectTimer();
112+
commitHookEffectListUnmount(
113+
hookFlags,
114+
finishedWork,
115+
nearestMountedAncestor,
116+
);
117+
recordEffectDuration(finishedWork);
118+
} else {
119+
commitHookEffectListUnmount(
120+
hookFlags,
121+
finishedWork,
122+
nearestMountedAncestor,
123+
);
124+
}
125+
}
126+
102127
export function commitHookEffectListMount(
103128
flags: HookFlags,
104129
finishedWork: Fiber,
@@ -265,9 +290,9 @@ export function commitHookPassiveMountEffects(
265290
hookFlags: HookFlags,
266291
) {
267292
if (shouldProfile(finishedWork)) {
268-
startPassiveEffectTimer();
293+
startEffectTimer();
269294
commitHookEffectListMount(hookFlags, finishedWork);
270-
recordPassiveEffectDuration(finishedWork);
295+
recordEffectDuration(finishedWork);
271296
} else {
272297
commitHookEffectListMount(hookFlags, finishedWork);
273298
}
@@ -279,13 +304,13 @@ export function commitHookPassiveUnmountEffects(
279304
hookFlags: HookFlags,
280305
) {
281306
if (shouldProfile(finishedWork)) {
282-
startPassiveEffectTimer();
307+
startEffectTimer();
283308
commitHookEffectListUnmount(
284309
hookFlags,
285310
finishedWork,
286311
nearestMountedAncestor,
287312
);
288-
recordPassiveEffectDuration(finishedWork);
313+
recordEffectDuration(finishedWork);
289314
} else {
290315
commitHookEffectListUnmount(
291316
hookFlags,
@@ -333,7 +358,7 @@ export function commitClassLayoutLifecycles(
333358
}
334359
}
335360
if (shouldProfile(finishedWork)) {
336-
startLayoutEffectTimer();
361+
startEffectTimer();
337362
if (__DEV__) {
338363
runWithFiberInDEV(
339364
finishedWork,
@@ -348,7 +373,7 @@ export function commitClassLayoutLifecycles(
348373
captureCommitPhaseError(finishedWork, finishedWork.return, error);
349374
}
350375
}
351-
recordLayoutEffectDuration(finishedWork);
376+
recordEffectDuration(finishedWork);
352377
} else {
353378
if (__DEV__) {
354379
runWithFiberInDEV(
@@ -404,7 +429,7 @@ export function commitClassLayoutLifecycles(
404429
}
405430
}
406431
if (shouldProfile(finishedWork)) {
407-
startLayoutEffectTimer();
432+
startEffectTimer();
408433
if (__DEV__) {
409434
runWithFiberInDEV(
410435
finishedWork,
@@ -426,7 +451,7 @@ export function commitClassLayoutLifecycles(
426451
captureCommitPhaseError(finishedWork, finishedWork.return, error);
427452
}
428453
}
429-
recordLayoutEffectDuration(finishedWork);
454+
recordEffectDuration(finishedWork);
430455
} else {
431456
if (__DEV__) {
432457
runWithFiberInDEV(
@@ -679,7 +704,7 @@ export function safelyCallComponentWillUnmount(
679704
);
680705
instance.state = current.memoizedState;
681706
if (shouldProfile(current)) {
682-
startLayoutEffectTimer();
707+
startEffectTimer();
683708
if (__DEV__) {
684709
runWithFiberInDEV(
685710
current,
@@ -695,7 +720,7 @@ export function safelyCallComponentWillUnmount(
695720
captureCommitPhaseError(current, nearestMountedAncestor, error);
696721
}
697722
}
698-
recordLayoutEffectDuration(current);
723+
recordEffectDuration(current);
699724
} else {
700725
if (__DEV__) {
701726
runWithFiberInDEV(
@@ -736,10 +761,10 @@ function commitAttachRef(finishedWork: Fiber) {
736761
if (typeof ref === 'function') {
737762
if (shouldProfile(finishedWork)) {
738763
try {
739-
startLayoutEffectTimer();
764+
startEffectTimer();
740765
finishedWork.refCleanup = ref(instanceToUse);
741766
} finally {
742-
recordLayoutEffectDuration(finishedWork);
767+
recordEffectDuration(finishedWork);
743768
}
744769
} else {
745770
finishedWork.refCleanup = ref(instanceToUse);
@@ -793,14 +818,14 @@ export function safelyDetachRef(
793818
try {
794819
if (shouldProfile(current)) {
795820
try {
796-
startLayoutEffectTimer();
821+
startEffectTimer();
797822
if (__DEV__) {
798823
runWithFiberInDEV(current, refCleanup);
799824
} else {
800825
refCleanup();
801826
}
802827
} finally {
803-
recordLayoutEffectDuration(current);
828+
recordEffectDuration(current);
804829
}
805830
} else {
806831
if (__DEV__) {
@@ -823,14 +848,14 @@ export function safelyDetachRef(
823848
try {
824849
if (shouldProfile(current)) {
825850
try {
826-
startLayoutEffectTimer();
851+
startEffectTimer();
827852
if (__DEV__) {
828853
(runWithFiberInDEV(current, ref, null): void);
829854
} else {
830855
ref(null);
831856
}
832857
} finally {
833-
recordLayoutEffectDuration(current);
858+
recordEffectDuration(current);
834859
}
835860
} else {
836861
if (__DEV__) {
@@ -849,7 +874,7 @@ export function safelyDetachRef(
849874
}
850875
}
851876

852-
export function safelyCallDestroy(
877+
function safelyCallDestroy(
853878
current: Fiber,
854879
nearestMountedAncestor: Fiber | null,
855880
destroy: () => void,

0 commit comments

Comments
 (0)