[Fiber] Move Profiler onPostCommit processing of passive effect durations to plain passive effect (#30966)
We used to queue a separate third passive phase to invoke onPostCommit but this is unnecessary. We can just treat it as a plain passive effect. This means it is interleaved with other passive effects but we only need to know the duration of the things below us which is already done at this point. I also extracted the user space call to onPostCommit into ReactCommitEffects. Same as onCommit. It's now covered by runWithFiberInDEV and catches.
Sebastian Markbåge committed
Sep 16, 2024 at 11:10 UTC
ee1a403a3019dd8bffb12174d269d8c85bfab8a1
4 files changed
+106
-96
packages/react-reconciler/src/ReactFiberBeginWork.js
+8
@@ -1026,6 +1026,10 @@ function updateProfiler(
1026
workInProgress.flags |= Update;
1027
1028
if (enableProfilerCommitHooks) {
1029
+ // Schedule a passive effect for this Profiler to call onPostCommit hooks.
1030
+ // This effect should be scheduled even if there is no onPostCommit callback for this Profiler,
1031
+ // because the effect is also where times bubble to parent Profilers.
1032
+ workInProgress.flags |= Passive;
1033
// Reset effect durations for the next eventual effect phase.
1034
// These are reset during render to allow the DevTools commit hook a chance to read them,
1035
const stateNode = workInProgress.stateNode;
@@ -3700,6 +3704,10 @@ function attemptEarlyBailoutIfNoScheduledUpdate(
3704
}
3705
3706
if (enableProfilerCommitHooks) {
3707
+ // Schedule a passive effect for this Profiler to call onPostCommit hooks.
3708
+ // This effect should be scheduled even if there is no onPostCommit callback for this Profiler,
3709
+ // because the effect is also where times bubble to parent Profilers.
3710
+ workInProgress.flags |= Passive;
3711
// Reset effect durations for the next eventual effect phase.
3712
// These are reset during render to allow the DevTools commit hook a chance to read them,
3713
const stateNode = workInProgress.stateNode;
packages/react-reconciler/src/ReactFiberCommitEffects.js
+51
-2
@@ -881,7 +881,7 @@ function commitProfiler(
881
commitTime: number,
882
effectDuration: number,
883
) {
884
- const {onCommit, onRender} = finishedWork.memoizedProps;
884
+ const {id, onCommit, onRender} = finishedWork.memoizedProps;
885
886
let phase = current === null ? 'mount' : 'update';
887
if (enableProfilerNestedUpdatePhase) {
@@ -892,7 +892,7 @@ function commitProfiler(
892
893
if (typeof onRender === 'function') {
894
onRender(
895
- finishedWork.memoizedProps.id,
895
+ id,
896
phase,
897
finishedWork.actualDuration,
898
finishedWork.treeBaseDuration,
@@ -938,3 +938,52 @@ export function commitProfilerUpdate(
938
}
939
}
940
}
941
+
942
+function commitProfilerPostCommitImpl(
943
+ finishedWork: Fiber,
944
+ current: Fiber | null,
945
+ commitTime: number,
946
+ passiveEffectDuration: number,
947
+): void {
948
+ const {id, onPostCommit} = finishedWork.memoizedProps;
949
+
950
+ let phase = current === null ? 'mount' : 'update';
951
+ if (enableProfilerNestedUpdatePhase) {
952
+ if (isCurrentUpdateNested()) {
953
+ phase = 'nested-update';
954
+ }
955
+ }
956
+
957
+ if (typeof onPostCommit === 'function') {
958
+ onPostCommit(id, phase, passiveEffectDuration, commitTime);
959
+ }
960
+}
961
+
962
+export function commitProfilerPostCommit(
963
+ finishedWork: Fiber,
964
+ current: Fiber | null,
965
+ commitTime: number,
966
+ passiveEffectDuration: number,
967
+) {
968
+ try {
969
+ if (__DEV__) {
970
+ runWithFiberInDEV(
971
+ finishedWork,
972
+ commitProfilerPostCommitImpl,
973
+ finishedWork,
974
+ current,
975
+ commitTime,
976
+ passiveEffectDuration,
977
+ );
978
+ } else {
979
+ commitProfilerPostCommitImpl(
980
+ finishedWork,
981
+ current,
982
+ commitTime,
983
+ passiveEffectDuration,
984
+ );
985
+ }
986
+ } catch (error) {
987
+ captureCommitPhaseError(finishedWork, finishedWork.return, error);
988
+ }
989
+}
packages/react-reconciler/src/ReactFiberCommitWork.js
+47
-69
@@ -44,7 +44,6 @@ import {
44
enablePersistedModeClonedFlag,
45
enableProfilerTimer,
46
enableProfilerCommitHooks,
47
- enableProfilerNestedUpdatePhase,
47
enableSchedulingProfiler,
48
enableSuspenseCallback,
49
enableScopeAPI,
@@ -100,7 +99,6 @@ import {
99
Cloned,
100
} from './ReactFiberFlags';
101
import {
103
- isCurrentUpdateNested,
102
getCommitTime,
103
recordLayoutEffectDuration,
104
startLayoutEffectTimer,
@@ -137,7 +135,6 @@ import {
135
captureCommitPhaseError,
136
resolveRetryWakeable,
137
markCommitTimeOfFallback,
140
- enqueuePendingPassiveProfilerEffect,
138
restorePendingUpdaters,
139
addTransitionStartCallbackToPendingTransition,
140
addTransitionProgressCallbackToPendingTransition,
@@ -193,6 +190,7 @@ import {
190
safelyDetachRef,
191
safelyCallDestroy,
192
commitProfilerUpdate,
193
+ commitProfilerPostCommit,
194
commitRootCallbacks,
195
} from './ReactFiberCommitEffects';
196
import {
@@ -394,62 +392,6 @@ function commitBeforeMutationEffectsDeletion(deletion: Fiber) {
392
}
393
}
394
397
-export function commitPassiveEffectDurations(
398
- finishedRoot: FiberRoot,
399
- finishedWork: Fiber,
400
-): void {
401
- if (
402
- enableProfilerTimer &&
403
- enableProfilerCommitHooks &&
404
- getExecutionContext() & CommitContext
405
- ) {
406
- // Only Profilers with work in their subtree will have an Update effect scheduled.
407
- if ((finishedWork.flags & Update) !== NoFlags) {
408
- switch (finishedWork.tag) {
409
- case Profiler: {
410
- const {passiveEffectDuration} = finishedWork.stateNode;
411
- const {id, onPostCommit} = finishedWork.memoizedProps;
412
-
413
- // This value will still reflect the previous commit phase.
414
- // It does not get reset until the start of the next commit phase.
415
- const commitTime = getCommitTime();
416
-
417
- let phase = finishedWork.alternate === null ? 'mount' : 'update';
418
- if (enableProfilerNestedUpdatePhase) {
419
- if (isCurrentUpdateNested()) {
420
- phase = 'nested-update';
421
- }
422
- }
423
-
424
- if (typeof onPostCommit === 'function') {
425
- onPostCommit(id, phase, passiveEffectDuration, commitTime);
426
- }
427
-
428
- // Bubble times to the next nearest ancestor Profiler.
429
- // After we process that Profiler, we'll bubble further up.
430
- let parentFiber = finishedWork.return;
431
- outer: while (parentFiber !== null) {
432
- switch (parentFiber.tag) {
433
- case HostRoot:
434
- const root = parentFiber.stateNode;
435
- root.passiveEffectDuration += passiveEffectDuration;
436
- break outer;
437
- case Profiler:
438
- const parentStateNode = parentFiber.stateNode;
439
- parentStateNode.passiveEffectDuration += passiveEffectDuration;
440
- break outer;
441
- }
442
- parentFiber = parentFiber.return;
443
- }
444
- break;
445
- }
446
- default:
447
- break;
448
- }
449
- }
450
- }
451
-}
452
-
395
function commitLayoutEffectOnFiber(
396
finishedRoot: FiberRoot,
397
current: Fiber | null,
@@ -557,11 +499,6 @@ function commitLayoutEffectOnFiber(
499
effectDuration,
500
);
501
560
- // Schedule a passive effect for this Profiler to call onPostCommit hooks.
561
- // This effect should be scheduled even if there is no onPostCommit callback for this Profiler,
562
- // because the effect is also where times bubble to parent Profilers.
563
- enqueuePendingPassiveProfilerEffect(finishedWork);
564
-
502
// Propagate layout effect durations to the next nearest Profiler ancestor.
503
// Do not reset these values until the next render so DevTools has a chance to read them first.
504
let parentFiber = finishedWork.return;
@@ -2475,11 +2412,6 @@ export function reappearLayoutEffects(
2412
effectDuration,
2413
);
2414
2478
- // Schedule a passive effect for this Profiler to call onPostCommit hooks.
2479
- // This effect should be scheduled even if there is no onPostCommit callback for this Profiler,
2480
- // because the effect is also where times bubble to parent Profilers.
2481
- enqueuePendingPassiveProfilerEffect(finishedWork);
2482
-
2415
// Propagate layout effect durations to the next nearest Profiler ancestor.
2416
// Do not reset these values until the next render so DevTools has a chance to read them first.
2417
let parentFiber = finishedWork.return;
@@ -2824,6 +2756,52 @@ function commitPassiveMountOnFiber(
2756
}
2757
break;
2758
}
2759
+ case Profiler: {
2760
+ recursivelyTraversePassiveMountEffects(
2761
+ finishedRoot,
2762
+ finishedWork,
2763
+ committedLanes,
2764
+ committedTransitions,
2765
+ );
2766
+
2767
+ // Only Profilers with work in their subtree will have a Passive effect scheduled.
2768
+ if (flags & Passive) {
2769
+ if (
2770
+ enableProfilerTimer &&
2771
+ enableProfilerCommitHooks &&
2772
+ getExecutionContext() & CommitContext
2773
+ ) {
2774
+ const {passiveEffectDuration} = finishedWork.stateNode;
2775
+
2776
+ commitProfilerPostCommit(
2777
+ finishedWork,
2778
+ finishedWork.alternate,
2779
+ // This value will still reflect the previous commit phase.
2780
+ // It does not get reset until the start of the next commit phase.
2781
+ getCommitTime(),
2782
+ passiveEffectDuration,
2783
+ );
2784
+
2785
+ // Bubble times to the next nearest ancestor Profiler.
2786
+ // After we process that Profiler, we'll bubble further up.
2787
+ let parentFiber = finishedWork.return;
2788
+ outer: while (parentFiber !== null) {
2789
+ switch (parentFiber.tag) {
2790
+ case HostRoot:
2791
+ const root = parentFiber.stateNode;
2792
+ root.passiveEffectDuration += passiveEffectDuration;
2793
+ break outer;
2794
+ case Profiler:
2795
+ const parentStateNode = parentFiber.stateNode;
2796
+ parentStateNode.passiveEffectDuration += passiveEffectDuration;
2797
+ break outer;
2798
+ }
2799
+ parentFiber = parentFiber.return;
2800
+ }
2801
+ }
2802
+ }
2803
+ break;
2804
+ }
2805
case LegacyHiddenComponent: {
2806
if (enableLegacyHidden) {
2807
recursivelyTraversePassiveMountEffects(
packages/react-reconciler/src/ReactFiberWorkLoop.js
-25
@@ -189,7 +189,6 @@ import {
189
commitBeforeMutationEffects,
190
commitLayoutEffects,
191
commitMutationEffects,
192
- commitPassiveEffectDurations,
192
commitPassiveMountEffects,
193
commitPassiveUnmountEffects,
194
disappearLayoutEffects,
@@ -580,7 +579,6 @@ let legacyErrorBoundariesThatAlreadyFailed: Set<mixed> | null = null;
579
let rootDoesHavePassiveEffects: boolean = false;
580
let rootWithPendingPassiveEffects: FiberRoot | null = null;
581
let pendingPassiveEffectsLanes: Lanes = NoLanes;
583
-let pendingPassiveProfilerEffects: Array<Fiber> = [];
582
let pendingPassiveEffectsRemainingLanes: Lanes = NoLanes;
583
let pendingPassiveTransitions: Array<Transition> | null = null;
584
@@ -3475,19 +3473,6 @@ export function flushPassiveEffects(): boolean {
3473
return false;
3474
}
3475
3478
-export function enqueuePendingPassiveProfilerEffect(fiber: Fiber): void {
3479
- if (enableProfilerTimer && enableProfilerCommitHooks) {
3480
- pendingPassiveProfilerEffects.push(fiber);
3481
- if (!rootDoesHavePassiveEffects) {
3482
- rootDoesHavePassiveEffects = true;
3483
- scheduleCallback(NormalSchedulerPriority, () => {
3484
- flushPassiveEffects();
3485
- return null;
3486
- });
3487
- }
3488
- }
3489
-}
3490
-
3476
function flushPassiveEffectsImpl() {
3477
if (rootWithPendingPassiveEffects === null) {
3478
return false;
@@ -3528,16 +3513,6 @@ function flushPassiveEffectsImpl() {
3513
commitPassiveUnmountEffects(root.current);
3514
commitPassiveMountEffects(root, root.current, lanes, transitions);
3515
3531
- // TODO: Move to commitPassiveMountEffects
3532
- if (enableProfilerTimer && enableProfilerCommitHooks) {
3533
- const profilerEffects = pendingPassiveProfilerEffects;
3534
- pendingPassiveProfilerEffects = [];
3535
- for (let i = 0; i < profilerEffects.length; i++) {
3536
- const fiber = ((profilerEffects[i]: any): Fiber);
3537
- commitPassiveEffectDurations(root, fiber);
3538
- }
3539
- }
3540
-
3516
if (__DEV__) {
3517
if (enableDebugTracing) {
3518
logPassiveEffectsStopped();