Skip to content

Commit

Permalink
[Transition Tracing] Code Cleanup (#24880)
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@lunaruan
lunaruan authored Jul 12, 2022
1 parent 5e8c196 commit fa20b31
Show file tree
Hide file tree
Showing 11 changed files with 111 additions and 127 deletions.
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactFiber.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -770,7 +770,7 @@ export function createFiberFromTracingMarker(
fiber.lanes = lanes;
const tracingMarkerInstance: TracingMarkerInstance = {
transitions: null,
pendingSuspenseBoundaries: null,
pendingBoundaries: null,
};
fiber.stateNode = tracingMarkerInstance;
return fiber;
Expand Down
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactFiber.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -770,7 +770,7 @@ export function createFiberFromTracingMarker(
fiber.lanes = lanes;
const tracingMarkerInstance: TracingMarkerInstance = {
transitions: null,
pendingSuspenseBoundaries: null,
pendingBoundaries: null,
};
fiber.stateNode = tracingMarkerInstance;
return fiber;
Expand Down
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactFiberBeginWork.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactFiberBeginWork.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
28 changes: 13 additions & 15 deletions packages/react-reconciler/src/ReactFiberCommitWork.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 &&
Expand Down Expand Up @@ -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 &&
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand Down
28 changes: 13 additions & 15 deletions packages/react-reconciler/src/ReactFiberCommitWork.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 &&
Expand Down Expand Up @@ -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 &&
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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;
Expand Down
62 changes: 26 additions & 36 deletions packages/react-reconciler/src/ReactFiberTracingMarkerComponent.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand All @@ -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,
Expand All @@ -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;
Expand All @@ -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(
Expand All @@ -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,
);
}
});
});
}

Expand All @@ -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),
);
}
}
}
Expand Down Expand Up @@ -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);
}
Expand Down
Loading

0 comments on commit fa20b31

Please sign in to comment.