Skip to content

Commit fa20b31

Browse files
lunaruanlunaruan
authored
[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
1 parent 5e8c196 commit fa20b31

11 files changed

+111
-127
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -770,7 +770,7 @@ export function createFiberFromTracingMarker(
770770
fiber.lanes = lanes;
771771
const tracingMarkerInstance: TracingMarkerInstance = {
772772
transitions: null,
773-
pendingSuspenseBoundaries: null,
773+
pendingBoundaries: null,
774774
};
775775
fiber.stateNode = tracingMarkerInstance;
776776
return fiber;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -770,7 +770,7 @@ export function createFiberFromTracingMarker(
770770
fiber.lanes = lanes;
771771
const tracingMarkerInstance: TracingMarkerInstance = {
772772
transitions: null,
773-
pendingSuspenseBoundaries: null,
773+
pendingBoundaries: null,
774774
};
775775
fiber.stateNode = tracingMarkerInstance;
776776
return fiber;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -977,7 +977,7 @@ function updateTracingMarkerComponent(
977977
if (currentTransitions !== null) {
978978
const markerInstance: TracingMarkerInstance = {
979979
transitions: new Set(currentTransitions),
980-
pendingSuspenseBoundaries: new Map(),
980+
pendingBoundaries: new Map(),
981981
name: workInProgress.pendingProps.name,
982982
};
983983
workInProgress.stateNode = markerInstance;

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -977,7 +977,7 @@ function updateTracingMarkerComponent(
977977
if (currentTransitions !== null) {
978978
const markerInstance: TracingMarkerInstance = {
979979
transitions: new Set(currentTransitions),
980-
pendingSuspenseBoundaries: new Map(),
980+
pendingBoundaries: new Map(),
981981
name: workInProgress.pendingProps.name,
982982
};
983983
workInProgress.stateNode = markerInstance;

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

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1210,7 +1210,7 @@ function reappearLayoutEffectsOnFiber(node: Fiber) {
12101210
function commitTransitionProgress(offscreenFiber: Fiber) {
12111211
if (enableTransitionTracing) {
12121212
// This function adds suspense boundaries to the root
1213-
// or tracing marker's pendingSuspenseBoundaries map.
1213+
// or tracing marker's pendingBoundaries map.
12141214
// When a suspense boundary goes from a resolved to a fallback
12151215
// state we add the boundary to the map, and when it goes from
12161216
// a fallback to a resolved state, we remove the boundary from
@@ -1250,7 +1250,7 @@ function commitTransitionProgress(offscreenFiber: Fiber) {
12501250
// to the pending boundary set if it's there
12511251
if (pendingMarkers !== null) {
12521252
pendingMarkers.forEach(markerInstance => {
1253-
const pendingBoundaries = markerInstance.pendingSuspenseBoundaries;
1253+
const pendingBoundaries = markerInstance.pendingBoundaries;
12541254
const transitions = markerInstance.transitions;
12551255
if (
12561256
pendingBoundaries !== null &&
@@ -1284,7 +1284,7 @@ function commitTransitionProgress(offscreenFiber: Fiber) {
12841284
// if it's there
12851285
if (pendingMarkers !== null) {
12861286
pendingMarkers.forEach(markerInstance => {
1287-
const pendingBoundaries = markerInstance.pendingSuspenseBoundaries;
1287+
const pendingBoundaries = markerInstance.pendingBoundaries;
12881288
const transitions = markerInstance.transitions;
12891289
if (
12901290
pendingBoundaries !== null &&
@@ -2977,7 +2977,7 @@ function commitPassiveMountOnFiber(
29772977
}
29782978

29792979
incompleteTransitions.forEach((markerInstance, transition) => {
2980-
const pendingBoundaries = markerInstance.pendingSuspenseBoundaries;
2980+
const pendingBoundaries = markerInstance.pendingBoundaries;
29812981
if (pendingBoundaries === null || pendingBoundaries.size === 0) {
29822982
addTransitionCompleteCallbackToPendingTransition(transition);
29832983
incompleteTransitions.delete(transition);
@@ -3052,8 +3052,8 @@ function commitPassiveMountOnFiber(
30523052
if (instance.transitions === null) {
30533053
instance.transitions = new Set();
30543054
} else if (instance.transitions.has(transition)) {
3055-
if (markerInstance.pendingSuspenseBoundaries === null) {
3056-
markerInstance.pendingSuspenseBoundaries = new Map();
3055+
if (markerInstance.pendingBoundaries === null) {
3056+
markerInstance.pendingBoundaries = new Map();
30573057
}
30583058
if (instance.pendingMarkers === null) {
30593059
instance.pendingMarkers = new Set();
@@ -3103,17 +3103,15 @@ function commitPassiveMountOnFiber(
31033103
const instance = finishedWork.stateNode;
31043104
if (
31053105
instance.transitions !== null &&
3106-
(instance.pendingSuspenseBoundaries === null ||
3107-
instance.pendingSuspenseBoundaries.size === 0)
3106+
(instance.pendingBoundaries === null ||
3107+
instance.pendingBoundaries.size === 0)
31083108
) {
3109-
instance.transitions.forEach(transition => {
3110-
addMarkerCompleteCallbackToPendingTransition({
3111-
transition,
3112-
name: finishedWork.memoizedProps.name,
3113-
});
3114-
});
3109+
addMarkerCompleteCallbackToPendingTransition(
3110+
finishedWork.memoizedProps.name,
3111+
instance.transitions,
3112+
);
31153113
instance.transitions = null;
3116-
instance.pendingSuspenseBoundaries = null;
3114+
instance.pendingBoundaries = null;
31173115
}
31183116
}
31193117
break;

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

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1210,7 +1210,7 @@ function reappearLayoutEffectsOnFiber(node: Fiber) {
12101210
function commitTransitionProgress(offscreenFiber: Fiber) {
12111211
if (enableTransitionTracing) {
12121212
// This function adds suspense boundaries to the root
1213-
// or tracing marker's pendingSuspenseBoundaries map.
1213+
// or tracing marker's pendingBoundaries map.
12141214
// When a suspense boundary goes from a resolved to a fallback
12151215
// state we add the boundary to the map, and when it goes from
12161216
// a fallback to a resolved state, we remove the boundary from
@@ -1250,7 +1250,7 @@ function commitTransitionProgress(offscreenFiber: Fiber) {
12501250
// to the pending boundary set if it's there
12511251
if (pendingMarkers !== null) {
12521252
pendingMarkers.forEach(markerInstance => {
1253-
const pendingBoundaries = markerInstance.pendingSuspenseBoundaries;
1253+
const pendingBoundaries = markerInstance.pendingBoundaries;
12541254
const transitions = markerInstance.transitions;
12551255
if (
12561256
pendingBoundaries !== null &&
@@ -1284,7 +1284,7 @@ function commitTransitionProgress(offscreenFiber: Fiber) {
12841284
// if it's there
12851285
if (pendingMarkers !== null) {
12861286
pendingMarkers.forEach(markerInstance => {
1287-
const pendingBoundaries = markerInstance.pendingSuspenseBoundaries;
1287+
const pendingBoundaries = markerInstance.pendingBoundaries;
12881288
const transitions = markerInstance.transitions;
12891289
if (
12901290
pendingBoundaries !== null &&
@@ -2977,7 +2977,7 @@ function commitPassiveMountOnFiber(
29772977
}
29782978

29792979
incompleteTransitions.forEach((markerInstance, transition) => {
2980-
const pendingBoundaries = markerInstance.pendingSuspenseBoundaries;
2980+
const pendingBoundaries = markerInstance.pendingBoundaries;
29812981
if (pendingBoundaries === null || pendingBoundaries.size === 0) {
29822982
addTransitionCompleteCallbackToPendingTransition(transition);
29832983
incompleteTransitions.delete(transition);
@@ -3052,8 +3052,8 @@ function commitPassiveMountOnFiber(
30523052
if (instance.transitions === null) {
30533053
instance.transitions = new Set();
30543054
} else if (instance.transitions.has(transition)) {
3055-
if (markerInstance.pendingSuspenseBoundaries === null) {
3056-
markerInstance.pendingSuspenseBoundaries = new Map();
3055+
if (markerInstance.pendingBoundaries === null) {
3056+
markerInstance.pendingBoundaries = new Map();
30573057
}
30583058
if (instance.pendingMarkers === null) {
30593059
instance.pendingMarkers = new Set();
@@ -3103,17 +3103,15 @@ function commitPassiveMountOnFiber(
31033103
const instance = finishedWork.stateNode;
31043104
if (
31053105
instance.transitions !== null &&
3106-
(instance.pendingSuspenseBoundaries === null ||
3107-
instance.pendingSuspenseBoundaries.size === 0)
3106+
(instance.pendingBoundaries === null ||
3107+
instance.pendingBoundaries.size === 0)
31083108
) {
3109-
instance.transitions.forEach(transition => {
3110-
addMarkerCompleteCallbackToPendingTransition({
3111-
transition,
3112-
name: finishedWork.memoizedProps.name,
3113-
});
3114-
});
3109+
addMarkerCompleteCallbackToPendingTransition(
3110+
finishedWork.memoizedProps.name,
3111+
instance.transitions,
3112+
);
31153113
instance.transitions = null;
3116-
instance.pendingSuspenseBoundaries = null;
3114+
instance.pendingBoundaries = null;
31173115
}
31183116
}
31193117
break;

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

Lines changed: 26 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,12 @@ import {getWorkInProgressTransitions} from './ReactFiberWorkLoop.new';
1717

1818
export type SuspenseInfo = {name: string | null};
1919

20-
export type MarkerTransition = {
21-
transition: Transition,
22-
name: string,
23-
};
24-
2520
export type PendingTransitionCallbacks = {
2621
transitionStart: Array<Transition> | null,
27-
transitionProgress: Map<Transition, PendingSuspenseBoundaries> | null,
22+
transitionProgress: Map<Transition, PendingBoundaries> | null,
2823
transitionComplete: Array<Transition> | null,
2924
markerProgress: Map<string, TracingMarkerInstance> | null,
30-
markerComplete: Array<MarkerTransition> | null,
25+
markerComplete: Map<string, Set<Transition>> | null,
3126
};
3227

3328
export type Transition = {
@@ -42,12 +37,12 @@ export type BatchConfigTransition = {
4237
};
4338

4439
export type TracingMarkerInstance = {|
45-
pendingSuspenseBoundaries: PendingSuspenseBoundaries | null,
40+
pendingBoundaries: PendingBoundaries | null,
4641
transitions: Set<Transition> | null,
4742
name?: string,
4843
|};
4944

50-
export type PendingSuspenseBoundaries = Map<OffscreenInstance, SuspenseInfo>;
45+
export type PendingBoundaries = Map<OffscreenInstance, SuspenseInfo>;
5146

5247
export function processTransitionCallbacks(
5348
pendingTransitions: PendingTransitionCallbacks,
@@ -57,12 +52,11 @@ export function processTransitionCallbacks(
5752
if (enableTransitionTracing) {
5853
if (pendingTransitions !== null) {
5954
const transitionStart = pendingTransitions.transitionStart;
60-
if (transitionStart !== null) {
61-
transitionStart.forEach(transition => {
62-
if (callbacks.onTransitionStart != null) {
63-
callbacks.onTransitionStart(transition.name, transition.startTime);
64-
}
65-
});
55+
const onTransitionStart = callbacks.onTransitionStart;
56+
if (transitionStart !== null && onTransitionStart != null) {
57+
transitionStart.forEach(transition =>
58+
onTransitionStart(transition.name, transition.startTime),
59+
);
6660
}
6761

6862
const markerProgress = pendingTransitions.markerProgress;
@@ -71,8 +65,8 @@ export function processTransitionCallbacks(
7165
markerProgress.forEach((markerInstance, markerName) => {
7266
if (markerInstance.transitions !== null) {
7367
const pending =
74-
markerInstance.pendingSuspenseBoundaries !== null
75-
? Array.from(markerInstance.pendingSuspenseBoundaries.values())
68+
markerInstance.pendingBoundaries !== null
69+
? Array.from(markerInstance.pendingBoundaries.values())
7670
: [];
7771
markerInstance.transitions.forEach(transition => {
7872
onMarkerProgress(
@@ -88,16 +82,17 @@ export function processTransitionCallbacks(
8882
}
8983

9084
const markerComplete = pendingTransitions.markerComplete;
91-
if (markerComplete !== null) {
92-
markerComplete.forEach(marker => {
93-
if (callbacks.onMarkerComplete != null) {
94-
callbacks.onMarkerComplete(
95-
marker.transition.name,
96-
marker.name,
97-
marker.transition.startTime,
85+
const onMarkerComplete = callbacks.onMarkerComplete;
86+
if (markerComplete !== null && onMarkerComplete != null) {
87+
markerComplete.forEach((transitions, markerName) => {
88+
transitions.forEach(transition => {
89+
onMarkerComplete(
90+
transition.name,
91+
markerName,
92+
transition.startTime,
9893
endTime,
9994
);
100-
}
95+
});
10196
});
10297
}
10398

@@ -115,16 +110,11 @@ export function processTransitionCallbacks(
115110
}
116111

117112
const transitionComplete = pendingTransitions.transitionComplete;
118-
if (transitionComplete !== null) {
119-
transitionComplete.forEach(transition => {
120-
if (callbacks.onTransitionComplete != null) {
121-
callbacks.onTransitionComplete(
122-
transition.name,
123-
transition.startTime,
124-
endTime,
125-
);
126-
}
127-
});
113+
const onTransitionComplete = callbacks.onTransitionComplete;
114+
if (transitionComplete !== null && onTransitionComplete != null) {
115+
transitionComplete.forEach(transition =>
116+
onTransitionComplete(transition.name, transition.startTime, endTime),
117+
);
128118
}
129119
}
130120
}
@@ -157,7 +147,7 @@ export function pushRootMarkerInstance(workInProgress: Fiber): void {
157147
if (!root.incompleteTransitions.has(transition)) {
158148
const markerInstance: TracingMarkerInstance = {
159149
transitions: new Set([transition]),
160-
pendingSuspenseBoundaries: null,
150+
pendingBoundaries: null,
161151
};
162152
root.incompleteTransitions.set(transition, markerInstance);
163153
}

0 commit comments

Comments
 (0)