Skip to content

Commit ab4951f

Browse files
authored
Re-arrange slightly to prevent refactor hazard (#16743)
* Track "pending" and "suspended" ranges A FiberRoot can have pending work at many distinct priorities. (Note: we refer to these levels as "expiration times" to distinguish the concept from Scheduler's notion of priority levels, which represent broad categories of work. React expiration times are more granualar. They're more like a concurrent thread ID, which also happens to correspond to a moment on a timeline. It's an overloaded concept and I'm handwaving over some of the details.) Given a root, there's no convenient way to read all the pending levels in the entire tree, i.e. there's no single queue-like structure that tracks all the levels, because that granularity of information is not needed by our algorithms. Instead we track the subset of information that we actually need — most importantly, the highest priority level that exists in the entire tree. Aside from that, the other information we track includes the range of pending levels that are known to be suspended, and therefore should not be worked on. This is a refactor of how that information is tracked, and what each field represents: - A *pending* level is work that is unfinished, or not yet committed. This includes work that is suspended from committing. `firstPendingTime` and `lastPendingTime` represent the range of pending work. (Previously, "pending" was the same as "not suspended.") - A *suspended* level is work that did not complete because data was missing. `firstSuspendedTime` and `lastSuspendedTime` represent the range of suspended work. It is a subset of the pending range. (These fields are new to this commit.) - `nextAfterSuspendedTime` represents the next known level that comes after the suspended range. This commit doesn't change much in terms of observable behavior. The one change is that, when a level is suspended, React will continue working on the next known level instead of jumping straight to the last pending level. Subsequent commits will use this new structure for a more substantial refactor for how tasks are scheduled per root. * Get next expiration time from FiberRoot Given a FiberRoot, we should be able to determine the next expiration time that needs to be worked on, taking into account the levels that are pending, suspended, pinged, and so on. This removes the `expirationTime` argument from `scheduleCallbackForRoot`, and renames it to `ensureRootIsScheduled` to reflect the new signature. The expiration time is instead read from the root using a new function, `getNextExpirationTimeToWorkOn`. The next step will be to remove the `expirationTime` argument from `renderRoot`, too. * Don't bind expiration time to render callback This is a fragile pattern because there's only meant to be a single task per root, running at a single expiration time. Instead of binding the expiration time to the render task, or closing over it, we should determine the correct expiration time to work on using fields we store on the root object itself. This removes the "return a continuation" pattern from the `renderRoot` function. Continuation handling is now handled by the wrapper function, which I've renamed from `runRootCallback` to `performWorkOnRoot`. That function is merely an entry point to `renderRoot`, so I've also removed the callback argument. So to sum up, at at the beginning of each task, `performWorkOnRoot` determines which expiration time to work on, then calls `renderRoot`. And before exiting, it checks if it needs to schedule another task. * Update error recovery test to match new semantics * Remove `lastPendingTime` field It's no longer used anywhere * Restart on update to already suspended root If the work-in-progress root already suspended with a delay, then the current render definitely won't finish. We should interrupt the render and switch to the incoming update. * Restart on suspend if return path has an update Similar to the previous commit, if we suspend with a delay, and something in the return path has a pending update, we should abort the current render and switch to the update instead. * Track the next unprocessed level globally Instead of backtracking the return path. The main advantage over the backtracking approach is that we don't have to backtrack from the source fiber. (The main disadvantages are that it requires another module-level variable, and that it could include updates from unrelated sibling paths.) * Re-arrange slightly to prevent refactor hazard It should not be possible to perform any work on a root without calling `ensureRootIsScheduled` before exiting. Otherwise, we could fail to schedule a callback for pending work and the app could freeze. To help prevent a future refactor from introducing such a bug, this change makes it so that `renderRoot` is always wrapped in try-finally, and the `finally` block calls `ensureRootIsScheduled`. * Remove recursive calls to `renderRoot`. There are a few leftover cases where `renderRoot` is called recursively. All of them are related to synchronously flushing work before its expiration time. We can remove these calls by tracking the last expired level on the root, similar to what we do for other types of pending work, like pings. * Remove argument from performSyncWorkOnRoot Read the expiration time from the root, like we do in performConcurrentWorkOnRoot.
1 parent b0a8a3e commit ab4951f

10 files changed

+900
-282
lines changed

packages/react-reconciler/src/ReactFiberBeginWork.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,6 +177,7 @@ import {
177177
retryDehydratedSuspenseBoundary,
178178
scheduleWork,
179179
renderDidSuspendDelayIfPossible,
180+
markUnprocessedUpdateTime,
180181
} from './ReactFiberWorkLoop';
181182

182183
const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner;
@@ -2709,6 +2710,11 @@ function bailoutOnAlreadyFinishedWork(
27092710
stopProfilerTimerIfRunning(workInProgress);
27102711
}
27112712

2713+
const updateExpirationTime = workInProgress.expirationTime;
2714+
if (updateExpirationTime !== NoWork) {
2715+
markUnprocessedUpdateTime(updateExpirationTime);
2716+
}
2717+
27122718
// Check if the children have any pending work.
27132719
const childExpirationTime = workInProgress.childExpirationTime;
27142720
if (childExpirationTime < renderExpirationTime) {

packages/react-reconciler/src/ReactFiberExpirationTime.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,10 @@ import {
2121
export type ExpirationTime = number;
2222

2323
export const NoWork = 0;
24+
// TODO: Think of a better name for Never.
2425
export const Never = 1;
26+
// TODO: Use the Idle expiration time for idle state updates
27+
export const Idle = 2;
2528
export const Sync = MAX_SIGNED_31_BIT_INT;
2629
export const Batched = Sync - 1;
2730

packages/react-reconciler/src/ReactFiberHooks.js

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import {
4343
warnIfNotCurrentlyActingUpdatesInDev,
4444
warnIfNotScopedWithMatchingAct,
4545
markRenderEventTimeAndConfig,
46+
markUnprocessedUpdateTime,
4647
} from './ReactFiberWorkLoop';
4748

4849
import invariant from 'shared/invariant';
@@ -531,6 +532,7 @@ export function resetHooks(): void {
531532
// This is used to reset the state of this module when a component throws.
532533
// It's also called inside mountIndeterminateComponent if we determine the
533534
// component is a module-style component.
535+
534536
renderExpirationTime = NoWork;
535537
currentlyRenderingFiber = null;
536538

@@ -755,6 +757,7 @@ function updateReducer<S, I, A>(
755757
// Update the remaining priority in the queue.
756758
if (updateExpirationTime > remainingExpirationTime) {
757759
remainingExpirationTime = updateExpirationTime;
760+
markUnprocessedUpdateTime(remainingExpirationTime);
758761
}
759762
} else {
760763
// This update does have sufficient priority.

packages/react-reconciler/src/ReactFiberRoot.js

Lines changed: 125 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import type {TimeoutHandle, NoTimeout} from './ReactFiberHostConfig';
1414
import type {Thenable} from './ReactFiberWorkLoop';
1515
import type {Interaction} from 'scheduler/src/Tracing';
1616
import type {SuspenseHydrationCallbacks} from './ReactFiberSuspenseComponent';
17+
import type {ReactPriorityLevel} from './SchedulerWithReactIntegration';
1718

1819
import {noTimeout} from './ReactFiberHostConfig';
1920
import {createHostRootFiber} from './ReactFiber';
@@ -23,6 +24,7 @@ import {
2324
enableSuspenseCallback,
2425
} from 'shared/ReactFeatureFlags';
2526
import {unstable_getThreadID} from 'scheduler/tracing';
27+
import {NoPriority} from './SchedulerWithReactIntegration';
2628

2729
// TODO: This should be lifted into the renderer.
2830
export type Batch = {
@@ -69,12 +71,20 @@ type BaseFiberRootProperties = {|
6971
callbackNode: *,
7072
// Expiration of the callback associated with this root
7173
callbackExpirationTime: ExpirationTime,
74+
// Priority of the callback associated with this root
75+
callbackPriority: ReactPriorityLevel,
7276
// The earliest pending expiration time that exists in the tree
7377
firstPendingTime: ExpirationTime,
74-
// The latest pending expiration time that exists in the tree
75-
lastPendingTime: ExpirationTime,
76-
// The time at which a suspended component pinged the root to render again
77-
pingTime: ExpirationTime,
78+
// The earliest suspended expiration time that exists in the tree
79+
firstSuspendedTime: ExpirationTime,
80+
// The latest suspended expiration time that exists in the tree
81+
lastSuspendedTime: ExpirationTime,
82+
// The next known expiration time after the suspended range
83+
nextKnownPendingLevel: ExpirationTime,
84+
// The latest time at which a suspended component pinged the root to
85+
// render again
86+
lastPingedTime: ExpirationTime,
87+
lastExpiredTime: ExpirationTime,
7888
|};
7989

8090
// The following attributes are only used by interaction tracing builds.
@@ -117,10 +127,13 @@ function FiberRootNode(containerInfo, tag, hydrate) {
117127
this.hydrate = hydrate;
118128
this.firstBatch = null;
119129
this.callbackNode = null;
120-
this.callbackExpirationTime = NoWork;
130+
this.callbackPriority = NoPriority;
121131
this.firstPendingTime = NoWork;
122-
this.lastPendingTime = NoWork;
123-
this.pingTime = NoWork;
132+
this.firstSuspendedTime = NoWork;
133+
this.lastSuspendedTime = NoWork;
134+
this.nextKnownPendingLevel = NoWork;
135+
this.lastPingedTime = NoWork;
136+
this.lastExpiredTime = NoWork;
124137

125138
if (enableSchedulerTracing) {
126139
this.interactionThreadID = unstable_getThreadID();
@@ -151,3 +164,108 @@ export function createFiberRoot(
151164

152165
return root;
153166
}
167+
168+
export function isRootSuspendedAtTime(
169+
root: FiberRoot,
170+
expirationTime: ExpirationTime,
171+
): boolean {
172+
const firstSuspendedTime = root.firstSuspendedTime;
173+
const lastSuspendedTime = root.lastSuspendedTime;
174+
return (
175+
firstSuspendedTime !== NoWork &&
176+
(firstSuspendedTime >= expirationTime &&
177+
lastSuspendedTime <= expirationTime)
178+
);
179+
}
180+
181+
export function markRootSuspendedAtTime(
182+
root: FiberRoot,
183+
expirationTime: ExpirationTime,
184+
): void {
185+
const firstSuspendedTime = root.firstSuspendedTime;
186+
const lastSuspendedTime = root.lastSuspendedTime;
187+
if (firstSuspendedTime < expirationTime) {
188+
root.firstSuspendedTime = expirationTime;
189+
}
190+
if (lastSuspendedTime > expirationTime || firstSuspendedTime === NoWork) {
191+
root.lastSuspendedTime = expirationTime;
192+
}
193+
194+
if (expirationTime <= root.lastPingedTime) {
195+
root.lastPingedTime = NoWork;
196+
}
197+
198+
if (expirationTime <= root.lastExpiredTime) {
199+
root.lastExpiredTime = NoWork;
200+
}
201+
}
202+
203+
export function markRootUpdatedAtTime(
204+
root: FiberRoot,
205+
expirationTime: ExpirationTime,
206+
): void {
207+
// Update the range of pending times
208+
const firstPendingTime = root.firstPendingTime;
209+
if (expirationTime > firstPendingTime) {
210+
root.firstPendingTime = expirationTime;
211+
}
212+
213+
// Update the range of suspended times. Treat everything lower priority or
214+
// equal to this update as unsuspended.
215+
const firstSuspendedTime = root.firstSuspendedTime;
216+
if (firstSuspendedTime !== NoWork) {
217+
if (expirationTime >= firstSuspendedTime) {
218+
// The entire suspended range is now unsuspended.
219+
root.firstSuspendedTime = root.lastSuspendedTime = root.nextKnownPendingLevel = NoWork;
220+
} else if (expirationTime >= root.lastSuspendedTime) {
221+
root.lastSuspendedTime = expirationTime + 1;
222+
}
223+
224+
// This is a pending level. Check if it's higher priority than the next
225+
// known pending level.
226+
if (expirationTime > root.nextKnownPendingLevel) {
227+
root.nextKnownPendingLevel = expirationTime;
228+
}
229+
}
230+
}
231+
232+
export function markRootFinishedAtTime(
233+
root: FiberRoot,
234+
finishedExpirationTime: ExpirationTime,
235+
remainingExpirationTime: ExpirationTime,
236+
): void {
237+
// Update the range of pending times
238+
root.firstPendingTime = remainingExpirationTime;
239+
240+
// Update the range of suspended times. Treat everything higher priority or
241+
// equal to this update as unsuspended.
242+
if (finishedExpirationTime <= root.lastSuspendedTime) {
243+
// The entire suspended range is now unsuspended.
244+
root.firstSuspendedTime = root.lastSuspendedTime = root.nextKnownPendingLevel = NoWork;
245+
} else if (finishedExpirationTime <= root.firstSuspendedTime) {
246+
// Part of the suspended range is now unsuspended. Narrow the range to
247+
// include everything between the unsuspended time (non-inclusive) and the
248+
// last suspended time.
249+
root.firstSuspendedTime = finishedExpirationTime - 1;
250+
}
251+
252+
if (finishedExpirationTime <= root.lastPingedTime) {
253+
// Clear the pinged time
254+
root.lastPingedTime = NoWork;
255+
}
256+
257+
if (finishedExpirationTime <= root.lastExpiredTime) {
258+
// Clear the expired time
259+
root.lastExpiredTime = NoWork;
260+
}
261+
}
262+
263+
export function markRootExpiredAtTime(
264+
root: FiberRoot,
265+
expirationTime: ExpirationTime,
266+
): void {
267+
const lastExpiredTime = root.lastExpiredTime;
268+
if (lastExpiredTime === NoWork || lastExpiredTime > expirationTime) {
269+
root.lastExpiredTime = expirationTime;
270+
}
271+
}

0 commit comments

Comments
 (0)