[Fiber] Move Profiler onPostCommit processing of passive effect durat…

…ions 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.
facebook · Sep 16, 2024 · ee1a403 · ee1a403
1 parent 0eab377
commit ee1a403
Show file tree

Hide file tree

Showing 4 changed files with 106 additions and 96 deletions.
diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js
@@ -1026,6 +1026,10 @@ function updateProfiler(
     workInProgress.flags |= Update;
 
     if (enableProfilerCommitHooks) {
+      // 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.
+      workInProgress.flags |= 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,
       const stateNode = workInProgress.stateNode;
@@ -3700,6 +3704,10 @@ function attemptEarlyBailoutIfNoScheduledUpdate(
         }
 
         if (enableProfilerCommitHooks) {
+          // 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.
+          workInProgress.flags |= 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,
           const stateNode = workInProgress.stateNode;

diff --git a/packages/react-reconciler/src/ReactFiberCommitEffects.js b/packages/react-reconciler/src/ReactFiberCommitEffects.js
@@ -881,7 +881,7 @@ function commitProfiler(
   commitTime: number,
   effectDuration: number,
 ) {
-  const {onCommit, onRender} = finishedWork.memoizedProps;
+  const {id, onCommit, onRender} = finishedWork.memoizedProps;
 
   let phase = current === null ? 'mount' : 'update';
   if (enableProfilerNestedUpdatePhase) {
@@ -892,7 +892,7 @@ function commitProfiler(
 
   if (typeof onRender === 'function') {
     onRender(
-      finishedWork.memoizedProps.id,
+      id,
       phase,
       finishedWork.actualDuration,
       finishedWork.treeBaseDuration,
@@ -938,3 +938,52 @@ export function commitProfilerUpdate(
     }
   }
 }
+
+function commitProfilerPostCommitImpl(
+  finishedWork: Fiber,
+  current: Fiber | null,
+  commitTime: number,
+  passiveEffectDuration: number,
+): void {
+  const {id, onPostCommit} = finishedWork.memoizedProps;
+
+  let phase = current === null ? 'mount' : 'update';
+  if (enableProfilerNestedUpdatePhase) {
+    if (isCurrentUpdateNested()) {
+      phase = 'nested-update';
+    }
+  }
+
+  if (typeof onPostCommit === 'function') {
+    onPostCommit(id, phase, passiveEffectDuration, commitTime);
+  }
+}
+
+export function commitProfilerPostCommit(
+  finishedWork: Fiber,
+  current: Fiber | null,
+  commitTime: number,
+  passiveEffectDuration: number,
+) {
+  try {
+    if (__DEV__) {
+      runWithFiberInDEV(
+        finishedWork,
+        commitProfilerPostCommitImpl,
+        finishedWork,
+        current,
+        commitTime,
+        passiveEffectDuration,
+      );
+    } else {
+      commitProfilerPostCommitImpl(
+        finishedWork,
+        current,
+        commitTime,
+        passiveEffectDuration,
+      );
+    }
+  } catch (error) {
+    captureCommitPhaseError(finishedWork, finishedWork.return, error);
+  }
+}
diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js
@@ -44,7 +44,6 @@ import {
   enablePersistedModeClonedFlag,
   enableProfilerTimer,
   enableProfilerCommitHooks,
-  enableProfilerNestedUpdatePhase,
   enableSchedulingProfiler,
   enableSuspenseCallback,
   enableScopeAPI,
@@ -100,7 +99,6 @@ import {
   Cloned,
 } from './ReactFiberFlags';
 import {
-  isCurrentUpdateNested,
   getCommitTime,
   recordLayoutEffectDuration,
   startLayoutEffectTimer,
@@ -137,7 +135,6 @@ import {
   captureCommitPhaseError,
   resolveRetryWakeable,
   markCommitTimeOfFallback,
-  enqueuePendingPassiveProfilerEffect,
   restorePendingUpdaters,
   addTransitionStartCallbackToPendingTransition,
   addTransitionProgressCallbackToPendingTransition,
@@ -193,6 +190,7 @@ import {
   safelyDetachRef,
   safelyCallDestroy,
   commitProfilerUpdate,
+  commitProfilerPostCommit,
   commitRootCallbacks,
 } from './ReactFiberCommitEffects';
 import {
@@ -394,62 +392,6 @@ function commitBeforeMutationEffectsDeletion(deletion: Fiber) {
   }
 }
 
-export function commitPassiveEffectDurations(
-  finishedRoot: FiberRoot,
-  finishedWork: Fiber,
-): void {
-  if (
-    enableProfilerTimer &&
-    enableProfilerCommitHooks &&
-    getExecutionContext() & CommitContext
-  ) {
-    // 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();
-
-          let phase = finishedWork.alternate === null ? 'mount' : 'update';
-          if (enableProfilerNestedUpdatePhase) {
-            if (isCurrentUpdateNested()) {
-              phase = 'nested-update';
-            }
-          }
-
-          if (typeof onPostCommit === 'function') {
-            onPostCommit(id, phase, passiveEffectDuration, commitTime);
-          }
-
-          // Bubble times to the next nearest ancestor Profiler.
-          // After we process that Profiler, we'll bubble further up.
-          let parentFiber = finishedWork.return;
-          outer: while (parentFiber !== null) {
-            switch (parentFiber.tag) {
-              case HostRoot:
-                const root = parentFiber.stateNode;
-                root.passiveEffectDuration += passiveEffectDuration;
-                break outer;
-              case Profiler:
-                const parentStateNode = parentFiber.stateNode;
-                parentStateNode.passiveEffectDuration += passiveEffectDuration;
-                break outer;
-            }
-            parentFiber = parentFiber.return;
-          }
-          break;
-        }
-        default:
-          break;
-      }
-    }
-  }
-}
-
 function commitLayoutEffectOnFiber(
   finishedRoot: FiberRoot,
   current: Fiber | null,
@@ -557,11 +499,6 @@ function commitLayoutEffectOnFiber(
           effectDuration,
         );
 
-        // 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.
         let parentFiber = finishedWork.return;
@@ -2475,11 +2412,6 @@ export function reappearLayoutEffects(
           effectDuration,
         );
 
-        // 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.
         let parentFiber = finishedWork.return;
@@ -2824,6 +2756,52 @@ function commitPassiveMountOnFiber(
       }
       break;
     }
+    case Profiler: {
+      recursivelyTraversePassiveMountEffects(
+        finishedRoot,
+        finishedWork,
+        committedLanes,
+        committedTransitions,
+      );
+
+      // Only Profilers with work in their subtree will have a Passive effect scheduled.
+      if (flags & Passive) {
+        if (
+          enableProfilerTimer &&
+          enableProfilerCommitHooks &&
+          getExecutionContext() & CommitContext
+        ) {
+          const {passiveEffectDuration} = finishedWork.stateNode;
+
+          commitProfilerPostCommit(
+            finishedWork,
+            finishedWork.alternate,
+            // This value will still reflect the previous commit phase.
+            // It does not get reset until the start of the next commit phase.
+            getCommitTime(),
+            passiveEffectDuration,
+          );
+
+          // Bubble times to the next nearest ancestor Profiler.
+          // After we process that Profiler, we'll bubble further up.
+          let parentFiber = finishedWork.return;
+          outer: while (parentFiber !== null) {
+            switch (parentFiber.tag) {
+              case HostRoot:
+                const root = parentFiber.stateNode;
+                root.passiveEffectDuration += passiveEffectDuration;
+                break outer;
+              case Profiler:
+                const parentStateNode = parentFiber.stateNode;
+                parentStateNode.passiveEffectDuration += passiveEffectDuration;
+                break outer;
+            }
+            parentFiber = parentFiber.return;
+          }
+        }
+      }
+      break;
+    }
     case LegacyHiddenComponent: {
       if (enableLegacyHidden) {
         recursivelyTraversePassiveMountEffects(

diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js
@@ -189,7 +189,6 @@ import {
   commitBeforeMutationEffects,
   commitLayoutEffects,
   commitMutationEffects,
-  commitPassiveEffectDurations,
   commitPassiveMountEffects,
   commitPassiveUnmountEffects,
   disappearLayoutEffects,
@@ -580,7 +579,6 @@ let legacyErrorBoundariesThatAlreadyFailed: Set<mixed> | null = null;
 let rootDoesHavePassiveEffects: boolean = false;
 let rootWithPendingPassiveEffects: FiberRoot | null = null;
 let pendingPassiveEffectsLanes: Lanes = NoLanes;
-let pendingPassiveProfilerEffects: Array<Fiber> = [];
 let pendingPassiveEffectsRemainingLanes: Lanes = NoLanes;
 let pendingPassiveTransitions: Array<Transition> | null = null;
 
@@ -3475,19 +3473,6 @@ 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 flushPassiveEffectsImpl() {
   if (rootWithPendingPassiveEffects === null) {
     return false;
@@ -3528,16 +3513,6 @@ function flushPassiveEffectsImpl() {
   commitPassiveUnmountEffects(root.current);
   commitPassiveMountEffects(root, root.current, lanes, transitions);
 
-  // TODO: Move to commitPassiveMountEffects
-  if (enableProfilerTimer && enableProfilerCommitHooks) {
-    const profilerEffects = pendingPassiveProfilerEffects;
-    pendingPassiveProfilerEffects = [];
-    for (let i = 0; i < profilerEffects.length; i++) {
-      const fiber = ((profilerEffects[i]: any): Fiber);
-      commitPassiveEffectDurations(root, fiber);
-    }
-  }
-
   if (__DEV__) {
     if (enableDebugTracing) {
       logPassiveEffectsStopped();