[Transition Tracing] Code Cleanup (#24880)

This PR cleans up some of the transition tracing code by:
* Looping through marker transitions only when we process the markerComplete callback (rather than in the commit phase) so we block for less time during commit.
* Renaming `PendingSuspenseBoundaries` to `pendingBoundaries`
* Cleaning up the callback functions

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

@@ -770,7 +770,7 @@ export function createFiberFromTracingMarker(
   fiber.lanes = lanes;
   const tracingMarkerInstance: TracingMarkerInstance = {
     transitions: null,
-    pendingSuspenseBoundaries: null,
+    pendingBoundaries: null,
   };
   fiber.stateNode = tracingMarkerInstance;
   return fiber;

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

@@ -770,7 +770,7 @@ export function createFiberFromTracingMarker(
   fiber.lanes = lanes;
   const tracingMarkerInstance: TracingMarkerInstance = {
     transitions: null,
-    pendingSuspenseBoundaries: null,
+    pendingBoundaries: null,
   };
   fiber.stateNode = tracingMarkerInstance;
   return fiber;

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

@@ -977,7 +977,7 @@ function updateTracingMarkerComponent(
     if (currentTransitions !== null) {
       const markerInstance: TracingMarkerInstance = {
         transitions: new Set(currentTransitions),
-        pendingSuspenseBoundaries: new Map(),
+        pendingBoundaries: new Map(),
         name: workInProgress.pendingProps.name,
       };
       workInProgress.stateNode = markerInstance;

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

@@ -977,7 +977,7 @@ function updateTracingMarkerComponent(
     if (currentTransitions !== null) {
       const markerInstance: TracingMarkerInstance = {
         transitions: new Set(currentTransitions),
-        pendingSuspenseBoundaries: new Map(),
+        pendingBoundaries: new Map(),
         name: workInProgress.pendingProps.name,
       };
       workInProgress.stateNode = markerInstance;

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

@@ -1210,7 +1210,7 @@ function reappearLayoutEffectsOnFiber(node: Fiber) {
 function commitTransitionProgress(offscreenFiber: Fiber) {
   if (enableTransitionTracing) {
     // This function adds suspense boundaries to the root
-    // or tracing marker's pendingSuspenseBoundaries map.
+    // or tracing marker's pendingBoundaries map.
     // When a suspense boundary goes from a resolved to a fallback
     // state we add the boundary to the map, and when it goes from
     // a fallback to a resolved state, we remove the boundary from
@@ -1250,7 +1250,7 @@ function commitTransitionProgress(offscreenFiber: Fiber) {
       // to the pending boundary set if it's there
       if (pendingMarkers !== null) {
         pendingMarkers.forEach(markerInstance => {
-          const pendingBoundaries = markerInstance.pendingSuspenseBoundaries;
+          const pendingBoundaries = markerInstance.pendingBoundaries;
           const transitions = markerInstance.transitions;
           if (
             pendingBoundaries !== null &&
@@ -1284,7 +1284,7 @@ function commitTransitionProgress(offscreenFiber: Fiber) {
       // if it's there
       if (pendingMarkers !== null) {
         pendingMarkers.forEach(markerInstance => {
-          const pendingBoundaries = markerInstance.pendingSuspenseBoundaries;
+          const pendingBoundaries = markerInstance.pendingBoundaries;
           const transitions = markerInstance.transitions;
           if (
             pendingBoundaries !== null &&
@@ -2977,7 +2977,7 @@ function commitPassiveMountOnFiber(
         }
 
         incompleteTransitions.forEach((markerInstance, transition) => {
-          const pendingBoundaries = markerInstance.pendingSuspenseBoundaries;
+          const pendingBoundaries = markerInstance.pendingBoundaries;
           if (pendingBoundaries === null || pendingBoundaries.size === 0) {
             addTransitionCompleteCallbackToPendingTransition(transition);
             incompleteTransitions.delete(transition);
@@ -3052,8 +3052,8 @@ function commitPassiveMountOnFiber(
                     if (instance.transitions === null) {
                       instance.transitions = new Set();
                     } else if (instance.transitions.has(transition)) {
-                      if (markerInstance.pendingSuspenseBoundaries === null) {
-                        markerInstance.pendingSuspenseBoundaries = new Map();
+                      if (markerInstance.pendingBoundaries === null) {
+                        markerInstance.pendingBoundaries = new Map();
                       }
                       if (instance.pendingMarkers === null) {
                         instance.pendingMarkers = new Set();
@@ -3103,17 +3103,15 @@ function commitPassiveMountOnFiber(
         const instance = finishedWork.stateNode;
         if (
           instance.transitions !== null &&
-          (instance.pendingSuspenseBoundaries === null ||
-            instance.pendingSuspenseBoundaries.size === 0)
+          (instance.pendingBoundaries === null ||
+            instance.pendingBoundaries.size === 0)
         ) {
-          instance.transitions.forEach(transition => {
-            addMarkerCompleteCallbackToPendingTransition({
-              transition,
-              name: finishedWork.memoizedProps.name,
-            });
-          });
+          addMarkerCompleteCallbackToPendingTransition(
+            finishedWork.memoizedProps.name,
+            instance.transitions,
+          );
           instance.transitions = null;
-          instance.pendingSuspenseBoundaries = null;
+          instance.pendingBoundaries = null;
         }
       }
       break;

packages/react-reconciler/src/ReactFiberCommitWork.old.js

@@ -1210,7 +1210,7 @@ function reappearLayoutEffectsOnFiber(node: Fiber) {
 function commitTransitionProgress(offscreenFiber: Fiber) {
   if (enableTransitionTracing) {
     // This function adds suspense boundaries to the root
-    // or tracing marker's pendingSuspenseBoundaries map.
+    // or tracing marker's pendingBoundaries map.
     // When a suspense boundary goes from a resolved to a fallback
     // state we add the boundary to the map, and when it goes from
     // a fallback to a resolved state, we remove the boundary from
@@ -1250,7 +1250,7 @@ function commitTransitionProgress(offscreenFiber: Fiber) {
       // to the pending boundary set if it's there
       if (pendingMarkers !== null) {
         pendingMarkers.forEach(markerInstance => {
-          const pendingBoundaries = markerInstance.pendingSuspenseBoundaries;
+          const pendingBoundaries = markerInstance.pendingBoundaries;
           const transitions = markerInstance.transitions;
           if (
             pendingBoundaries !== null &&
@@ -1284,7 +1284,7 @@ function commitTransitionProgress(offscreenFiber: Fiber) {
       // if it's there
       if (pendingMarkers !== null) {
         pendingMarkers.forEach(markerInstance => {
-          const pendingBoundaries = markerInstance.pendingSuspenseBoundaries;
+          const pendingBoundaries = markerInstance.pendingBoundaries;
           const transitions = markerInstance.transitions;
           if (
             pendingBoundaries !== null &&
@@ -2977,7 +2977,7 @@ function commitPassiveMountOnFiber(
         }
 
         incompleteTransitions.forEach((markerInstance, transition) => {
-          const pendingBoundaries = markerInstance.pendingSuspenseBoundaries;
+          const pendingBoundaries = markerInstance.pendingBoundaries;
           if (pendingBoundaries === null || pendingBoundaries.size === 0) {
             addTransitionCompleteCallbackToPendingTransition(transition);
             incompleteTransitions.delete(transition);
@@ -3052,8 +3052,8 @@ function commitPassiveMountOnFiber(
                     if (instance.transitions === null) {
                       instance.transitions = new Set();
                     } else if (instance.transitions.has(transition)) {
-                      if (markerInstance.pendingSuspenseBoundaries === null) {
-                        markerInstance.pendingSuspenseBoundaries = new Map();
+                      if (markerInstance.pendingBoundaries === null) {
+                        markerInstance.pendingBoundaries = new Map();
                       }
                       if (instance.pendingMarkers === null) {
                         instance.pendingMarkers = new Set();
@@ -3103,17 +3103,15 @@ function commitPassiveMountOnFiber(
         const instance = finishedWork.stateNode;
         if (
           instance.transitions !== null &&
-          (instance.pendingSuspenseBoundaries === null ||
-            instance.pendingSuspenseBoundaries.size === 0)
+          (instance.pendingBoundaries === null ||
+            instance.pendingBoundaries.size === 0)
         ) {
-          instance.transitions.forEach(transition => {
-            addMarkerCompleteCallbackToPendingTransition({
-              transition,
-              name: finishedWork.memoizedProps.name,
-            });
-          });
+          addMarkerCompleteCallbackToPendingTransition(
+            finishedWork.memoizedProps.name,
+            instance.transitions,
+          );
           instance.transitions = null;
-          instance.pendingSuspenseBoundaries = null;
+          instance.pendingBoundaries = null;
         }
       }
       break;

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

@@ -17,17 +17,12 @@ import {getWorkInProgressTransitions} from './ReactFiberWorkLoop.new';
 
 export type SuspenseInfo = {name: string | null};
 
-export type MarkerTransition = {
-  transition: Transition,
-  name: string,
-};
-
 export type PendingTransitionCallbacks = {
   transitionStart: Array<Transition> | null,
-  transitionProgress: Map<Transition, PendingSuspenseBoundaries> | null,
+  transitionProgress: Map<Transition, PendingBoundaries> | null,
   transitionComplete: Array<Transition> | null,
   markerProgress: Map<string, TracingMarkerInstance> | null,
-  markerComplete: Array<MarkerTransition> | null,
+  markerComplete: Map<string, Set<Transition>> | null,
 };
 
 export type Transition = {
@@ -42,12 +37,12 @@ export type BatchConfigTransition = {
 };
 
 export type TracingMarkerInstance = {|
-  pendingSuspenseBoundaries: PendingSuspenseBoundaries | null,
+  pendingBoundaries: PendingBoundaries | null,
   transitions: Set<Transition> | null,
   name?: string,
 |};
 
-export type PendingSuspenseBoundaries = Map<OffscreenInstance, SuspenseInfo>;
+export type PendingBoundaries = Map<OffscreenInstance, SuspenseInfo>;
 
 export function processTransitionCallbacks(
   pendingTransitions: PendingTransitionCallbacks,
@@ -57,12 +52,11 @@ export function processTransitionCallbacks(
   if (enableTransitionTracing) {
     if (pendingTransitions !== null) {
       const transitionStart = pendingTransitions.transitionStart;
-      if (transitionStart !== null) {
-        transitionStart.forEach(transition => {
-          if (callbacks.onTransitionStart != null) {
-            callbacks.onTransitionStart(transition.name, transition.startTime);
-          }
-        });
+      const onTransitionStart = callbacks.onTransitionStart;
+      if (transitionStart !== null && onTransitionStart != null) {
+        transitionStart.forEach(transition =>
+          onTransitionStart(transition.name, transition.startTime),
+        );
       }
 
       const markerProgress = pendingTransitions.markerProgress;
@@ -71,8 +65,8 @@ export function processTransitionCallbacks(
         markerProgress.forEach((markerInstance, markerName) => {
           if (markerInstance.transitions !== null) {
             const pending =
-              markerInstance.pendingSuspenseBoundaries !== null
-                ? Array.from(markerInstance.pendingSuspenseBoundaries.values())
+              markerInstance.pendingBoundaries !== null
+                ? Array.from(markerInstance.pendingBoundaries.values())
                 : [];
             markerInstance.transitions.forEach(transition => {
               onMarkerProgress(
@@ -88,16 +82,17 @@ export function processTransitionCallbacks(
       }
 
       const markerComplete = pendingTransitions.markerComplete;
-      if (markerComplete !== null) {
-        markerComplete.forEach(marker => {
-          if (callbacks.onMarkerComplete != null) {
-            callbacks.onMarkerComplete(
-              marker.transition.name,
-              marker.name,
-              marker.transition.startTime,
+      const onMarkerComplete = callbacks.onMarkerComplete;
+      if (markerComplete !== null && onMarkerComplete != null) {
+        markerComplete.forEach((transitions, markerName) => {
+          transitions.forEach(transition => {
+            onMarkerComplete(
+              transition.name,
+              markerName,
+              transition.startTime,
               endTime,
             );
-          }
+          });
         });
       }
 
@@ -115,16 +110,11 @@ export function processTransitionCallbacks(
       }
 
       const transitionComplete = pendingTransitions.transitionComplete;
-      if (transitionComplete !== null) {
-        transitionComplete.forEach(transition => {
-          if (callbacks.onTransitionComplete != null) {
-            callbacks.onTransitionComplete(
-              transition.name,
-              transition.startTime,
-              endTime,
-            );
-          }
-        });
+      const onTransitionComplete = callbacks.onTransitionComplete;
+      if (transitionComplete !== null && onTransitionComplete != null) {
+        transitionComplete.forEach(transition =>
+          onTransitionComplete(transition.name, transition.startTime, endTime),
+        );
       }
     }
   }
@@ -157,7 +147,7 @@ export function pushRootMarkerInstance(workInProgress: Fiber): void {
         if (!root.incompleteTransitions.has(transition)) {
           const markerInstance: TracingMarkerInstance = {
             transitions: new Set([transition]),
-            pendingSuspenseBoundaries: null,
+            pendingBoundaries: null,
           };
           root.incompleteTransitions.set(transition, markerInstance);
         }

packages/react-reconciler/src/ReactFiberTracingMarkerComponent.old.js

@@ -17,17 +17,12 @@ import {getWorkInProgressTransitions} from './ReactFiberWorkLoop.old';
 
 export type SuspenseInfo = {name: string | null};
 
-export type MarkerTransition = {
-  transition: Transition,
-  name: string,
-};
-
 export type PendingTransitionCallbacks = {
   transitionStart: Array<Transition> | null,
-  transitionProgress: Map<Transition, PendingSuspenseBoundaries> | null,
+  transitionProgress: Map<Transition, PendingBoundaries> | null,
   transitionComplete: Array<Transition> | null,
   markerProgress: Map<string, TracingMarkerInstance> | null,
-  markerComplete: Array<MarkerTransition> | null,
+  markerComplete: Map<string, Set<Transition>> | null,
 };
 
 export type Transition = {
@@ -42,12 +37,12 @@ export type BatchConfigTransition = {
 };
 
 export type TracingMarkerInstance = {|
-  pendingSuspenseBoundaries: PendingSuspenseBoundaries | null,
+  pendingBoundaries: PendingBoundaries | null,
   transitions: Set<Transition> | null,
   name?: string,
 |};
 
-export type PendingSuspenseBoundaries = Map<OffscreenInstance, SuspenseInfo>;
+export type PendingBoundaries = Map<OffscreenInstance, SuspenseInfo>;
 
 export function processTransitionCallbacks(
   pendingTransitions: PendingTransitionCallbacks,
@@ -57,12 +52,11 @@ export function processTransitionCallbacks(
   if (enableTransitionTracing) {
     if (pendingTransitions !== null) {
       const transitionStart = pendingTransitions.transitionStart;
-      if (transitionStart !== null) {
-        transitionStart.forEach(transition => {
-          if (callbacks.onTransitionStart != null) {
-            callbacks.onTransitionStart(transition.name, transition.startTime);
-          }
-        });
+      const onTransitionStart = callbacks.onTransitionStart;
+      if (transitionStart !== null && onTransitionStart != null) {
+        transitionStart.forEach(transition =>
+          onTransitionStart(transition.name, transition.startTime),
+        );
       }
 
       const markerProgress = pendingTransitions.markerProgress;
@@ -71,8 +65,8 @@ export function processTransitionCallbacks(
         markerProgress.forEach((markerInstance, markerName) => {
           if (markerInstance.transitions !== null) {
             const pending =
-              markerInstance.pendingSuspenseBoundaries !== null
-                ? Array.from(markerInstance.pendingSuspenseBoundaries.values())
+              markerInstance.pendingBoundaries !== null
+                ? Array.from(markerInstance.pendingBoundaries.values())
                 : [];
             markerInstance.transitions.forEach(transition => {
               onMarkerProgress(
@@ -88,16 +82,17 @@ export function processTransitionCallbacks(
       }
 
       const markerComplete = pendingTransitions.markerComplete;
-      if (markerComplete !== null) {
-        markerComplete.forEach(marker => {
-          if (callbacks.onMarkerComplete != null) {
-            callbacks.onMarkerComplete(
-              marker.transition.name,
-              marker.name,
-              marker.transition.startTime,
+      const onMarkerComplete = callbacks.onMarkerComplete;
+      if (markerComplete !== null && onMarkerComplete != null) {
+        markerComplete.forEach((transitions, markerName) => {
+          transitions.forEach(transition => {
+            onMarkerComplete(
+              transition.name,
+              markerName,
+              transition.startTime,
               endTime,
             );
-          }
+          });
         });
       }
 
@@ -115,16 +110,11 @@ export function processTransitionCallbacks(
       }
 
       const transitionComplete = pendingTransitions.transitionComplete;
-      if (transitionComplete !== null) {
-        transitionComplete.forEach(transition => {
-          if (callbacks.onTransitionComplete != null) {
-            callbacks.onTransitionComplete(
-              transition.name,
-              transition.startTime,
-              endTime,
-            );
-          }
-        });
+      const onTransitionComplete = callbacks.onTransitionComplete;
+      if (transitionComplete !== null && onTransitionComplete != null) {
+        transitionComplete.forEach(transition =>
+          onTransitionComplete(transition.name, transition.startTime, endTime),
+        );
       }
     }
   }
@@ -157,7 +147,7 @@ export function pushRootMarkerInstance(workInProgress: Fiber): void {
         if (!root.incompleteTransitions.has(transition)) {
           const markerInstance: TracingMarkerInstance = {
             transitions: new Set([transition]),
-            pendingSuspenseBoundaries: null,
+            pendingBoundaries: null,
           };
           root.incompleteTransitions.set(transition, markerInstance);
         }

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

@@ -16,8 +16,7 @@ import type {FunctionComponentUpdateQueue} from './ReactFiberHooks.new';
 import type {EventPriority} from './ReactEventPriorities.new';
 import type {
   PendingTransitionCallbacks,
-  MarkerTransition,
-  PendingSuspenseBoundaries,
+  PendingBoundaries,
   Transition,
 } from './ReactFiberTracingMarkerComponent.new';
 import type {OffscreenInstance} from './ReactFiberOffscreenComponent';
@@ -358,7 +357,7 @@ export function addTransitionStartCallbackToPendingTransition(
 export function addMarkerProgressCallbackToPendingTransition(
   markerName: string,
   transitions: Set<Transition>,
-  pendingSuspenseBoundaries: PendingSuspenseBoundaries | null,
+  pendingBoundaries: PendingBoundaries | null,
 ) {
   if (enableTransitionTracing) {
     if (currentPendingTransitionCallbacks === null) {
@@ -376,14 +375,15 @@ export function addMarkerProgressCallbackToPendingTransition(
     }
 
     currentPendingTransitionCallbacks.markerProgress.set(markerName, {
-      pendingSuspenseBoundaries,
+      pendingBoundaries,
       transitions,
     });
   }
 }
 
 export function addMarkerCompleteCallbackToPendingTransition(
-  transition: MarkerTransition,
+  markerName: string,
+  transitions: Set<Transition>,
 ) {
   if (enableTransitionTracing) {
     if (currentPendingTransitionCallbacks === null) {
@@ -392,21 +392,24 @@ export function addMarkerCompleteCallbackToPendingTransition(
         transitionProgress: null,
         transitionComplete: null,
         markerProgress: null,
-        markerComplete: [],
+        markerComplete: new Map(),
       };
     }
 
     if (currentPendingTransitionCallbacks.markerComplete === null) {
-      currentPendingTransitionCallbacks.markerComplete = [];
+      currentPendingTransitionCallbacks.markerComplete = new Map();
     }
 
-    currentPendingTransitionCallbacks.markerComplete.push(transition);
+    currentPendingTransitionCallbacks.markerComplete.set(
+      markerName,
+      transitions,
+    );
   }
 }
 
 export function addTransitionProgressCallbackToPendingTransition(
   transition: Transition,
-  boundaries: PendingSuspenseBoundaries,
+  boundaries: PendingBoundaries,
 ) {
   if (enableTransitionTracing) {
     if (currentPendingTransitionCallbacks === null) {

packages/react-reconciler/src/ReactFiberWorkLoop.old.js

@@ -16,8 +16,7 @@ import type {FunctionComponentUpdateQueue} from './ReactFiberHooks.old';
 import type {EventPriority} from './ReactEventPriorities.old';
 import type {
   PendingTransitionCallbacks,
-  MarkerTransition,
-  PendingSuspenseBoundaries,
+  PendingBoundaries,
   Transition,
 } from './ReactFiberTracingMarkerComponent.old';
 import type {OffscreenInstance} from './ReactFiberOffscreenComponent';
@@ -358,7 +357,7 @@ export function addTransitionStartCallbackToPendingTransition(
 export function addMarkerProgressCallbackToPendingTransition(
   markerName: string,
   transitions: Set<Transition>,
-  pendingSuspenseBoundaries: PendingSuspenseBoundaries | null,
+  pendingBoundaries: PendingBoundaries | null,
 ) {
   if (enableTransitionTracing) {
     if (currentPendingTransitionCallbacks === null) {
@@ -376,14 +375,15 @@ export function addMarkerProgressCallbackToPendingTransition(
     }
 
     currentPendingTransitionCallbacks.markerProgress.set(markerName, {
-      pendingSuspenseBoundaries,
+      pendingBoundaries,
       transitions,
     });
   }
 }
 
 export function addMarkerCompleteCallbackToPendingTransition(
-  transition: MarkerTransition,
+  markerName: string,
+  transitions: Set<Transition>,
 ) {
   if (enableTransitionTracing) {
     if (currentPendingTransitionCallbacks === null) {
@@ -392,21 +392,24 @@ export function addMarkerCompleteCallbackToPendingTransition(
         transitionProgress: null,
         transitionComplete: null,
         markerProgress: null,
-        markerComplete: [],
+        markerComplete: new Map(),
       };
     }
 
     if (currentPendingTransitionCallbacks.markerComplete === null) {
-      currentPendingTransitionCallbacks.markerComplete = [];
+      currentPendingTransitionCallbacks.markerComplete = new Map();
     }
 
-    currentPendingTransitionCallbacks.markerComplete.push(transition);
+    currentPendingTransitionCallbacks.markerComplete.set(
+      markerName,
+      transitions,
+    );
   }
 }
 
 export function addTransitionProgressCallbackToPendingTransition(
   transition: Transition,
-  boundaries: PendingSuspenseBoundaries,
+  boundaries: PendingBoundaries,
 ) {
   if (enableTransitionTracing) {
     if (currentPendingTransitionCallbacks === null) {

packages/react-reconciler/src/ReactInternalTypes.js

@@ -28,8 +28,10 @@ import type {TimeoutHandle, NoTimeout} from './ReactFiberHostConfig';
 import type {Cache} from './ReactFiberCacheComponent.old';
 // Doing this because there's a merge conflict because of the way sync-reconciler-fork
 // is implemented
-import type {PendingSuspenseBoundaries} from './ReactFiberTracingMarkerComponent.new';
-import type {Transition} from './ReactFiberTracingMarkerComponent.new';
+import type {
+  TracingMarkerInstance,
+  Transition,
+} from './ReactFiberTracingMarkerComponent.new';
 import type {ConcurrentUpdate} from './ReactFiberConcurrentUpdates.new';
 
 // Unwind Circular: moved from ReactFiberHooks.old
@@ -335,7 +337,7 @@ type TransitionTracingOnlyFiberRootProperties = {|
   // are considered complete when the pending suspense boundaries set is
   // empty. We can represent this as a Map of transitions to suspense
   // boundary sets
-  incompleteTransitions: Map<Transition, PendingSuspenseBoundaries>,
+  incompleteTransitions: Map<Array<Transition>, TracingMarkerInstance>,
 |};
 
 // Exported FiberRoot type includes all properties,