Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Track "pending" and "suspended" ranges #16663

Closed
wants to merge 1 commit into from
Closed

Track "pending" and "suspended" ranges #16663

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
66 changes: 63 additions & 3 deletions packages/react-reconciler/src/ReactFiberRoot.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,15 @@ type BaseFiberRootProperties = {|
firstPendingTime: ExpirationTime,
// The latest pending expiration time that exists in the tree
lastPendingTime: ExpirationTime,
// The time at which a suspended component pinged the root to render again
pingTime: ExpirationTime,
// The earliest suspended expiration time that exists in the tree
firstSuspendedTime: ExpirationTime,
// The latest suspended expiration time that exists in the tree
lastSuspendedTime: ExpirationTime,
// The next known expiration time after the suspended range
nextAfterSuspendedTime: ExpirationTime,
// The latest time at which a suspended component pinged the root to
// render again
lastPingedTime: ExpirationTime,
|};

// The following attributes are only used by interaction tracing builds.
Expand Down Expand Up @@ -120,7 +127,10 @@ function FiberRootNode(containerInfo, tag, hydrate) {
this.callbackExpirationTime = NoWork;
this.firstPendingTime = NoWork;
this.lastPendingTime = NoWork;
this.pingTime = NoWork;
this.firstSuspendedTime = NoWork;
this.lastSuspendedTime = NoWork;
this.nextAfterSuspendedTime = NoWork;
this.lastPingedTime = NoWork;

if (enableSchedulerTracing) {
this.interactionThreadID = unstable_getThreadID();
Expand Down Expand Up @@ -151,3 +161,53 @@ export function createFiberRoot(

return root;
}

export function isRootSuspendedAtTime(
root: FiberRoot,
expirationTime: ExpirationTime,
): boolean {
const firstSuspendedTime = root.firstSuspendedTime;
const lastSuspendedTime = root.lastSuspendedTime;
acdlite marked this conversation as resolved.
Show resolved Hide resolved
return (
firstSuspendedTime !== NoWork &&
(firstSuspendedTime >= expirationTime &&
lastSuspendedTime <= expirationTime)
Copy link
Contributor

@NE-SmallTown NE-SmallTown Sep 5, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI, totally I think it's time to rename the name 'expirationTime' because it's been talked/discussed a few months, but for now, since it's a 'time' could we compare them through the 'time order'? I.e. from small to large (lastSuspendedTime <= expirationTime && expirationTime <= firstSuspendedTime)

);
}

export function markRootSuspendedAtTime(
root: FiberRoot,
expirationTime: ExpirationTime,
): void {
const firstSuspendedTime = root.firstSuspendedTime;
const lastSuspendedTime = root.lastSuspendedTime;
acdlite marked this conversation as resolved.
Show resolved Hide resolved
if (firstSuspendedTime < expirationTime) {
acdlite marked this conversation as resolved.
Show resolved Hide resolved
root.firstSuspendedTime = expirationTime;
}
if (lastSuspendedTime > expirationTime || firstSuspendedTime === NoWork) {
root.lastSuspendedTime = expirationTime;
}

if (expirationTime <= root.lastPingedTime) {
root.lastPingedTime = NoWork;
}
}

export function markRootUnsuspendedAtTime(
root: FiberRoot,
expirationTime: ExpirationTime,
): void {
if (expirationTime <= root.lastSuspendedTime) {
// The entire suspended range is now unsuspended.
root.firstSuspendedTime = root.lastSuspendedTime = root.nextAfterSuspendedTime = NoWork;
} else if (expirationTime <= root.firstSuspendedTime) {
// Part of the suspended range is now unsuspended. Narrow the range to
// include everything between the unsuspended time (non-inclusive) and the
// last suspended time.
root.firstSuspendedTime = expirationTime - 1;
}

if (expirationTime <= root.lastPingedTime) {
root.lastPingedTime = NoWork;
}
}
117 changes: 78 additions & 39 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,11 @@ import {
} from './ReactFiberHostConfig';

import {createWorkInProgress, assignFiberPropertiesInDEV} from './ReactFiber';
import {
isRootSuspendedAtTime,
markRootSuspendedAtTime,
markRootUnsuspendedAtTime,
} from './ReactFiberRoot';
import {
NoMode,
StrictMode,
Expand Down Expand Up @@ -377,8 +382,6 @@ export function scheduleUpdateOnFiber(
return;
}

root.pingTime = NoWork;

checkForInterruption(fiber, expirationTime);
recordScheduleUpdate();

Expand Down Expand Up @@ -492,6 +495,9 @@ function markUpdateTimeFromFiberToRoot(fiber, expirationTime) {
if (lastPendingTime === NoWork || expirationTime < lastPendingTime) {
root.lastPendingTime = expirationTime;
}

// Mark that the root is no longer suspended at this time.
markRootUnsuspendedAtTime(root, expirationTime);
}

return root;
Expand Down Expand Up @@ -807,13 +813,6 @@ function renderRoot(
'Should not already be working.',
);

if (root.firstPendingTime < expirationTime) {
// If there's no work left at this expiration time, exit immediately. This
// happens when multiple callbacks are scheduled for a single root, but an
// earlier callback flushes the work of a later one.
return null;
}

if (isSync && root.finishedExpirationTime === expirationTime) {
// There's already a pending commit at this expiration time.
// TODO: This is poorly factored. This case only exists for the
Expand All @@ -831,21 +830,25 @@ function renderRoot(
} else if (workInProgressRootExitStatus === RootSuspendedWithDelay) {
// We could've received an update at a lower priority while we yielded.
// We're suspended in a delayed state. Once we complete this render we're
// just going to try to recover at the last pending time anyway so we might
// as well start doing that eagerly.
// just going to try to recover at the pending time anyway so we might as
// well start doing that eagerly.
//
// Ideally we should be able to do this even for retries but we don't yet
// know if we're going to process an update which wants to commit earlier,
// and this path happens very early so it would happen too often. Instead,
// for that case, we'll wait until we complete.
if (workInProgressRootHasPendingPing) {
// We have a ping at this expiration. Let's restart to see if we get unblocked.
prepareFreshStack(root, expirationTime);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be removed (maybe) because there exists one in the ping function already.

} else {
const lastPendingTime = root.lastPendingTime;
if (lastPendingTime < expirationTime) {
// There's lower priority work. It might be unsuspended. Try rendering
// at that level immediately, while preserving the position in the queue.
return renderRoot.bind(null, root, lastPendingTime);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're discussing this. Seems like this refactoring changed things.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This can be replaced by a prepareFreshStack in an update that happens during a yield.

If the current tree has already determined that it has remaining work at lower pri, we can use that info to eagerly throw to terminate the render and try at lower pri.

} else if (!isSync) {
// Check if there's work that isn't in the suspended range
const firstPendingTime = root.firstPendingTime;
if (!isRootSuspendedAtTime(root, firstPendingTime)) {
// There's a pending update that falls outside the range of
// suspended work.
if (firstPendingTime > expirationTime) {
return renderRoot.bind(null, root, firstPendingTime);
}
}
}
}
Expand Down Expand Up @@ -958,7 +961,8 @@ function renderRoot(
// something suspended, wait to commit it after a timeout.
stopFinishedWorkLoopTimer();

root.finishedWork = root.current.alternate;
const finishedWork: Fiber = ((root.finishedWork =
root.current.alternate): any);
root.finishedExpirationTime = expirationTime;

const isLocked = resolveLocksOnRoot(root, expirationTime);
Expand Down Expand Up @@ -1002,6 +1006,11 @@ function renderRoot(
return commitRoot.bind(null, root);
}
case RootSuspended: {
markRootSuspendedAtTime(root, expirationTime);
const lastSuspendedTime = root.lastSuspendedTime;
if (expirationTime === lastSuspendedTime) {
root.nextAfterSuspendedTime = getRemainingExpirationTime(finishedWork);
}
flushSuspensePriorityWarningInDEV();

// We have an acceptable loading state. We need to figure out if we should
Expand Down Expand Up @@ -1038,11 +1047,20 @@ function renderRoot(
prepareFreshStack(root, expirationTime);
return renderRoot.bind(null, root, expirationTime);
}
const lastPendingTime = root.lastPendingTime;
if (lastPendingTime < expirationTime) {

const nextAfterSuspendedTime = root.nextAfterSuspendedTime;
acdlite marked this conversation as resolved.
Show resolved Hide resolved
if (nextAfterSuspendedTime !== NoWork) {
// There's lower priority work. It might be unsuspended. Try rendering
// at that level.
return renderRoot.bind(null, root, lastPendingTime);
return renderRoot.bind(null, root, nextAfterSuspendedTime);
}
if (
lastSuspendedTime !== NoWork &&
lastSuspendedTime !== expirationTime
) {
// We should prefer to render the fallback of at the last
// suspended level.
return renderRoot.bind(null, root, lastSuspendedTime);
}
// The render is suspended, it hasn't timed out, and there's no lower
// priority work to do. Instead of committing the fallback
Expand All @@ -1058,6 +1076,11 @@ function renderRoot(
return commitRoot.bind(null, root);
}
case RootSuspendedWithDelay: {
markRootSuspendedAtTime(root, expirationTime);
const lastSuspendedTime = root.lastSuspendedTime;
acdlite marked this conversation as resolved.
Show resolved Hide resolved
if (expirationTime === lastSuspendedTime) {
root.nextAfterSuspendedTime = getRemainingExpirationTime(finishedWork);
}
flushSuspensePriorityWarningInDEV();

if (
Expand All @@ -1077,11 +1100,20 @@ function renderRoot(
prepareFreshStack(root, expirationTime);
return renderRoot.bind(null, root, expirationTime);
}
const lastPendingTime = root.lastPendingTime;
if (lastPendingTime < expirationTime) {

const nextAfterSuspendedTime = root.nextAfterSuspendedTime;
acdlite marked this conversation as resolved.
Show resolved Hide resolved
if (nextAfterSuspendedTime !== NoWork) {
// There's lower priority work. It might be unsuspended. Try rendering
// at that level immediately.
return renderRoot.bind(null, root, lastPendingTime);
// at that level.
return renderRoot.bind(null, root, nextAfterSuspendedTime);
}
if (
lastSuspendedTime !== NoWork &&
lastSuspendedTime !== expirationTime
) {
// We should prefer to render the fallback of at the last
// suspended level.
return renderRoot.bind(null, root, lastSuspendedTime);
}

let msUntilTimeout;
Expand Down Expand Up @@ -1425,6 +1457,14 @@ function completeUnitOfWork(unitOfWork: Fiber): Fiber | null {
return null;
}

function getRemainingExpirationTime(fiber: Fiber) {
const updateExpirationTime = fiber.expirationTime;
const childExpirationTime = fiber.childExpirationTime;
return updateExpirationTime > childExpirationTime
? updateExpirationTime
: childExpirationTime;
acdlite marked this conversation as resolved.
Show resolved Hide resolved
}

function resetChildExpirationTime(completedWork: Fiber) {
if (
renderExpirationTime !== Never &&
Expand Down Expand Up @@ -1540,19 +1580,19 @@ function commitRootImpl(root, renderPriorityLevel) {

// Update the first and last pending times on this root. The new first
// pending time is whatever is left on the root fiber.
const updateExpirationTimeBeforeCommit = finishedWork.expirationTime;
const childExpirationTimeBeforeCommit = finishedWork.childExpirationTime;
const firstPendingTimeBeforeCommit =
childExpirationTimeBeforeCommit > updateExpirationTimeBeforeCommit
? childExpirationTimeBeforeCommit
: updateExpirationTimeBeforeCommit;
root.firstPendingTime = firstPendingTimeBeforeCommit;
if (firstPendingTimeBeforeCommit < root.lastPendingTime) {
const remainingExpirationTimeBeforeCommit = getRemainingExpirationTime(
finishedWork,
);
root.firstPendingTime = remainingExpirationTimeBeforeCommit;
if (remainingExpirationTimeBeforeCommit < root.lastPendingTime) {
// This usually means we've finished all the work, but it can also happen
// when something gets downprioritized during render, like a hidden tree.
root.lastPendingTime = firstPendingTimeBeforeCommit;
root.lastPendingTime = remainingExpirationTimeBeforeCommit;
}

// Mark that the root is no longer suspended at the finished time
markRootUnsuspendedAtTime(root, expirationTime);

if (root === workInProgressRoot) {
// We can reset these now that they are finished.
workInProgressRoot = null;
Expand Down Expand Up @@ -2148,20 +2188,19 @@ export function pingSuspendedRoot(
return;
}

const lastPendingTime = root.lastPendingTime;
if (lastPendingTime < suspendedTime) {
if (!isRootSuspendedAtTime(root, suspendedTime)) {
// The root is no longer suspended at this time.
return;
}

const pingTime = root.pingTime;
if (pingTime !== NoWork && pingTime < suspendedTime) {
const lastPingedTime = root.lastPingedTime;
if (lastPingedTime !== NoWork && lastPingedTime < suspendedTime) {
// There's already a lower priority ping scheduled.
return;
}

// Mark the time at which this ping was scheduled.
root.pingTime = suspendedTime;
root.lastPingedTime = suspendedTime;

if (root.finishedExpirationTime === suspendedTime) {
// If there's a pending fallback waiting to commit, throw it away.
Expand Down
65 changes: 65 additions & 0 deletions packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -518,6 +518,71 @@ describe('ReactSuspenseWithNoopRenderer', () => {
expect(ReactNoop.getChildren()).toEqual([span('(empty)')]);
});

it('tries each subsequent level after suspending', async () => {
const root = ReactNoop.createRoot();

function App({step, shouldSuspend}) {
return (
<Suspense fallback="Loading...">
<Text text="Sibling" />
{shouldSuspend ? (
<AsyncText ms={10000} text={'Step ' + step} />
) : (
<Text text={'Step ' + step} />
)}
</Suspense>
);
}

function interrupt() {
// React has a heuristic to batch all updates that occur within the same
// event. This is a trick to circumvent that heuristic.
ReactNoop.flushSync(() => {
ReactNoop.renderToRootWithID(null, 'other-root');
});
}

// Mount the Suspense boundary without suspending, so that the subsequent
// updates suspend with a delay.
await ReactNoop.act(async () => {
root.render(<App step={0} shouldSuspend={false} />);
});
await advanceTimers(1000);
expect(Scheduler).toHaveYielded(['Sibling', 'Step 0']);

// Schedule an update at several distinct expiration times
await ReactNoop.act(async () => {
root.render(<App step={1} shouldSuspend={true} />);
Scheduler.unstable_advanceTime(1000);
expect(Scheduler).toFlushAndYieldThrough(['Sibling']);
interrupt();

root.render(<App step={2} shouldSuspend={true} />);
Scheduler.unstable_advanceTime(1000);
expect(Scheduler).toFlushAndYieldThrough(['Sibling']);
interrupt();

root.render(<App step={3} shouldSuspend={true} />);
Scheduler.unstable_advanceTime(1000);
expect(Scheduler).toFlushAndYieldThrough(['Sibling']);
interrupt();

root.render(<App step={4} shouldSuspend={false} />);
});

// Should suspend at each distinct level
expect(Scheduler).toHaveYielded([
'Sibling',
'Suspend! [Step 1]',
'Sibling',
'Suspend! [Step 2]',
'Sibling',
'Suspend! [Step 3]',
'Sibling',
'Step 4',
]);
});

it('forces an expiration after an update times out', async () => {
ReactNoop.render(
<Fragment>
Expand Down