Skip to content

Commit

Permalink
Track the next unprocessed level globally
Browse files Browse the repository at this point in the history
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.)
  • Loading branch information
acdlite committed Sep 10, 2019
1 parent 2d93c7e commit 09461dc
Show file tree
Hide file tree
Showing 6 changed files with 159 additions and 48 deletions.
8 changes: 7 additions & 1 deletion packages/react-reconciler/src/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ import {
retryDehydratedSuspenseBoundary,
scheduleWork,
renderDidSuspendDelayIfPossible,
markUnprocessedUpdateTime,
} from './ReactFiberWorkLoop';

const ReactCurrentOwner = ReactSharedInternals.ReactCurrentOwner;
Expand Down Expand Up @@ -2064,7 +2065,7 @@ function updateDehydratedSuspenseComponent(
// render something, if we time out. Even if that requires us to delete everything and
// skip hydration.
// Delay having to do this as long as the suspense timeout allows us.
renderDidSuspendDelayIfPossible(workInProgress);
renderDidSuspendDelayIfPossible();
return retrySuspenseComponentWithoutHydrating(
current,
workInProgress,
Expand Down Expand Up @@ -2708,6 +2709,11 @@ function bailoutOnAlreadyFinishedWork(
stopProfilerTimerIfRunning(workInProgress);
}

const updateExpirationTime = workInProgress.expirationTime;
if (updateExpirationTime !== NoWork) {
markUnprocessedUpdateTime(updateExpirationTime);
}

// Check if the children have any pending work.
const childExpirationTime = workInProgress.childExpirationTime;
if (childExpirationTime < renderExpirationTime) {
Expand Down
2 changes: 1 addition & 1 deletion packages/react-reconciler/src/ReactFiberCompleteWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -922,7 +922,7 @@ function completeWork(
} else {
// Otherwise, we're going to have to hide content so we should
// suspend for longer if possible.
renderDidSuspendDelayIfPossible(workInProgress);
renderDidSuspendDelayIfPossible();
}
}
}
Expand Down
9 changes: 2 additions & 7 deletions packages/react-reconciler/src/ReactFiberHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import {
warnIfNotCurrentlyActingUpdatesInDev,
warnIfNotScopedWithMatchingAct,
markRenderEventTimeAndConfig,
markUnprocessedUpdateTime,
} from './ReactFiberWorkLoop';

import invariant from 'shared/invariant';
Expand Down Expand Up @@ -532,13 +533,6 @@ export function resetHooks(): void {
// It's also called inside mountIndeterminateComponent if we determine the
// component is a module-style component.

if (currentlyRenderingFiber !== null) {
// Even though this component didn't complete, set the remaining time left
// on this fiber. This is sometimes useful when suspending to determine if
// there's a lower priority update that could "unsuspend."
currentlyRenderingFiber.expirationTime = remainingExpirationTime;
}

renderExpirationTime = NoWork;
currentlyRenderingFiber = null;

Expand Down Expand Up @@ -763,6 +757,7 @@ function updateReducer<S, I, A>(
// Update the remaining priority in the queue.
if (updateExpirationTime > remainingExpirationTime) {
remainingExpirationTime = updateExpirationTime;
markUnprocessedUpdateTime(remainingExpirationTime);
}
} else {
// This update does have sufficient priority.
Expand Down
87 changes: 49 additions & 38 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,10 @@ let workInProgressRootExitStatus: RootExitStatus = RootIncomplete;
let workInProgressRootLatestProcessedExpirationTime: ExpirationTime = Sync;
let workInProgressRootLatestSuspenseTimeout: ExpirationTime = Sync;
let workInProgressRootCanSuspendUsingConfig: null | SuspenseConfig = null;
// The work left over by components that were visited during this render. Only
// includes unprocessed updates, not work in bailed out children.
let workInProgressRootNextUnprocessedUpdateTime: ExpirationTime = NoWork;

// If we're pinged while rendering we don't always restart immediately.
// This flag determines if it might be worthwhile to restart if an opportunity
// happens latere.
Expand Down Expand Up @@ -484,21 +488,27 @@ function markUpdateTimeFromFiberToRoot(fiber, expirationTime) {
}

if (root !== null) {
if (workInProgressRootExitStatus === RootSuspendedWithDelay) {
// The root already suspended with a delay, which means this render
// definitely won't finish. Since we have a new update, let's mark it as
// suspended now, right before marking the incoming update. This has the
// effect of interrupting the current render and switching to the update.
// TODO: This happens to work when receiving an update during the render
// phase, because of the trick inside computeExpirationForFiber to
// subtract 1 from `renderExpirationTime` to move it into a
// separate bucket. But we should probably model it with an exception,
// using the same mechanism we use to force hydration of a subtree.
// TODO: This does not account for low pri updates that were already
// scheduled before the root started rendering. Need to track the next
// pending expiration time (perhaps by backtracking the return path) and
// then trigger a restart in the `renderDidSuspendDelayIfPossible` path.
markRootSuspendedAtTime(root, renderExpirationTime);
if (workInProgressRoot === root) {
// Received an update to a tree that's in the middle of rendering. Mark
// that's unprocessed work on this root.
markUnprocessedUpdateTime(expirationTime);

if (workInProgressRootExitStatus === RootSuspendedWithDelay) {
// The root already suspended with a delay, which means this render
// definitely won't finish. Since we have a new update, let's mark it as
// suspended now, right before marking the incoming update. This has the
// effect of interrupting the current render and switching to the update.
// TODO: This happens to work when receiving an update during the render
// phase, because of the trick inside computeExpirationForFiber to
// subtract 1 from `renderExpirationTime` to move it into a
// separate bucket. But we should probably model it with an exception,
// using the same mechanism we use to force hydration of a subtree.
// TODO: This does not account for low pri updates that were already
// scheduled before the root started rendering. Need to track the next
// pending expiration time (perhaps by backtracking the return path) and
// then trigger a restart in the `renderDidSuspendDelayIfPossible` path.
markRootSuspendedAtTime(root, renderExpirationTime);
}
}
// Mark that the root has a pending update.
markRootUpdatedAtTime(root, expirationTime);
Expand Down Expand Up @@ -856,6 +866,7 @@ function prepareFreshStack(root, expirationTime) {
workInProgressRootLatestProcessedExpirationTime = Sync;
workInProgressRootLatestSuspenseTimeout = Sync;
workInProgressRootCanSuspendUsingConfig = null;
workInProgressRootNextUnprocessedUpdateTime = NoWork;
workInProgressRootHasPendingPing = false;

if (enableSchedulerTracing) {
Expand Down Expand Up @@ -1255,42 +1266,42 @@ export function markRenderEventTimeAndConfig(
}
}

export function markUnprocessedUpdateTime(
expirationTime: ExpirationTime,
): void {
if (expirationTime > workInProgressRootNextUnprocessedUpdateTime) {
workInProgressRootNextUnprocessedUpdateTime = expirationTime;
}
}

export function renderDidSuspend(): void {
if (workInProgressRootExitStatus === RootIncomplete) {
workInProgressRootExitStatus = RootSuspended;
}
}

export function renderDidSuspendDelayIfPossible(suspendedWork: Fiber): void {
export function renderDidSuspendDelayIfPossible(): void {
if (
workInProgressRootExitStatus === RootIncomplete ||
workInProgressRootExitStatus === RootSuspended
) {
workInProgressRootExitStatus = RootSuspendedWithDelay;
}

if (workInProgressRoot !== null) {
// Check if the component that suspsended, or any components in the return
// path, have a pending update. If so, those updates might unsuspend us, so
// interrupt the current render and restart.
let nextAfterSuspendedTime = NoWork;
let fiber = suspendedWork;
while (fiber !== null) {
const updateExpirationTime = fiber.expirationTime;
if (updateExpirationTime > nextAfterSuspendedTime) {
nextAfterSuspendedTime = updateExpirationTime;
}
fiber = fiber.return;
}

if (nextAfterSuspendedTime !== NoWork) {
// Mark the current render as suspended, and then mark that there's a
// pending update.
// TODO: This should immediately interrupt the current render, instead
// of waiting until the next time we yield.
markRootSuspendedAtTime(workInProgressRoot, renderExpirationTime);
markRootUpdatedAtTime(workInProgressRoot, nextAfterSuspendedTime);
}
// Check if there's a lower priority update somewhere else in the tree.
if (
workInProgressRootNextUnprocessedUpdateTime !== NoWork &&
workInProgressRoot !== null
) {
// Mark the current render as suspended, and then mark that there's a
// pending update.
// TODO: This should immediately interrupt the current render, instead
// of waiting until the next time we yield.
markRootSuspendedAtTime(workInProgressRoot, renderExpirationTime);
markRootUpdatedAtTime(
workInProgressRoot,
workInProgressRootNextUnprocessedUpdateTime,
);
}
}

Expand Down
6 changes: 5 additions & 1 deletion packages/react-reconciler/src/ReactUpdateQueue.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,10 @@ import {
} from 'shared/ReactFeatureFlags';

import {StrictMode} from './ReactTypeOfMode';
import {markRenderEventTimeAndConfig} from './ReactFiberWorkLoop';
import {
markRenderEventTimeAndConfig,
markUnprocessedUpdateTime,
} from './ReactFiberWorkLoop';

import invariant from 'shared/invariant';
import warningWithoutStack from 'shared/warningWithoutStack';
Expand Down Expand Up @@ -580,6 +583,7 @@ export function processUpdateQueue<State>(
// dealt with the props. Context in components that specify
// shouldComponentUpdate is tricky; but we'll have to account for
// that regardless.
markUnprocessedUpdateTime(newExpirationTime);
workInProgress.expirationTime = newExpirationTime;
workInProgress.memoizedState = resultState;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -382,6 +382,101 @@ describe('ReactSuspense', () => {
},
);

it(
'interrupts current render when something suspends with a ' +
"delay and we've already bailed out lower priority update in " +
'a parent',
async () => {
// This is similar to the previous test case, except this covers when
// React completely bails out on the parent component, without processing
// the update queue.

const {useState} = React;

function interrupt() {
// React has a heuristic to batch all updates that occur within the same
// event. This is a trick to circumvent that heuristic.
ReactTestRenderer.create('whatever');
}

let setShouldSuspend;
function Async() {
const [shouldSuspend, _setShouldSuspend] = useState(false);
setShouldSuspend = _setShouldSuspend;
return (
<>
<Text text="A" />
<Suspense fallback={<Text text="Loading..." />}>
{shouldSuspend ? <AsyncText text="Async" ms={2000} /> : null}
</Suspense>
<Text text="B" />
<Text text="C" />
</>
);
}

let setShouldHideInParent;
function App() {
const [shouldHideInParent, _setShouldHideInParent] = useState(false);
setShouldHideInParent = _setShouldHideInParent;
Scheduler.unstable_yieldValue(
'shouldHideInParent: ' + shouldHideInParent,
);
return shouldHideInParent ? <Text text="(empty)" /> : <Async />;
}

const root = ReactTestRenderer.create(null, {
unstable_isConcurrent: true,
});

await ReactTestRenderer.act(async () => {
root.update(<App />);
expect(Scheduler).toFlushAndYield([
'shouldHideInParent: false',
'A',
'B',
'C',
]);
expect(root).toMatchRenderedOutput('ABC');

// This update will suspend.
setShouldSuspend(true);

// Need to move into the next async bucket.
Scheduler.unstable_advanceTime(1000);
// Do a bit of work, then interrupt to trigger a restart.
expect(Scheduler).toFlushAndYieldThrough(['A']);
interrupt();
// Should not have committed loading state
expect(root).toMatchRenderedOutput('ABC');

// Schedule another update. This will have lower priority because of
// the interrupt trick above.
setShouldHideInParent(true);

expect(Scheduler).toFlushAndYieldThrough([
// Should have restarted the first update, because of the interruption
'A',
'Suspend! [Async]',
'Loading...',
'B',
]);

// Should not have committed loading state
expect(root).toMatchRenderedOutput('ABC');

// After suspending, should abort the first update and switch to the
// second update.
expect(Scheduler).toFlushAndYield([
'shouldHideInParent: true',
'(empty)',
]);

expect(root).toMatchRenderedOutput('(empty)');
});
},
);

it(
'interrupts current render when something suspends with a ' +
'delay, and a parent received an update after it completed',
Expand Down

0 comments on commit 09461dc

Please sign in to comment.