Use Passive flag to schedule onPostCommit (#19862)

acdlite · web-flow · commit 8b2d3783e58d · 2020-09-18T13:02:26.000-07:00
Instead of calling `onPostCommit` in a separate phase, we can fire
them during the same traversal as the rest of the passive effects.

This works because effects are executed depth-first. So by the time we
reach a Profiler node, we'll have already executed all the effects in
its subtree.
diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js
@@ -61,6 +61,7 @@ import {
   ContentReset,
   DidCapture,
   Update,
+  Passive,
   Ref,
   Deletion,
   ForceUpdateForLegacySuspense,
@@ -674,7 +675,8 @@ function updateProfiler(
   renderLanes: Lanes,
 ) {
   if (enableProfilerTimer) {
-    workInProgress.flags |= Update;
+    // TODO: Only call onRender et al if subtree has effects
+    workInProgress.flags |= Update | 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,
@@ -3116,12 +3118,13 @@ function beginWork(
         case Profiler:
           if (enableProfilerTimer) {
             // Profiler should only call onRender when one of its descendants actually rendered.
+            // TODO: Only call onRender et al if subtree has effects
             const hasChildWork = includesSomeLane(
               renderLanes,
               workInProgress.childLanes,
             );
             if (hasChildWork) {
-              workInProgress.flags |= Update;
+              workInProgress.flags |= Passive | Update;
             }
 
             // Reset effect durations for the next eventual effect phase.
diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js
@@ -121,7 +121,6 @@ import {
   captureCommitPhaseError,
   resolveRetryWakeable,
   markCommitTimeOfFallback,
-  enqueuePendingPassiveProfilerEffect,
   schedulePassiveEffectCallback,
 } from './ReactFiberWorkLoop.new';
 import {
@@ -507,57 +506,55 @@ function commitHookEffectListMount2(fiber: Fiber): void {
   }
 }
 
-export function commitPassiveEffectDurations(
+function commitProfilerPassiveEffect(
   finishedRoot: FiberRoot,
   finishedWork: Fiber,
 ): void {
   if (enableProfilerTimer && enableProfilerCommitHooks) {
-    // 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();
-
-          if (typeof onPostCommit === 'function') {
-            if (enableSchedulerTracing) {
-              onPostCommit(
-                id,
-                finishedWork.alternate === null ? 'mount' : 'update',
-                passiveEffectDuration,
-                commitTime,
-                finishedRoot.memoizedInteractions,
-              );
-            } else {
-              onPostCommit(
-                id,
-                finishedWork.alternate === null ? 'mount' : 'update',
-                passiveEffectDuration,
-                commitTime,
-              );
-            }
+    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();
+
+        if (typeof onPostCommit === 'function') {
+          if (enableSchedulerTracing) {
+            onPostCommit(
+              id,
+              finishedWork.alternate === null ? 'mount' : 'update',
+              passiveEffectDuration,
+              commitTime,
+              finishedRoot.memoizedInteractions,
+            );
+          } else {
+            onPostCommit(
+              id,
+              finishedWork.alternate === null ? 'mount' : 'update',
+              passiveEffectDuration,
+              commitTime,
+            );
           }
+        }
 
-          // Bubble times to the next nearest ancestor Profiler.
-          // After we process that Profiler, we'll bubble further up.
-          let parentFiber = finishedWork.return;
-          while (parentFiber !== null) {
-            if (parentFiber.tag === Profiler) {
-              const parentStateNode = parentFiber.stateNode;
-              parentStateNode.passiveEffectDuration += passiveEffectDuration;
-              break;
-            }
-            parentFiber = parentFiber.return;
+        // Bubble times to the next nearest ancestor Profiler.
+        // After we process that Profiler, we'll bubble further up.
+        // TODO: Use JS Stack instead
+        let parentFiber = finishedWork.return;
+        while (parentFiber !== null) {
+          if (parentFiber.tag === Profiler) {
+            const parentStateNode = parentFiber.stateNode;
+            parentStateNode.passiveEffectDuration += passiveEffectDuration;
+            break;
           }
-          break;
+          parentFiber = parentFiber.return;
         }
-        default:
-          break;
+        break;
       }
+      default:
+        break;
     }
   }
 }
@@ -841,13 +838,9 @@ function commitLifeCycles(
             }
           }
 
-          // 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.
+          // TODO: Use JS Stack instead
           let parentFiber = finishedWork.return;
           while (parentFiber !== null) {
             if (parentFiber.tag === Profiler) {
@@ -1912,6 +1905,7 @@ function commitPassiveWork(finishedWork: Fiber): void {
         finishedWork,
         finishedWork.return,
       );
+      break;
     }
   }
 }
@@ -1933,13 +1927,21 @@ function commitPassiveUnmount(
   }
 }
 
-function commitPassiveLifeCycles(finishedWork: Fiber): void {
+function commitPassiveLifeCycles(
+  finishedRoot: FiberRoot,
+  finishedWork: Fiber,
+): void {
   switch (finishedWork.tag) {
     case FunctionComponent:
     case ForwardRef:
     case SimpleMemoComponent:
     case Block: {
       commitHookEffectListMount2(finishedWork);
+      break;
+    }
+    case Profiler: {
+      commitProfilerPassiveEffect(finishedRoot, finishedWork);
+      break;
     }
   }
 }
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
@@ -21,7 +21,6 @@ import {
   enableSuspenseServerRenderer,
   replayFailedUnitOfWorkWithInvokeGuardedCallback,
   enableProfilerTimer,
-  enableProfilerCommitHooks,
   enableSchedulerTracing,
   warnAboutUnmockedScheduler,
   deferRenderPhaseUpdateToNextBatch,
@@ -197,7 +196,6 @@ import {
   commitPassiveLifeCycles as commitPassiveEffectOnFiber,
   commitDetachRef,
   commitAttachRef,
-  commitPassiveEffectDurations,
   commitResetTextContent,
   isSuspenseBoundaryBeingHidden,
 } from './ReactFiberCommitWork.new';
@@ -336,7 +334,6 @@ let rootDoesHavePassiveEffects: boolean = false;
 let rootWithPendingPassiveEffects: FiberRoot | null = null;
 let pendingPassiveEffectsRenderPriority: ReactPriorityLevel = NoSchedulerPriority;
 let pendingPassiveEffectsLanes: Lanes = NoLanes;
-let pendingPassiveProfilerEffects: Array<Fiber> = [];
 
 let rootsWithPendingDiscreteUpdates: Set<FiberRoot> | null = null;
 
@@ -2451,31 +2448,18 @@ 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 flushPassiveMountEffects(firstChild: Fiber): void {
+function flushPassiveMountEffects(root, firstChild: Fiber): void {
   let fiber = firstChild;
   while (fiber !== null) {
     const primarySubtreeFlags = fiber.subtreeFlags & PassiveMask;
 
     if (fiber.child !== null && primarySubtreeFlags !== NoFlags) {
-      flushPassiveMountEffects(fiber.child);
+      flushPassiveMountEffects(root, fiber.child);
     }
 
     if ((fiber.flags & Passive) !== NoFlags) {
       setCurrentDebugFiberInDEV(fiber);
-      commitPassiveEffectOnFiber(fiber);
+      commitPassiveEffectOnFiber(root, fiber);
       resetCurrentDebugFiberInDEV();
     }
 
@@ -2586,16 +2570,7 @@ function flushPassiveEffectsImpl() {
   // value set by a create function in another component.
   // Layout effects have the same constraint.
   flushPassiveUnmountEffects(root.current);
-  flushPassiveMountEffects(root.current);
-
-  if (enableProfilerTimer && enableProfilerCommitHooks) {
-    const profilerEffects = pendingPassiveProfilerEffects;
-    pendingPassiveProfilerEffects = [];
-    for (let i = 0; i < profilerEffects.length; i++) {
-      const fiber = ((profilerEffects[i]: any): Fiber);
-      commitPassiveEffectDurations(root, fiber);
-    }
-  }
+  flushPassiveMountEffects(root, root.current);
 
   if (enableSchedulerTracing) {
     popInteractions(((prevInteractions: any): Set<Interaction>));
