[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);
  }