Skip to content

Commit a7dabcb

Browse files
authored
Revert "Re-arrange slightly to prevent refactor hazard (#16743)" (#16769)
This reverts commit ab4951f. * 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 4b0b556 commit a7dabcb

10 files changed

+282
-900
lines changed

packages/react-reconciler/src/ReactFiberBeginWork.js

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

183182
const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner;
@@ -2710,11 +2709,6 @@ function bailoutOnAlreadyFinishedWork(
27102709
stopProfilerTimerIfRunning(workInProgress);
27112710
}
27122711

2713-
const updateExpirationTime = workInProgress.expirationTime;
2714-
if (updateExpirationTime !== NoWork) {
2715-
markUnprocessedUpdateTime(updateExpirationTime);
2716-
}
2717-
27182712
// Check if the children have any pending work.
27192713
const childExpirationTime = workInProgress.childExpirationTime;
27202714
if (childExpirationTime < renderExpirationTime) {

packages/react-reconciler/src/ReactFiberExpirationTime.js

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

2323
export const NoWork = 0;
24-
// TODO: Think of a better name for Never.
2524
export const Never = 1;
26-
// TODO: Use the Idle expiration time for idle state updates
27-
export const Idle = 2;
2825
export const Sync = MAX_SIGNED_31_BIT_INT;
2926
export const Batched = Sync - 1;
3027

packages/react-reconciler/src/ReactFiberHooks.js

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

4948
import invariant from 'shared/invariant';
@@ -532,7 +531,6 @@ export function resetHooks(): void {
532531
// This is used to reset the state of this module when a component throws.
533532
// It's also called inside mountIndeterminateComponent if we determine the
534533
// component is a module-style component.
535-
536534
renderExpirationTime = NoWork;
537535
currentlyRenderingFiber = null;
538536

@@ -757,7 +755,6 @@ function updateReducer<S, I, A>(
757755
// Update the remaining priority in the queue.
758756
if (updateExpirationTime > remainingExpirationTime) {
759757
remainingExpirationTime = updateExpirationTime;
760-
markUnprocessedUpdateTime(remainingExpirationTime);
761758
}
762759
} else {
763760
// This update does have sufficient priority.

packages/react-reconciler/src/ReactFiberRoot.js

Lines changed: 7 additions & 125 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ 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';
1817

1918
import {noTimeout} from './ReactFiberHostConfig';
2019
import {createHostRootFiber} from './ReactFiber';
@@ -24,7 +23,6 @@ import {
2423
enableSuspenseCallback,
2524
} from 'shared/ReactFeatureFlags';
2625
import {unstable_getThreadID} from 'scheduler/tracing';
27-
import {NoPriority} from './SchedulerWithReactIntegration';
2826

2927
// TODO: This should be lifted into the renderer.
3028
export type Batch = {
@@ -71,20 +69,12 @@ type BaseFiberRootProperties = {|
7169
callbackNode: *,
7270
// Expiration of the callback associated with this root
7371
callbackExpirationTime: ExpirationTime,
74-
// Priority of the callback associated with this root
75-
callbackPriority: ReactPriorityLevel,
7672
// The earliest pending expiration time that exists in the tree
7773
firstPendingTime: 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,
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,
8878
|};
8979

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

138125
if (enableSchedulerTracing) {
139126
this.interactionThreadID = unstable_getThreadID();
@@ -164,108 +151,3 @@ export function createFiberRoot(
164151

165152
return root;
166153
}
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)