Bubble profiler durations to parent (rather than walking children)

Note this commit fails 5 tests. I am pushing it purely for sharing purposes.

Author: Brian Vaughn

packages/react-reconciler/src/ReactFiber.new.js

@@ -293,6 +293,10 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber {
       // But works for yielding (the common case) and should support resuming.
       workInProgress.actualDuration = 0;
       workInProgress.actualStartTime = -1;
+
+      // Reset treeBaseDuration when cloning.
+      // As each child completes rendering, its base durations will bubble up to the parent.
+      workInProgress.treeBaseDuration = 0;
     }
   }
 
@@ -323,11 +327,6 @@ export function createWorkInProgress(current: Fiber, pendingProps: any): Fiber {
   workInProgress.index = current.index;
   workInProgress.ref = current.ref;
 
-  if (enableProfilerTimer) {
-    workInProgress.selfBaseDuration = current.selfBaseDuration;
-    workInProgress.treeBaseDuration = current.treeBaseDuration;
-  }
-
   if (__DEV__) {
     workInProgress._debugNeedsRemount = current._debugNeedsRemount;
     switch (workInProgress.tag) {
@@ -381,8 +380,8 @@ export function resetWorkInProgress(workInProgress: Fiber, renderLanes: Lanes) {
     workInProgress.stateNode = null;
 
     if (enableProfilerTimer) {
-      // Note: We don't reset the actualTime counts. It's useful to accumulate
-      // actual time across multiple render passes.
+      // Note: We don't reset the actualTime counts.
+      // It's useful to accumulate actual time across multiple render passes.
       workInProgress.selfBaseDuration = 0;
       workInProgress.treeBaseDuration = 0;
     }
@@ -412,10 +411,10 @@ export function resetWorkInProgress(workInProgress: Fiber, renderLanes: Lanes) {
           };
 
     if (enableProfilerTimer) {
-      // Note: We don't reset the actualTime counts. It's useful to accumulate
-      // actual time across multiple render passes.
-      workInProgress.selfBaseDuration = current.selfBaseDuration;
-      workInProgress.treeBaseDuration = current.treeBaseDuration;
+      // Note: We don't reset the actualTime counts.
+      // It's useful to accumulate actual time across multiple render passes.
+      workInProgress.selfBaseDuration = 0;
+      workInProgress.treeBaseDuration = 0;
     }
   }
 

packages/react-reconciler/src/ReactFiberBeginWork.new.js

@@ -1714,6 +1714,15 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) {
       renderLanes,
     )
   ) {
+    if (enableProfilerTimer) {
+      if ((workInProgress.mode & ProfileMode) !== NoMode) {
+        if (current !== null) {
+          // TODO (effects) Document
+          workInProgress.actualDuration = current.actualDuration;
+        }
+      }
+    }
+
     // Something in this boundary's subtree already suspended. Switch to
     // rendering the fallback children.
     showFallback = true;
@@ -1990,10 +1999,9 @@ function mountSuspenseFallbackChildren(
     primaryChildFragment.pendingProps = primaryChildProps;
 
     if (enableProfilerTimer && workInProgress.mode & ProfileMode) {
-      // Reset the durations from the first pass so they aren't included in the
-      // final amounts. This seems counterintuitive, since we're intentionally
-      // not measuring part of the render phase, but this makes it match what we
-      // do in Concurrent Mode.
+      // Reset the durations from the first pass so they aren't included in the final amounts.
+      // This seems counterintuitive, since we're intentionally not measuring part of the render phase,
+      // but this makes it match what we do in Concurrent Mode.
       primaryChildFragment.actualDuration = 0;
       primaryChildFragment.actualStartTime = -1;
       primaryChildFragment.selfBaseDuration = 0;
@@ -2111,10 +2119,9 @@ function updateSuspenseFallbackChildren(
     primaryChildFragment.pendingProps = primaryChildProps;
 
     if (enableProfilerTimer && workInProgress.mode & ProfileMode) {
-      // Reset the durations from the first pass so they aren't included in the
-      // final amounts. This seems counterintuitive, since we're intentionally
-      // not measuring part of the render phase, but this makes it match what we
-      // do in Concurrent Mode.
+      // Reset the durations from the first pass so they aren't included in the final amounts.
+      // This seems counterintuitive, since we're intentionally not measuring part of the render phase,
+      // but this makes it match what we do in Concurrent Mode.
       primaryChildFragment.actualDuration = 0;
       primaryChildFragment.actualStartTime = -1;
       primaryChildFragment.selfBaseDuration =
@@ -2970,6 +2977,13 @@ function bailoutOnAlreadyFinishedWork(
 
   // Check if the children have any pending work.
   if (!includesSomeLane(renderLanes, workInProgress.childLanes)) {
+    if (enableProfilerTimer) {
+      if (current !== null) {
+        workInProgress.treeBaseDuration = current.treeBaseDuration;
+        workInProgress.selfBaseDuration = 0;
+      }
+    }
+
     // The children don't have any work either. We can skip them.
     // TODO: Once we add back resuming, we should check if the children are
     // a work-in-progress set. If so, we need to transfer their effects.

packages/react-reconciler/src/ReactFiberCompleteWork.new.js

@@ -71,6 +71,7 @@ import {
   DidCapture,
   Snapshot,
   MutationMask,
+  PerformedWork,
   StaticMask,
 } from './ReactFiberFlags';
 import invariant from 'shared/invariant';
@@ -155,7 +156,10 @@ import {
 } from './ReactFiberLane';
 import {resetChildFibers} from './ReactChildFiber.new';
 import {createScopeInstance} from './ReactFiberScope.new';
-import {transferActualDuration} from './ReactProfilerTimer.new';
+import {
+  stopProfilerTimerIfRunning,
+  stopProfilerTimerIfRunningAndRecordDelta,
+} from './ReactProfilerTimer.new';
 
 function markUpdate(workInProgress: Fiber) {
   // Tag the fiber with an update effect. This turns a Placement into
@@ -681,112 +685,99 @@ function cutOffTailIfNeeded(
   }
 }
 
-function bubbleProperties(completedWork: Fiber) {
-  const didBailout =
-    completedWork.alternate !== null &&
-    completedWork.alternate.child === completedWork.child;
+export function bubbleProfilerDurationsAfterError(erroredWork: Fiber): void {
+  if (enableProfilerTimer) {
+    if ((erroredWork.mode & ProfileMode) !== NoMode) {
+      const parent = erroredWork.return;
+      if (parent !== null) {
+        // TODO (effects) Document
+        parent.actualDuration += erroredWork.actualDuration;
 
-  let newChildLanes = NoLanes;
-  let subtreeFlags = NoFlags;
+        // TODO (effects) Document
+        parent.treeBaseDuration = 0;
+      }
+    }
+  }
+}
 
-  if (!didBailout) {
-    // Bubble up the earliest expiration time.
-    if (enableProfilerTimer && (completedWork.mode & ProfileMode) !== NoMode) {
-      // In profiling mode, resetChildExpirationTime is also used to reset
-      // profiler durations.
-      let actualDuration = completedWork.actualDuration;
-      let treeBaseDuration = ((completedWork.selfBaseDuration: any): number);
-
-      let child = completedWork.child;
-      while (child !== null) {
-        newChildLanes = mergeLanes(
-          newChildLanes,
-          mergeLanes(child.lanes, child.childLanes),
-        );
+// TODO (effects) Temorary method; replace with bubblePropertiesToParent
+function bubbleProperties(completedWork: Fiber): void {
+  bubblePropertiesFromChildren(completedWork);
+  bubblePropertiesToParent(completedWork);
+}
 
-        subtreeFlags |= child.subtreeFlags;
-        subtreeFlags |= child.flags;
+function bubblePropertiesToParent(completedWork: Fiber): void {
+  const parent = completedWork.return;
+  if (parent !== null) {
+    const didBailout = (completeWork.flags & PerformedWork) !== NoFlags;
+    if (!didBailout) {
+      if (enableProfilerTimer) {
+        if ((completedWork.mode & ProfileMode) !== NoMode) {
+          stopProfilerTimerIfRunningAndRecordDelta(completedWork, true);
 
-        // When a fiber is cloned, its actualDuration is reset to 0. This value will
-        // only be updated if work is done on the fiber (i.e. it doesn't bailout).
-        // When work is done, it should bubble to the parent's actualDuration. If
-        // the fiber has not been cloned though, (meaning no work was done), then
-        // this value will reflect the amount of time spent working on a previous
-        // render. In that case it should not bubble. We determine whether it was
-        // cloned by comparing the child pointer.
-        actualDuration += child.actualDuration;
+          // At this point child base durations have already bubbled up to treeBaseDuration.
+          // Add our own base duration before bubbling further to the parent Fiber.
+          completedWork.treeBaseDuration += completedWork.selfBaseDuration;
 
-        treeBaseDuration += child.treeBaseDuration;
-        child = child.sibling;
+          // Bubble base durations to the parent.
+          parent.actualDuration += completedWork.actualDuration;
+          parent.treeBaseDuration += completedWork.treeBaseDuration;
+        }
       }
-
-      completedWork.actualDuration = actualDuration;
-      completedWork.treeBaseDuration = treeBaseDuration;
     } else {
-      let child = completedWork.child;
-      while (child !== null) {
-        newChildLanes = mergeLanes(
-          newChildLanes,
-          mergeLanes(child.lanes, child.childLanes),
-        );
-
-        subtreeFlags |= child.subtreeFlags;
-        subtreeFlags |= child.flags;
+      if (enableProfilerTimer) {
+        if ((completedWork.mode & ProfileMode) !== NoMode) {
+          stopProfilerTimerIfRunning(completedWork);
 
-        child = child.sibling;
+          parent.treeBaseDuration += completedWork.treeBaseDuration;
+        }
       }
     }
+  }
+}
 
-    completedWork.subtreeFlags |= subtreeFlags;
-  } else {
-    // Bubble up the earliest expiration time.
-    if (enableProfilerTimer && (completedWork.mode & ProfileMode) !== NoMode) {
-      // In profiling mode, resetChildExpirationTime is also used to reset
-      // profiler durations.
-      let treeBaseDuration = ((completedWork.selfBaseDuration: any): number);
-
-      let child = completedWork.child;
-      while (child !== null) {
-        newChildLanes = mergeLanes(
-          newChildLanes,
-          mergeLanes(child.lanes, child.childLanes),
-        );
+// TODO (effects) Move everything in this method into bubblePropertiesToParent
+function bubblePropertiesFromChildren(completedWork: Fiber): void {
+  let newChildLanes = NoLanes;
+  let subtreeFlags = NoFlags;
 
-        // "Static" flags share the lifetime of the fiber/hook they belong to,
-        // so we should bubble those up even during a bailout. All the other
-        // flags have a lifetime only of a single render + commit, so we should
-        // ignore them.
-        subtreeFlags |= child.subtreeFlags & StaticMask;
-        subtreeFlags |= child.flags & StaticMask;
+  const didBailout =
+    completedWork.alternate !== null &&
+    completedWork.alternate.child === completedWork.child;
+  if (!didBailout) {
+    let child = completedWork.child;
+    while (child !== null) {
+      newChildLanes = mergeLanes(
+        newChildLanes,
+        mergeLanes(child.lanes, child.childLanes),
+      );
 
-        treeBaseDuration += child.treeBaseDuration;
-        child = child.sibling;
-      }
+      subtreeFlags |= child.subtreeFlags;
+      subtreeFlags |= child.flags;
 
-      completedWork.treeBaseDuration = treeBaseDuration;
-    } else {
-      let child = completedWork.child;
-      while (child !== null) {
-        newChildLanes = mergeLanes(
-          newChildLanes,
-          mergeLanes(child.lanes, child.childLanes),
-        );
+      child = child.sibling;
+    }
+  } else {
+    let child = completedWork.child;
+    while (child !== null) {
+      newChildLanes = mergeLanes(
+        newChildLanes,
+        mergeLanes(child.lanes, child.childLanes),
+      );
 
-        // "Static" flags share the lifetime of the fiber/hook they belong to,
-        // so we should bubble those up even during a bailout. All the other
-        // flags have a lifetime only of a single render + commit, so we should
-        // ignore them.
-        subtreeFlags |= child.subtreeFlags & StaticMask;
-        subtreeFlags |= child.flags & StaticMask;
+      // "Static" flags share the lifetime of the fiber/hook they belong to,
+      // so we should bubble those up even during a bailout. All the other
+      // flags have a lifetime only of a single render + commit, so we should
+      // ignore them.
+      subtreeFlags |= child.subtreeFlags & StaticMask;
+      subtreeFlags |= child.flags & StaticMask;
 
-        child = child.sibling;
-      }
+      child = child.sibling;
     }
-
-    completedWork.subtreeFlags |= subtreeFlags;
   }
 
   completedWork.childLanes = newChildLanes;
+  completedWork.subtreeFlags |= subtreeFlags;
 }
 
 function completeWork(
@@ -1036,12 +1027,6 @@ function completeWork(
         // Something suspended. Re-render with the fallback children.
         workInProgress.lanes = renderLanes;
         // Do not reset the effect list.
-        if (
-          enableProfilerTimer &&
-          (workInProgress.mode & ProfileMode) !== NoMode
-        ) {
-          transferActualDuration(workInProgress);
-        }
         return workInProgress;
       }
 

packages/react-reconciler/src/ReactFiberUnwindWork.new.js

@@ -24,11 +24,7 @@ import {
   LegacyHiddenComponent,
 } from './ReactWorkTags';
 import {DidCapture, NoFlags, ShouldCapture} from './ReactFiberFlags';
-import {NoMode, ProfileMode} from './ReactTypeOfMode';
-import {
-  enableSuspenseServerRenderer,
-  enableProfilerTimer,
-} from 'shared/ReactFeatureFlags';
+import {enableSuspenseServerRenderer} from 'shared/ReactFeatureFlags';
 
 import {popHostContainer, popHostContext} from './ReactFiberHostContext.new';
 import {popSuspenseContext} from './ReactFiberSuspenseContext.new';
@@ -40,7 +36,6 @@ import {
 } from './ReactFiberContext.new';
 import {popProvider} from './ReactFiberNewContext.new';
 import {popRenderLanes} from './ReactFiberWorkLoop.new';
-import {transferActualDuration} from './ReactProfilerTimer.new';
 
 import invariant from 'shared/invariant';
 
@@ -54,12 +49,6 @@ function unwindWork(workInProgress: Fiber, renderLanes: Lanes) {
       const flags = workInProgress.flags;
       if (flags & ShouldCapture) {
         workInProgress.flags = (flags & ~ShouldCapture) | DidCapture;
-        if (
-          enableProfilerTimer &&
-          (workInProgress.mode & ProfileMode) !== NoMode
-        ) {
-          transferActualDuration(workInProgress);
-        }
         return workInProgress;
       }
       return null;
@@ -100,12 +89,6 @@ function unwindWork(workInProgress: Fiber, renderLanes: Lanes) {
       if (flags & ShouldCapture) {
         workInProgress.flags = (flags & ~ShouldCapture) | DidCapture;
         // Captured a suspense effect. Re-render the boundary.
-        if (
-          enableProfilerTimer &&
-          (workInProgress.mode & ProfileMode) !== NoMode
-        ) {
-          transferActualDuration(workInProgress);
-        }
         return workInProgress;
       }
       return null;