Skip to content

Commit

Permalink
Remove recursive calls to renderRoot.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
acdlite committed Sep 11, 2019
1 parent 628d185 commit 2ca796b
Show file tree
Hide file tree
Showing 2 changed files with 52 additions and 18 deletions.
21 changes: 21 additions & 0 deletions packages/react-reconciler/src/ReactFiberRoot.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ type BaseFiberRootProperties = {|
// The latest time at which a suspended component pinged the root to
// render again
lastPingedTime: ExpirationTime,
lastExpiredTime: ExpirationTime,
|};

// The following attributes are only used by interaction tracing builds.
Expand Down Expand Up @@ -132,6 +133,7 @@ function FiberRootNode(containerInfo, tag, hydrate) {
this.lastSuspendedTime = NoWork;
this.nextKnownPendingLevel = NoWork;
this.lastPingedTime = NoWork;
this.lastExpiredTime = NoWork;

if (enableSchedulerTracing) {
this.interactionThreadID = unstable_getThreadID();
Expand Down Expand Up @@ -192,6 +194,10 @@ export function markRootSuspendedAtTime(
if (expirationTime <= root.lastPingedTime) {
root.lastPingedTime = NoWork;
}

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

export function markRootUpdatedAtTime(
Expand Down Expand Up @@ -247,4 +253,19 @@ export function markRootFinishedAtTime(
// Clear the pinged time
root.lastPingedTime = NoWork;
}

if (finishedExpirationTime <= root.lastExpiredTime) {
// Clear the expired time
root.lastExpiredTime = NoWork;
}
}

export function markRootExpiredAtTime(
root: FiberRoot,
expirationTime: ExpirationTime,
): void {
const lastExpiredTime = root.lastExpiredTime;
if (lastExpiredTime === NoWork || lastExpiredTime > expirationTime) {
root.lastExpiredTime = expirationTime;
}
}
49 changes: 31 additions & 18 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ import {
markRootSuspendedAtTime,
markRootFinishedAtTime,
markRootUpdatedAtTime,
markRootExpiredAtTime,
} from './ReactFiberRoot';
import {
NoMode,
Expand Down Expand Up @@ -521,10 +522,14 @@ function getNextRootExpirationTimeToWorkOn(root: FiberRoot): ExpirationTime {
// Determines the next expiration time that the root should render, taking
// into account levels that may be suspended, or levels that may have
// received a ping.
//

const lastExpiredTime = root.lastExpiredTime;
if (lastExpiredTime !== NoWork) {
return lastExpiredTime;
}

// "Pending" refers to any update that hasn't committed yet, including if it
// suspended. The "suspended" range is therefore a subset.

const firstPendingTime = root.firstPendingTime;
if (!isRootSuspendedAtTime(root, firstPendingTime)) {
// The highest priority pending time is not suspended. Let's work on that.
Expand All @@ -547,6 +552,17 @@ function getNextRootExpirationTimeToWorkOn(root: FiberRoot): ExpirationTime {
// the next level that the root has work on. This function is called on every
// update, and right before exiting a task.
function ensureRootIsScheduled(root: FiberRoot) {
const lastExpiredTime = root.lastExpiredTime;
if (lastExpiredTime !== NoWork) {
// Special case: Expired work should flush synchronously.
root.callbackExpirationTime = Sync;
root.callbackPriority = ImmediatePriority;
root.callbackNode = scheduleSyncCallback(
performSyncWorkOnRoot.bind(null, root, lastExpiredTime),
);
return;
}

const expirationTime = getNextRootExpirationTimeToWorkOn(root);
const existingCallbackNode = root.callbackNode;
if (expirationTime === NoWork) {
Expand Down Expand Up @@ -621,20 +637,16 @@ function performConcurrentWorkOnRoot(root, didTimeout) {
// event time. The next update will compute a new event time.
currentEventTime = NoWork;

if (didTimeout) {
// An async update expired.
const currentTime = requestCurrentTime();
markRootExpiredAtTime(root, currentTime);
}

// Determine the next expiration time to work on, using the fields stored
// on the root.
let expirationTime = getNextRootExpirationTimeToWorkOn(root);
const expirationTime = getNextRootExpirationTimeToWorkOn(root);
if (expirationTime !== NoWork) {
if (didTimeout) {
// An async update expired. There may be other expired updates on
// this root.
const currentTime = requestCurrentTime();
if (currentTime < expirationTime) {
// Render all the expired work in a single batch.
expirationTime = currentTime;
}
}

const originalCallbackNode = root.callbackNode;
try {
renderRoot(root, expirationTime, didTimeout);
Expand Down Expand Up @@ -673,7 +685,9 @@ export function flushRoot(root: FiberRoot, expirationTime: ExpirationTime) {
'means you attempted to commit from inside a lifecycle method.',
);
}
performSyncWorkOnRoot(root, expirationTime);
markRootExpiredAtTime(root, expirationTime);
ensureRootIsScheduled(root);
flushSyncCallbackQueue();
}

export function flushDiscreteUpdates() {
Expand Down Expand Up @@ -741,9 +755,8 @@ function flushPendingDiscreteUpdates() {
const roots = rootsWithPendingDiscreteUpdates;
rootsWithPendingDiscreteUpdates = null;
roots.forEach((expirationTime, root) => {
scheduleSyncCallback(
performSyncWorkOnRoot.bind(null, root, expirationTime),
);
markRootExpiredAtTime(root, expirationTime);
ensureRootIsScheduled(root);
});
// Now flush the immediate queue.
flushSyncCallbackQueue();
Expand Down Expand Up @@ -1032,7 +1045,7 @@ function renderRoot(
// synchronously, to see if the error goes away. If there are lower
// priority updates, let's include those, too, in case they fix the
// inconsistency. Render at Idle to include all updates.
performSyncWorkOnRoot(root, Idle);
markRootExpiredAtTime(root, Idle);
return;
}
// Commit the root in its errored state.
Expand Down

0 comments on commit 2ca796b

Please sign in to comment.