Skip to content

Commit

Permalink
Remove Task priority (facebook#11307)
Browse files Browse the repository at this point in the history 
* Remove Task priority

The concept of Task priority was originally added as a way to avoid
reentrancy. Sync priority is for work that flushes synchronously, and
Task is for work that flushes at the end of the event loop.

But it turns out that in most cases, it's simpler to model Task and
Sync as the same priority level. For example, it's never correct to
flush Sync work from the update queue without flushing Task. Doing so
can lead to infinite update loops.

And using a separate priority level is not necessary to avoid
reentrancy. We already track when work is being rendered, and exit
before entering the render cycle again. That alone is sufficient.

This commit removes Task priority from the codebase. Now we use the
same level for both truly synchronous work and work that is deferred
until the end of the event loop.

This also enables us to remove DOM-specific legacy cases from the 
reconciler and lift them to the renderer.

* Remove isUnbatched from FiberRoot

Simpler to render unbatched roots immediately.

* Use a cyclic linked list for root schedule

Avoids the need for a separate `isScheduled` field.
  • Loading branch information
@acdlite @Ethan-Arrowood
acdlite authored and Ethan-Arrowood committed Dec 8, 2017
1 parent 9448c15 commit eaf2537
Show file tree
Hide file tree
Showing 2 changed files with 137 additions and 109 deletions.
4 changes: 1 addition & 3 deletions packages/react-reconciler/src/ReactFiberExpirationTime.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,14 +14,12 @@ export type ExpirationTime = number;

const NoWork = 0;
const Sync = 1;
const Task = 2;
const Never = 2147483647; // Max int32: Math.pow(2, 31) - 1

const UNIT_SIZE = 10;
const MAGIC_NUMBER_OFFSET = 3;
const MAGIC_NUMBER_OFFSET = 2;

exports.Sync = Sync;
exports.Task = Task;
exports.NoWork = NoWork;
exports.Never = Never;

Expand Down
242 changes: 136 additions & 106 deletions packages/react-reconciler/src/ReactFiberScheduler.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,6 @@ var {createWorkInProgress} = require('./ReactFiber');
var {onCommitRoot} = require('./ReactFiberDevToolsHook');
var {
NoWork,
Task,
Sync,
Never,
msToExpirationTime,
Expand Down Expand Up @@ -1172,15 +1171,6 @@ module.exports = function<T, P, I, TI, PI, C, CC, CX, PL>(
expirationTime = computeAsyncExpiration();
}
}

if (
expirationTime === Sync &&
isBatchingUpdates &&
(!isUnbatchingUpdates || isCommitting)
) {
// If we're in a batch, downgrade sync to task.
expirationTime = Task;
}
return expirationTime;
}

Expand Down Expand Up @@ -1250,7 +1240,7 @@ module.exports = function<T, P, I, TI, PI, C, CC, CX, PL>(
}

function scheduleErrorRecovery(fiber: Fiber) {
scheduleWorkImpl(fiber, Task, true);
scheduleWorkImpl(fiber, Sync, true);
}

function recalculateCurrentTime(): ExpirationTime {
Expand Down Expand Up @@ -1318,15 +1308,18 @@ module.exports = function<T, P, I, TI, PI, C, CC, CX, PL>(
);
}

// Add the root to the schedule.
// Check if this root is already part of the schedule.
if (root.remainingExpirationTime === NoWork) {
if (root.nextScheduledRoot === null) {
// This root is not already scheduled. Add it.
root.remainingExpirationTime = expirationTime;
if (lastScheduledRoot === null) {
firstScheduledRoot = lastScheduledRoot = root;
root.nextScheduledRoot = root;
} else {
lastScheduledRoot.nextScheduledRoot = root;
lastScheduledRoot = root;
lastScheduledRoot.nextScheduledRoot = firstScheduledRoot;
}
} else {
// This root is already scheduled, but its priority may have increased.
Expand All @@ -1340,56 +1333,89 @@ module.exports = function<T, P, I, TI, PI, C, CC, CX, PL>(
}
}

// If we're not already rendering, schedule work to flush now (if it's
// sync) or later (if it's async).
if (!isRendering) {
// TODO: Remove distinction between sync and task. Maybe we can remove
// these magic numbers entirely by always comparing to the current time?
if (expirationTime === Sync) {
if (isUnbatchingUpdates) {
performWork(Sync, null);
} else {
performWork(Task, null);
}
} else if (!isCallbackScheduled) {
isCallbackScheduled = true;
scheduleDeferredCallback(flushAsyncWork);
if (isRendering) {
// Prevent reentrancy. Remaining work will be scheduled at the end of
// the currently rendering batch.
return;
}

if (isBatchingUpdates) {
// Flush work at the end of the batch.
if (isUnbatchingUpdates) {
// ...unless we're inside unbatchedUpdates, in which case we should
// flush it now.
performWorkOnRoot(root, Sync);
}
return;
}

// TODO: Get rid of Sync and use current time?
if (expirationTime === Sync) {
performWork(Sync, null);
} else if (!isCallbackScheduled) {
isCallbackScheduled = true;
scheduleDeferredCallback(performAsyncWork);
}
}

function findHighestPriorityRoot() {
let highestPriorityWork = NoWork;
let highestPriorityRoot = null;

let previousScheduledRoot = null;
let root = firstScheduledRoot;
while (root !== null) {
const remainingExpirationTime = root.remainingExpirationTime;
if (remainingExpirationTime === NoWork) {
// If this root no longer has work, remove it from the scheduler.
let next = root.nextScheduledRoot;
root.nextScheduledRoot = null;
if (previousScheduledRoot === null) {
firstScheduledRoot = next;
if (lastScheduledRoot !== null) {
let previousScheduledRoot = lastScheduledRoot;
let root = firstScheduledRoot;
while (root !== null) {
const remainingExpirationTime = root.remainingExpirationTime;
if (remainingExpirationTime === NoWork) {
// This root no longer has work. Remove it from the scheduler.

// TODO: This check is redudant, but Flow is confused by the branch
// below where we set lastScheduledRoot to null, even though we break
// from the loop right after.
invariant(
previousScheduledRoot !== null && lastScheduledRoot !== null,
'Should have a previous and last root. This error is likely ' +
'caused by a bug in React. Please file an issue.',
);
if (root === root.nextScheduledRoot) {
// This is the only root in the list.
root.nextScheduledRoot = null;
firstScheduledRoot = lastScheduledRoot = null;
break;
} else if (root === firstScheduledRoot) {
// This is the first root in the list.
const next = root.nextScheduledRoot;
firstScheduledRoot = next;
lastScheduledRoot.nextScheduledRoot = next;
root.nextScheduledRoot = null;
} else if (root === lastScheduledRoot) {
// This is the last root in the list.
lastScheduledRoot = previousScheduledRoot;
lastScheduledRoot.nextScheduledRoot = firstScheduledRoot;
root.nextScheduledRoot = null;
break;
} else {
previousScheduledRoot.nextScheduledRoot = root.nextScheduledRoot;
root.nextScheduledRoot = null;
}
root = previousScheduledRoot.nextScheduledRoot;
} else {
previousScheduledRoot.nextScheduledRoot = next;
}
if (next === null) {
lastScheduledRoot = null;
if (
highestPriorityWork === NoWork ||
remainingExpirationTime < highestPriorityWork
) {
// Update the priority, if it's higher
highestPriorityWork = remainingExpirationTime;
highestPriorityRoot = root;
}
if (root === lastScheduledRoot) {
break;
}
previousScheduledRoot = root;
root = root.nextScheduledRoot;
}
root = next;
continue;
} else if (
highestPriorityWork === NoWork ||
remainingExpirationTime < highestPriorityWork
) {
// Update the priority, if it's higher
highestPriorityWork = remainingExpirationTime;
highestPriorityRoot = root;
}
previousScheduledRoot = root;
root = root.nextScheduledRoot;
}

// If the next root is the same as the previous root, this is a nested
Expand All @@ -1408,18 +1434,11 @@ module.exports = function<T, P, I, TI, PI, C, CC, CX, PL>(
nextFlushedExpirationTime = highestPriorityWork;
}

function flushAsyncWork(dl) {
function performAsyncWork(dl) {
performWork(NoWork, dl);
}

function performWork(minExpirationTime: ExpirationTime, dl: Deadline | null) {
invariant(
!isRendering,
'performWork was called recursively. This error is likely caused ' +
'by a bug in React. Please file an issue.',
);

isRendering = true;
deadline = dl;

// Keep working on roots until there's no more work, or until the we reach
Expand All @@ -1432,49 +1451,7 @@ module.exports = function<T, P, I, TI, PI, C, CC, CX, PL>(
nextFlushedExpirationTime <= minExpirationTime) &&
!deadlineDidExpire
) {
// Check if this is async work or sync/expired work.
// TODO: Pass current time as argument to renderRoot, commitRoot
if (nextFlushedExpirationTime <= recalculateCurrentTime()) {
// Flush sync work.
let finishedWork = nextFlushedRoot.finishedWork;
if (finishedWork !== null) {
// This root is already complete. We can commit it.
nextFlushedRoot.finishedWork = null;
nextFlushedRoot.remainingExpirationTime = commitRoot(finishedWork);
} else {
nextFlushedRoot.finishedWork = null;
finishedWork = renderRoot(nextFlushedRoot, nextFlushedExpirationTime);
if (finishedWork !== null) {
// We've completed the root. Commit it.
nextFlushedRoot.remainingExpirationTime = commitRoot(finishedWork);
}
}
} else {
// Flush async work.
let finishedWork = nextFlushedRoot.finishedWork;
if (finishedWork !== null) {
// This root is already complete. We can commit it.
nextFlushedRoot.finishedWork = null;
nextFlushedRoot.remainingExpirationTime = commitRoot(finishedWork);
} else {
nextFlushedRoot.finishedWork = null;
finishedWork = renderRoot(nextFlushedRoot, nextFlushedExpirationTime);
if (finishedWork !== null) {
// We've completed the root. Check the deadline one more time
// before committing.
if (!shouldYield()) {
// Still time left. Commit the root.
nextFlushedRoot.remainingExpirationTime = commitRoot(
finishedWork,
);
} else {
// There's no time left. Mark this root as complete. We'll come
// back and commit it later.
nextFlushedRoot.finishedWork = finishedWork;
}
}
}
}
performWorkOnRoot(nextFlushedRoot, nextFlushedExpirationTime);
// Find the next highest priority work.
findHighestPriorityRoot();
}
Expand All @@ -1489,13 +1466,12 @@ module.exports = function<T, P, I, TI, PI, C, CC, CX, PL>(
// If there's work left over, schedule a new callback.
if (nextFlushedRoot !== null && !isCallbackScheduled) {
isCallbackScheduled = true;
scheduleDeferredCallback(flushAsyncWork);
scheduleDeferredCallback(performAsyncWork);
}

// Clean-up.
deadline = null;
deadlineDidExpire = false;
isRendering = false;
nestedUpdateCount = 0;

if (hasUnhandledError) {
Expand All @@ -1506,6 +1482,60 @@ module.exports = function<T, P, I, TI, PI, C, CC, CX, PL>(
}
}

function performWorkOnRoot(root, expirationTime) {
invariant(
!isRendering,
'performWorkOnRoot was called recursively. This error is likely caused ' +
'by a bug in React. Please file an issue.',
);

isRendering = true;

// Check if this is async work or sync/expired work.
// TODO: Pass current time as argument to renderRoot, commitRoot
if (expirationTime <= recalculateCurrentTime()) {
// Flush sync work.
let finishedWork = root.finishedWork;
if (finishedWork !== null) {
// This root is already complete. We can commit it.
root.finishedWork = null;
root.remainingExpirationTime = commitRoot(finishedWork);
} else {
root.finishedWork = null;
finishedWork = renderRoot(root, expirationTime);
if (finishedWork !== null) {
// We've completed the root. Commit it.
root.remainingExpirationTime = commitRoot(finishedWork);
}
}
} else {
// Flush async work.
let finishedWork = root.finishedWork;
if (finishedWork !== null) {
// This root is already complete. We can commit it.
root.finishedWork = null;
root.remainingExpirationTime = commitRoot(finishedWork);
} else {
root.finishedWork = null;
finishedWork = renderRoot(root, expirationTime);
if (finishedWork !== null) {
// We've completed the root. Check the deadline one more time
// before committing.
if (!shouldYield()) {
// Still time left. Commit the root.
root.remainingExpirationTime = commitRoot(finishedWork);
} else {
// There's no time left. Mark this root as complete. We'll come
// back and commit it later.
root.finishedWork = finishedWork;
}
}
}
}

isRendering = false;
}

// When working on async work, the reconciler asks the renderer if it should
// yield execution. For DOM, we implement this with requestIdleCallback.
function shouldYield() {
Expand Down Expand Up @@ -1546,7 +1576,7 @@ module.exports = function<T, P, I, TI, PI, C, CC, CX, PL>(
} finally {
isBatchingUpdates = previousIsBatchingUpdates;
if (!isBatchingUpdates && !isRendering) {
performWork(Task, null);
performWork(Sync, null);
}
}
}
Expand Down Expand Up @@ -1579,7 +1609,7 @@ module.exports = function<T, P, I, TI, PI, C, CC, CX, PL>(
'flushSync was called from inside a lifecycle method. It cannot be ' +
'called when React is already rendering.',
);
performWork(Task, null);
performWork(Sync, null);
}
}

Expand Down

0 comments on commit eaf2537

Please sign in to comment.