[Re-land] Make prerendering always non-blocking (#31268)

Follows https://github.com/facebook/react/pull/31238

___

This is a partial re-land of
https://github.com/facebook/react/pull/31056. We saw breakages surface
after the original land and had to revert. Now that they've been fixed,
let's try this again. This time we'll split up the commits to give us
more control of testing and rollout internally.

Original PR: https://github.com/facebook/react/pull/31056
Original Commit:
https://github.com/facebook/react/pull/31056/commits/4c71025d8d1bd46344ad793e7ed3049d24f7395a
Revert PR: https://github.com/facebook/react/pull/31080

Commit description:

> When a synchronous update suspends, and we prerender the siblings, the
prerendering should be non-blocking so that we can immediately restart
once the data arrives.
>
> This happens automatically when there's a Suspense boundary, because
we immediately commit the boundary and then proceed to a Retry render,
which are always concurrent. When there's not a Suspense boundary, there
is no Retry, so we need to take care to switch from the synchronous work
loop to the concurrent one, to enable time slicing.

Co-authored-by: Andrew Clark <git@andrewclark.io>
This commit is contained in:
Jack PopeandAndrew Clark authored and GitHub committed 2024-10-15 16:47:02 -04:00
1 parent 8382581446
commit 6c4bbc7832
6 files changed
+284 -132

No files matched your search

@@ -744,7 +744,7 @@ describe('ReactDOMFiberAsync', () => {
// Because it suspended, it remains on the current path
expect(div.textContent).toBe('/path/a');
});
assertLog([]);
assertLog(gate('enableSiblingPrerendering') ? ['Suspend! [/path/b]'] : []);
await act(async () => {
resolvePromise();
+4 -2
View File
@@ -765,12 +765,14 @@ export function markRootSuspended(
root: FiberRoot,
suspendedLanes: Lanes,
spawnedLane: Lane,
didSkipSuspendedSiblings: boolean,
didAttemptEntireTree: boolean,
) {
// TODO: Split this into separate functions for marking the root at the end of
// a render attempt versus suspending while the root is still in progress.
root.suspendedLanes |= suspendedLanes;
root.pingedLanes &= ~suspendedLanes;
if (enableSiblingPrerendering && !didSkipSuspendedSiblings) {
if (enableSiblingPrerendering && didAttemptEntireTree) {
// Mark these lanes as warm so we know there's nothing else to work on.
root.warmLanes |= suspendedLanes;
} else {
+16 -4
View File
@@ -18,6 +18,7 @@ import {
disableSchedulerTimeoutInWorkLoop,
enableProfilerTimer,
enableProfilerNestedUpdatePhase,
enableSiblingPrerendering,
} from 'shared/ReactFeatureFlags';
import {
NoLane,
@@ -29,6 +30,7 @@ import {
markStarvedLanesAsExpired,
claimNextTransitionLane,
getNextLanesToFlushSync,
checkIfRootIsPrerendering,
} from './ReactFiberLane';
import {
CommitContext,
@@ -206,7 +208,10 @@ function flushSyncWorkAcrossRoots_impl(
? workInProgressRootRenderLanes
: NoLanes,
);
if (includesSyncLane(nextLanes)) {
if (
includesSyncLane(nextLanes) &&
!checkIfRootIsPrerendering(root, nextLanes)
) {
// This root has pending sync work. Flush it now.
didPerformSomeWork = true;
performSyncWorkOnRoot(root, nextLanes);
@@ -341,7 +346,13 @@ function scheduleTaskForRootDuringMicrotask(
}
// Schedule a new callback in the host environment.
if (includesSyncLane(nextLanes)) {
if (
includesSyncLane(nextLanes) &&
// If we're prerendering, then we should use the concurrent work loop
// even if the lanes are synchronous, so that prerendering never blocks
// the main thread.
!(enableSiblingPrerendering && checkIfRootIsPrerendering(root, nextLanes))
) {
// Synchronous work is always flushed at the end of the microtask, so we
// don't need to schedule an additional task.
if (existingCallbackNode !== null) {
@@ -375,9 +386,10 @@ function scheduleTaskForRootDuringMicrotask(
let schedulerPriorityLevel;
switch (lanesToEventPriority(nextLanes)) {
// Scheduler does have an "ImmediatePriority", but now that we use
// microtasks for sync work we no longer use that. Any sync work that
// reaches this path is meant to be time sliced.
case DiscreteEventPriority:
schedulerPriorityLevel = ImmediateSchedulerPriority;
break;
case ContinuousEventPriority:
schedulerPriorityLevel = UserBlockingSchedulerPriority;
break;
+180 -125
View File
@@ -765,11 +765,12 @@ export function scheduleUpdateOnFiber(
// The incoming update might unblock the current render. Interrupt the
// current attempt and restart from the top.
prepareFreshStack(root, NoLanes);
const didAttemptEntireTree = false;
markRootSuspended(
root,
workInProgressRootRenderLanes,
workInProgressDeferredLane,
workInProgressRootDidSkipSuspendedSiblings,
didAttemptEntireTree,
);
}
@@ -832,11 +833,12 @@ export function scheduleUpdateOnFiber(
// effect of interrupting the current render and switching to the update.
// TODO: Make sure this doesn't override pings that happen while we've
// already started rendering.
const didAttemptEntireTree = false;
markRootSuspended(
root,
workInProgressRootRenderLanes,
workInProgressDeferredLane,
workInProgressRootDidSkipSuspendedSiblings,
didAttemptEntireTree,
);
}
}
@@ -898,100 +900,120 @@ export function performWorkOnRoot(
// for too long ("expired" work, to prevent starvation), or we're in
// sync-updates-by-default mode.
const shouldTimeSlice =
!forceSync &&
!includesBlockingLane(lanes) &&
!includesExpiredLane(root, lanes);
(!forceSync &&
!includesBlockingLane(lanes) &&
!includesExpiredLane(root, lanes)) ||
// If we're prerendering, then we should use the concurrent work loop
// even if the lanes are synchronous, so that prerendering never blocks
// the main thread.
// TODO: We should consider doing this whenever a sync lane is suspended,
// even for regular pings.
(enableSiblingPrerendering && checkIfRootIsPrerendering(root, lanes));
let exitStatus = shouldTimeSlice
? renderRootConcurrent(root, lanes)
: renderRootSync(root, lanes);
: renderRootSync(root, lanes, true);
if (exitStatus !== RootInProgress) {
do {
let renderWasConcurrent = shouldTimeSlice;
do {
if (exitStatus === RootDidNotComplete) {
// The render unwound without completing the tree. This happens in special
// cases where need to exit the current render without producing a
// consistent tree or committing.
markRootSuspended(
root,
lanes,
NoLane,
workInProgressRootDidSkipSuspendedSiblings,
);
} else {
// The render completed.
// Check if this render may have yielded to a concurrent event, and if so,
// confirm that any newly rendered stores are consistent.
// TODO: It's possible that even a concurrent render may never have yielded
// to the main thread, if it was fast enough, or if it expired. We could
// skip the consistency check in that case, too.
const finishedWork: Fiber = (root.current.alternate: any);
if (
renderWasConcurrent &&
!isRenderConsistentWithExternalStores(finishedWork)
) {
// A store was mutated in an interleaved event. Render again,
// synchronously, to block further mutations.
exitStatus = renderRootSync(root, lanes);
// We assume the tree is now consistent because we didn't yield to any
// concurrent events.
renderWasConcurrent = false;
// Need to check the exit status again.
continue;
}
// Check if something threw
if (
(disableLegacyMode || root.tag !== LegacyRoot) &&
exitStatus === RootErrored
) {
const lanesThatJustErrored = lanes;
const errorRetryLanes = getLanesToRetrySynchronouslyOnError(
root,
lanesThatJustErrored,
);
if (errorRetryLanes !== NoLanes) {
lanes = errorRetryLanes;
exitStatus = recoverFromConcurrentError(
root,
lanesThatJustErrored,
errorRetryLanes,
);
renderWasConcurrent = false;
// Need to check the exit status again.
if (exitStatus !== RootErrored) {
// The root did not error this time. Restart the exit algorithm
// from the beginning.
// TODO: Refactor the exit algorithm to be less confusing. Maybe
// more branches + recursion instead of a loop. I think the only
// thing that causes it to be a loop is the RootDidNotComplete
// check. If that's true, then we don't need a loop/recursion
// at all.
continue;
} else {
// The root errored yet again. Proceed to commit the tree.
}
}
}
if (exitStatus === RootFatalErrored) {
prepareFreshStack(root, NoLanes);
markRootSuspended(
root,
lanes,
NoLane,
workInProgressRootDidSkipSuspendedSiblings,
);
break;
}
// We now have a consistent tree. The next step is either to commit it,
// or, if something suspended, wait to commit it after a timeout.
finishConcurrentRender(root, exitStatus, finishedWork, lanes);
if (exitStatus === RootInProgress) {
// Render phase is still in progress.
if (
enableSiblingPrerendering &&
workInProgressRootIsPrerendering &&
!shouldTimeSlice
) {
// We're in prerendering mode, but time slicing is not enabled. This
// happens when something suspends during a synchronous update. Exit the
// the work loop. When we resume, we'll use the concurrent work loop so
// that prerendering is non-blocking.
//
// Mark the root as suspended. Usually we do this at the end of the
// render phase, but we do it here so that we resume in
// prerendering mode.
// TODO: Consider always calling markRootSuspended immediately.
// Needs to be *after* we attach a ping listener, though.
const didAttemptEntireTree = false;
markRootSuspended(root, lanes, NoLane, didAttemptEntireTree);
}
break;
} while (true);
}
} else if (exitStatus === RootDidNotComplete) {
// The render unwound without completing the tree. This happens in special
// cases where need to exit the current render without producing a
// consistent tree or committing.
const didAttemptEntireTree = !workInProgressRootDidSkipSuspendedSiblings;
markRootSuspended(root, lanes, NoLane, didAttemptEntireTree);
} else {
// The render completed.
// Check if this render may have yielded to a concurrent event, and if so,
// confirm that any newly rendered stores are consistent.
// TODO: It's possible that even a concurrent render may never have yielded
// to the main thread, if it was fast enough, or if it expired. We could
// skip the consistency check in that case, too.
const finishedWork: Fiber = (root.current.alternate: any);
if (
renderWasConcurrent &&
!isRenderConsistentWithExternalStores(finishedWork)
) {
// A store was mutated in an interleaved event. Render again,
// synchronously, to block further mutations.
exitStatus = renderRootSync(root, lanes, false);
// We assume the tree is now consistent because we didn't yield to any
// concurrent events.
renderWasConcurrent = false;
// Need to check the exit status again.
continue;
}
// Check if something threw
if (
(disableLegacyMode || root.tag !== LegacyRoot) &&
exitStatus === RootErrored
) {
const lanesThatJustErrored = lanes;
const errorRetryLanes = getLanesToRetrySynchronouslyOnError(
root,
lanesThatJustErrored,
);
if (errorRetryLanes !== NoLanes) {
lanes = errorRetryLanes;
exitStatus = recoverFromConcurrentError(
root,
lanesThatJustErrored,
errorRetryLanes,
);
renderWasConcurrent = false;
// Need to check the exit status again.
if (exitStatus !== RootErrored) {
// The root did not error this time. Restart the exit algorithm
// from the beginning.
// TODO: Refactor the exit algorithm to be less confusing. Maybe
// more branches + recursion instead of a loop. I think the only
// thing that causes it to be a loop is the RootDidNotComplete
// check. If that's true, then we don't need a loop/recursion
// at all.
continue;
} else {
// The root errored yet again. Proceed to commit the tree.
}
}
}
if (exitStatus === RootFatalErrored) {
prepareFreshStack(root, NoLanes);
// Since this is a fatal error, we're going to pretend we attempted
// the entire tree, to avoid scheduling a prerender.
const didAttemptEntireTree = true;
markRootSuspended(root, lanes, NoLane, didAttemptEntireTree);
break;
}
// We now have a consistent tree. The next step is either to commit it,
// or, if something suspended, wait to commit it after a timeout.
finishConcurrentRender(root, exitStatus, finishedWork, lanes);
}
break;
} while (true);
ensureRootIsScheduled(root);
}
@@ -1024,7 +1046,7 @@ function recoverFromConcurrentError(
rootWorkInProgress.flags |= ForceClientRender;
}
const exitStatus = renderRootSync(root, errorRetryLanes);
const exitStatus = renderRootSync(root, errorRetryLanes, false);
if (exitStatus !== RootErrored) {
// Successfully finished rendering on retry
@@ -1108,11 +1130,13 @@ function finishConcurrentRender(
// This is a transition, so we should exit without committing a
// placeholder and without scheduling a timeout. Delay indefinitely
// until we receive more data.
const didAttemptEntireTree =
!workInProgressRootDidSkipSuspendedSiblings;
markRootSuspended(
root,
lanes,
workInProgressDeferredLane,
workInProgressRootDidSkipSuspendedSiblings,
didAttemptEntireTree,
);
return;
}
@@ -1168,11 +1192,13 @@ function finishConcurrentRender(
// Don't bother with a very short suspense time.
if (msUntilTimeout > 10) {
const didAttemptEntireTree =
!workInProgressRootDidSkipSuspendedSiblings;
markRootSuspended(
root,
lanes,
workInProgressDeferredLane,
workInProgressRootDidSkipSuspendedSiblings,
didAttemptEntireTree,
);
const nextLanes = getNextLanes(root, NoLanes);
@@ -1286,7 +1312,8 @@ function commitRootWhenReady(
completedRenderEndTime,
),
);
markRootSuspended(root, lanes, spawnedLane, didSkipSuspendedSiblings);
const didAttemptEntireTree = !didSkipSuspendedSiblings;
markRootSuspended(root, lanes, spawnedLane, didAttemptEntireTree);
return;
}
}
@@ -1409,7 +1436,7 @@ function markRootSuspended(
root: FiberRoot,
suspendedLanes: Lanes,
spawnedLane: Lane,
didSkipSuspendedSiblings: boolean,
didAttemptEntireTree: boolean,
) {
// When suspending, we should always exclude lanes that were pinged or (more
// rarely, since we try to avoid it) updated during the render phase.
@@ -1418,12 +1445,7 @@ function markRootSuspended(
suspendedLanes,
workInProgressRootInterleavedUpdatedLanes,
);
_markRootSuspended(
root,
suspendedLanes,
spawnedLane,
didSkipSuspendedSiblings,
);
_markRootSuspended(root, suspendedLanes, spawnedLane, didAttemptEntireTree);
}
export function flushRoot(root: FiberRoot, lanes: Lanes) {
@@ -1965,7 +1987,12 @@ export function renderDidSuspendDelayIfPossible(): void {
if (
!workInProgressRootDidSkipSuspendedSiblings &&
!includesBlockingLane(workInProgressRootRenderLanes)
// Check if the root will be blocked from committing.
// TODO: Consider aligning this better with the rest of the logic. Maybe
// we should only set the exit status to RootSuspendedWithDelay if this
// condition is true? And remove the equivalent checks elsewhere.
(includesOnlyTransitions(workInProgressRootRenderLanes) ||
getSuspenseHandler() === null)
) {
// This render may not have originally been scheduled as a prerender, but
// something suspended inside the visible part of the tree, which means we
@@ -1991,11 +2018,12 @@ export function renderDidSuspendDelayIfPossible(): void {
// pinged or updated while we were rendering.
// TODO: Consider unwinding immediately, using the
// SuspendedOnHydration mechanism.
const didAttemptEntireTree = false;
markRootSuspended(
workInProgressRoot,
workInProgressRootRenderLanes,
workInProgressDeferredLane,
workInProgressRootDidSkipSuspendedSiblings,
didAttemptEntireTree,
);
}
}
@@ -2025,7 +2053,11 @@ export function renderHasNotSuspendedYet(): boolean {
// TODO: Over time, this function and renderRootConcurrent have become more
// and more similar. Not sure it makes sense to maintain forked paths. Consider
// unifying them again.
function renderRootSync(root: FiberRoot, lanes: Lanes) {
function renderRootSync(
root: FiberRoot,
lanes: Lanes,
shouldYieldForPrerendering: boolean,
): RootExitStatus {
const prevExecutionContext = executionContext;
executionContext |= RenderContext;
const prevDispatcher = pushDispatcher(root.containerInfo);
@@ -2065,6 +2097,7 @@ function renderRootSync(root: FiberRoot, lanes: Lanes) {
}
let didSuspendInShell = false;
let exitStatus = workInProgressRootExitStatus;
outer: do {
try {
if (
@@ -2086,16 +2119,37 @@ function renderRootSync(root: FiberRoot, lanes: Lanes) {
// Selective hydration. An update flowed into a dehydrated tree.
// Interrupt the current render so the work loop can switch to the
// hydration lane.
// TODO: I think we might not need to reset the stack here; we can
// just yield and reset the stack when we re-enter the work loop,
// like normal.
resetWorkInProgressStack();
workInProgressRootExitStatus = RootDidNotComplete;
exitStatus = RootDidNotComplete;
break outer;
}
case SuspendedOnImmediate:
case SuspendedOnData: {
if (!didSuspendInShell && getSuspenseHandler() === null) {
case SuspendedOnData:
case SuspendedOnDeprecatedThrowPromise: {
if (getSuspenseHandler() === null) {
didSuspendInShell = true;
}
// Intentional fallthrough
const reason = workInProgressSuspendedReason;
workInProgressSuspendedReason = NotSuspended;
workInProgressThrownValue = null;
throwAndUnwindWorkLoop(root, unitOfWork, thrownValue, reason);
if (
enableSiblingPrerendering &&
shouldYieldForPrerendering &&
workInProgressRootIsPrerendering
) {
// We've switched into prerendering mode. This implies that we
// suspended outside of a Suspense boundary, which means this
// render will be blocked from committing. Yield to the main
// thread so we can switch to prerendering using the concurrent
// work loop.
exitStatus = RootInProgress;
break outer;
}
break;
}
default: {
// Unwind then continue with the normal work loop.
@@ -2108,6 +2162,7 @@ function renderRootSync(root: FiberRoot, lanes: Lanes) {
}
}
workLoopSync();
exitStatus = workInProgressRootExitStatus;
break;
} catch (thrownValue) {
handleThrow(root, thrownValue);
@@ -2130,14 +2185,6 @@ function renderRootSync(root: FiberRoot, lanes: Lanes) {
popDispatcher(prevDispatcher);
popAsyncDispatcher(prevAsyncDispatcher);
if (workInProgress !== null) {
// This is a sync render, so we should have finished the whole tree.
throw new Error(
'Cannot commit an incomplete root. This error is likely caused by a ' +
'bug in React. Please file an issue.',
);
}
if (__DEV__) {
if (enableDebugTracing) {
logRenderStopped();
@@ -2148,14 +2195,21 @@ function renderRootSync(root: FiberRoot, lanes: Lanes) {
markRenderStopped();
}
// Set this to null to indicate there's no in-progress render.
workInProgressRoot = null;
workInProgressRootRenderLanes = NoLanes;
if (workInProgress !== null) {
// Did not complete the tree. This can happen if something suspended in
// the shell.
} else {
// Normal case. We completed the whole tree.
// It's safe to process the queue now that the render phase is complete.
finishQueueingConcurrentUpdates();
// Set this to null to indicate there's no in-progress render.
workInProgressRoot = null;
workInProgressRootRenderLanes = NoLanes;
return workInProgressRootExitStatus;
// It's safe to process the queue now that the render phase is complete.
finishQueueingConcurrentUpdates();
}
return exitStatus;
}
// The work loop is an extremely hot path. Tell Closure not to inline it.
@@ -2201,9 +2255,7 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) {
//
// If we were previously in prerendering mode, check if we received any new
// data during an interleaved event.
if (workInProgressRootIsPrerendering) {
workInProgressRootIsPrerendering = checkIfRootIsPrerendering(root, lanes);
}
workInProgressRootIsPrerendering = checkIfRootIsPrerendering(root, lanes);
}
if (__DEV__) {
@@ -3753,6 +3805,9 @@ function pingSuspendedRoot(
// the logic of whether or not a root suspends once it completes.
// TODO: If we're rendering sync either due to Sync, Batched or expired,
// we should probably never restart.
// TODO: Attach different listeners depending on whether the listener was
// attached during prerendering. Prerender pings should not interrupt
// normal renders.
// If we're suspended with delay, or if it's a retry, we'll always suspend
// so we can always restart.
@@ -420,6 +420,10 @@ describe('ReactDeferredValue', () => {
// The initial value suspended, so we attempt the final value, which
// also suspends.
'Suspend! [Final]',
...(gate('enableSiblingPrerendering')
? ['Suspend! [Loading...]', 'Suspend! [Final]']
: []),
]);
expect(root).toMatchRenderedOutput(null);
@@ -459,6 +463,10 @@ describe('ReactDeferredValue', () => {
// The initial value suspended, so we attempt the final value, which
// also suspends.
'Suspend! [Final]',
...(gate('enableSiblingPrerendering')
? ['Suspend! [Loading...]', 'Suspend! [Final]']
: []),
]);
expect(root).toMatchRenderedOutput(null);
@@ -533,6 +541,10 @@ describe('ReactDeferredValue', () => {
// The initial value suspended, so we attempt the final value, which
// also suspends.
'Suspend! [Final]',
...(gate('enableSiblingPrerendering')
? ['Suspend! [Loading...]', 'Suspend! [Final]']
: []),
]);
expect(root).toMatchRenderedOutput(null);
@@ -479,4 +479,75 @@ describe('ReactSiblingPrerendering', () => {
assertLog([]);
},
);
it(
'when a synchronous update suspends outside a boundary, the resulting' +
'prerender is concurrent',
async () => {
function App() {
return (
<>
<Text text="A" />
<Text text="B" />
<AsyncText text="Async" />
<Text text="C" />
<Text text="D" />
</>
);
}
const root = ReactNoop.createRoot();
// Mount the root synchronously
ReactNoop.flushSync(() => root.render(<App />));
// Synchronously render everything until we suspend in the shell
assertLog(['A', 'B', 'Suspend! [Async]']);
if (gate('enableSiblingPrerendering')) {
// The rest of the siblings begin to prerender concurrently. Notice
// that we don't unwind here; we pick up where we left off above.
await waitFor(['C']);
await waitFor(['D']);
}
assertLog([]);
expect(root).toMatchRenderedOutput(null);
await resolveText('Async');
assertLog(['A', 'B', 'Async', 'C', 'D']);
expect(root).toMatchRenderedOutput('ABAsyncCD');
},
);
it('restart a suspended sync render if something suspends while prerendering the siblings', async () => {
function App() {
return (
<>
<Text text="A" />
<Text text="B" />
<AsyncText text="Async" />
<Text text="C" />
<Text text="D" />
</>
);
}
const root = ReactNoop.createRoot();
// Mount the root synchronously
ReactNoop.flushSync(() => root.render(<App />));
// Synchronously render everything until we suspend in the shell
assertLog(['A', 'B', 'Suspend! [Async]']);
if (gate('enableSiblingPrerendering')) {
// The rest of the siblings begin to prerender concurrently
await waitFor(['C']);
}
// While we're prerendering, Async resolves. We should unwind and
// start over, rather than continue prerendering D.
await resolveText('Async');
assertLog(['A', 'B', 'Async', 'C', 'D']);
expect(root).toMatchRenderedOutput('ABAsyncCD');
});
});