Skip to content

Commit 235642b

Browse files
author
Brian Vaughn
committed
Reduce overhead from calling work-started hook
* Remove interaction-tracking wrap() references from unwind work in favor of managing suspense/interaction continuations in the scheduler * Moved the logic for calling work-started hook from performWorkOnRoot() to renderRoot()
1 parent 26dc2a8 commit 235642b

File tree

3 files changed

+98
-79
lines changed

3 files changed

+98
-79
lines changed

packages/react-reconciler/src/ReactFiberScheduler.js

Lines changed: 96 additions & 66 deletions
Original file line numberDiff line numberDiff line change
@@ -245,6 +245,10 @@ let legacyErrorBoundariesThatAlreadyFailed: Set<mixed> | null = null;
245245
// Used for performance tracking.
246246
let interruptedBy: Fiber | null = null;
247247

248+
// Do not increment or decrement interaction counts in the event of suspense timeouts.
249+
// This flag is only used when enableInteractionTracking is true.
250+
let freezeInteractionCount: boolean = false;
251+
248252
let stashedWorkInProgressProperties;
249253
let replayUnitOfWork;
250254
let isReplayingFailedUnitOfWork;
@@ -772,24 +776,28 @@ function commitRoot(root: FiberRoot, finishedWork: Fiber): void {
772776
unhandledError = error;
773777
}
774778
} finally {
775-
// Now that we're done, check the completed batch of interactions.
776-
// If no more work is outstanding for a given interaction,
777-
// We need to notify the subscribers that it's finished.
778-
committedInteractions.forEach(interaction => {
779-
interaction.__count--;
780-
if (subscriber !== null && interaction.__count === 0) {
781-
try {
782-
subscriber.onInteractionScheduledWorkCompleted(interaction);
783-
} catch (error) {
784-
// It's not safe for commitRoot() to throw.
785-
// Store the error for now and we'll re-throw in finishRendering().
786-
if (!hasUnhandledError) {
787-
hasUnhandledError = true;
788-
unhandledError = error;
779+
// Don't update interaction counts if we're frozen due to suspense.
780+
// In this case, we can skip the completed-work check entirely.
781+
if (!freezeInteractionCount) {
782+
// Now that we're done, check the completed batch of interactions.
783+
// If no more work is outstanding for a given interaction,
784+
// We need to notify the subscribers that it's finished.
785+
committedInteractions.forEach(interaction => {
786+
interaction.__count--;
787+
if (subscriber !== null && interaction.__count === 0) {
788+
try {
789+
subscriber.onInteractionScheduledWorkCompleted(interaction);
790+
} catch (error) {
791+
// It's not safe for commitRoot() to throw.
792+
// Store the error for now and we'll re-throw in finishRendering().
793+
if (!hasUnhandledError) {
794+
hasUnhandledError = true;
795+
unhandledError = error;
796+
}
789797
}
790798
}
791-
}
792-
});
799+
});
800+
}
793801
}
794802
}
795803
}
@@ -1178,6 +1186,49 @@ function renderRoot(
11781186
nextRenderExpirationTime,
11791187
);
11801188
root.pendingCommitExpirationTime = NoWork;
1189+
1190+
if (enableInteractionTracking) {
1191+
// Determine which interactions this batch of work currently includes,
1192+
// So that we can accurately attribute time spent working on it,
1193+
// And so that cascading work triggered during the render phase will be associated with it.
1194+
const interactions: Set<Interaction> = new Set();
1195+
root.pendingInteractionMap.forEach(
1196+
(scheduledInteractions, scheduledExpirationTime) => {
1197+
if (scheduledExpirationTime <= expirationTime) {
1198+
scheduledInteractions.forEach(interaction =>
1199+
interactions.add(interaction),
1200+
);
1201+
}
1202+
},
1203+
);
1204+
1205+
// Store the current set of interactions on the FiberRoot for a few reasons:
1206+
// We can re-use it in hot functions like renderRoot() without having to recalculate it.
1207+
// We will also use it in commitWork() to pass to any Profiler onRender() hooks.
1208+
// This also provides DevTools with a way to access it when the onCommitRoot() hook is called.
1209+
root.memoizedInteractions = interactions;
1210+
1211+
if (interactions.size > 0) {
1212+
const subscriber = __subscriberRef.current;
1213+
if (subscriber !== null) {
1214+
const threadID = computeThreadID(
1215+
expirationTime,
1216+
root.interactionThreadID,
1217+
);
1218+
try {
1219+
subscriber.onWorkStarted(interactions, threadID);
1220+
} catch (error) {
1221+
// Work thrown by a interaction-tracking subscriber should be rethrown,
1222+
// But only once it's safe (to avoid leaveing the scheduler in an invalid state).
1223+
// Store the error for now and we'll re-throw in finishRendering().
1224+
if (!hasUnhandledError) {
1225+
hasUnhandledError = true;
1226+
unhandledError = error;
1227+
}
1228+
}
1229+
}
1230+
}
1231+
}
11811232
}
11821233

11831234
let didFatal = false;
@@ -1550,7 +1601,19 @@ function retrySuspendedRoot(
15501601
scheduleWorkToRoot(fiber, retryTime);
15511602
const rootExpirationTime = root.expirationTime;
15521603
if (rootExpirationTime !== NoWork) {
1553-
requestWork(root, rootExpirationTime);
1604+
if (enableInteractionTracking) {
1605+
// Restore previous interactions so that new work is associated with them.
1606+
let prevInteractions = __interactionsRef.current;
1607+
__interactionsRef.current = root.memoizedInteractions;
1608+
// Because suspense timeouts do not decrement the interaction count,
1609+
// Continued suspense work should also not increment the count.
1610+
freezeInteractionCount = true;
1611+
requestWork(root, rootExpirationTime);
1612+
freezeInteractionCount = false;
1613+
__interactionsRef.current = prevInteractions;
1614+
} else {
1615+
requestWork(root, rootExpirationTime);
1616+
}
15541617
}
15551618
}
15561619
}
@@ -1618,7 +1681,7 @@ function storeInteractionsForExpirationTime(
16181681
const pendingInteractions = root.pendingInteractionMap.get(expirationTime);
16191682
if (pendingInteractions != null) {
16201683
interactions.forEach(interaction => {
1621-
if (!pendingInteractions.has(interaction)) {
1684+
if (!freezeInteractionCount && !pendingInteractions.has(interaction)) {
16221685
// Update the pending async work count for previously unscheduled interaction.
16231686
interaction.__count++;
16241687
}
@@ -1629,9 +1692,11 @@ function storeInteractionsForExpirationTime(
16291692
root.pendingInteractionMap.set(expirationTime, new Set(interactions));
16301693

16311694
// Update the pending async work count for the current interactions.
1632-
interactions.forEach(interaction => {
1633-
interaction.__count++;
1634-
});
1695+
if (!freezeInteractionCount) {
1696+
interactions.forEach(interaction => {
1697+
interaction.__count++;
1698+
});
1699+
}
16351700
}
16361701

16371702
const subscriber = __subscriberRef.current;
@@ -1860,7 +1925,16 @@ function onTimeout(root, finishedWork, suspendedExpirationTime) {
18601925
// because we're at the top of a timer event.
18611926
recomputeCurrentRendererTime();
18621927
currentSchedulerTime = currentRendererTime;
1863-
flushRoot(root, suspendedExpirationTime);
1928+
1929+
if (enableInteractionTracking) {
1930+
// Don't update pending interaction counts for suspense timeouts,
1931+
// Because we know we still need to do more work in this case.
1932+
freezeInteractionCount = true;
1933+
flushRoot(root, suspendedExpirationTime);
1934+
freezeInteractionCount = false;
1935+
} else {
1936+
flushRoot(root, suspendedExpirationTime);
1937+
}
18641938
}
18651939
}
18661940

@@ -2171,49 +2245,6 @@ function performWorkOnRoot(
21712245

21722246
isRendering = true;
21732247

2174-
if (enableInteractionTracking) {
2175-
// Determine which interactions this batch of work currently includes,
2176-
// So that we can accurately attribute time spent working on it,
2177-
// And so that cascading work triggered during the render phase will be associated with it.
2178-
const interactions: Set<Interaction> = new Set();
2179-
root.pendingInteractionMap.forEach(
2180-
(scheduledInteractions, scheduledExpirationTime) => {
2181-
if (scheduledExpirationTime <= expirationTime) {
2182-
scheduledInteractions.forEach(interaction =>
2183-
interactions.add(interaction),
2184-
);
2185-
}
2186-
},
2187-
);
2188-
2189-
// Store the current set of interactions on the FiberRoot for a few reasons:
2190-
// We can re-use it in hot functions like renderRoot() without having to recalculate it.
2191-
// We will also use it in commitWork() to pass to any Profiler onRender() hooks.
2192-
// This also provides DevTools with a way to access it when the onCommitRoot() hook is called.
2193-
root.memoizedInteractions = interactions;
2194-
2195-
if (interactions.size > 0) {
2196-
const subscriber = __subscriberRef.current;
2197-
if (subscriber !== null) {
2198-
const threadID = computeThreadID(
2199-
expirationTime,
2200-
root.interactionThreadID,
2201-
);
2202-
try {
2203-
subscriber.onWorkStarted(interactions, threadID);
2204-
} catch (error) {
2205-
// Work thrown by a interaction-tracking subscriber should be rethrown,
2206-
// But only once it's safe (to avoid leaveing the scheduler in an invalid state).
2207-
// Store the error for now and we'll re-throw in finishRendering().
2208-
if (!hasUnhandledError) {
2209-
hasUnhandledError = true;
2210-
unhandledError = error;
2211-
}
2212-
}
2213-
}
2214-
}
2215-
}
2216-
22172248
// Check if this is async work or sync/expired work.
22182249
if (deadline === null || isExpired) {
22192250
// Flush work without yielding.
@@ -2477,5 +2508,4 @@ export {
24772508
interactiveUpdates,
24782509
flushInteractiveUpdates,
24792510
computeUniqueAsyncExpiration,
2480-
computeThreadID,
24812511
};

packages/react-reconciler/src/ReactFiberUnwindWork.js

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ import {
3535
} from 'shared/ReactSideEffectTags';
3636
import {
3737
enableGetDerivedStateFromCatch,
38-
enableInteractionTracking,
3938
enableSuspense,
4039
} from 'shared/ReactFeatureFlags';
4140
import {StrictMode, AsyncMode} from './ReactTypeOfMode';
@@ -61,7 +60,6 @@ import {
6160
markLegacyErrorBoundaryAsFailed,
6261
isAlreadyFailedLegacyErrorBoundary,
6362
retrySuspendedRoot,
64-
computeThreadID,
6563
} from './ReactFiberScheduler';
6664
import {Sync} from './ReactFiberExpirationTime';
6765

@@ -73,7 +71,6 @@ import {
7371
} from './ReactFiberExpirationTime';
7472
import {findEarliestOutstandingPriorityLevel} from './ReactFiberPendingPriority';
7573
import {reconcileChildren} from './ReactFiberBeginWork';
76-
import {wrap} from 'interaction-tracking';
7774

7875
function NoopComponent() {
7976
return null;
@@ -226,14 +223,6 @@ function throwException(
226223
workInProgress,
227224
pingTime,
228225
);
229-
if (enableInteractionTracking) {
230-
// Wrap suspense callback so that the current interaction are preserved.
231-
const threadID = computeThreadID(
232-
renderExpirationTime,
233-
root.interactionThreadID,
234-
);
235-
onResolveOrReject = wrap(onResolveOrReject, threadID);
236-
}
237226
thenable.then(onResolveOrReject, onResolveOrReject);
238227

239228
// If the boundary is outside of strict mode, we should *not* suspend

packages/react/src/__tests__/ReactProfiler-test.internal.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1497,8 +1497,8 @@ describe('Profiler', () => {
14971497

14981498
didRunCallback = true;
14991499

1500-
expect(getWorkForReactThreads(onWorkStarted)).toHaveLength(2);
1501-
expect(getWorkForReactThreads(onWorkStarted)[1][0]).toMatchInteractions(
1500+
expect(getWorkForReactThreads(onWorkStarted)).toHaveLength(1);
1501+
expect(getWorkForReactThreads(onWorkStarted)[0][0]).toMatchInteractions(
15021502
[interactionOne],
15031503
);
15041504
expect(getWorkForReactThreads(onWorkStopped)).toHaveLength(1);

0 commit comments

Comments
 (0)