Skip to content

Commit

Permalink
Remove ReactNoop.flushDeferredPri and flushUnitsOfWork (#14934)
Browse files Browse the repository at this point in the history
* Remove ReactNoop.flushDeferredPri and flushUnitsOfWork

Some of our older tests worked by counting how many times React checked
whether it should yield to the main thread, instead of something
publicly observable like how many times a component is rendered.

Our newer tests have converged on a style where we push into a log and
make assertions on the log. This pattern is less coupled to the
implementation while still being sufficient to test performance
optimizations, like resuming (whenever we add that back).

This commit removes flushDeferredPri and flushUnitsOfWork and upgrades
the affected tests.

* Remove shouldYieldToRenderer indirection

This wrapper is no longer necessary.
  • Loading branch information
acdlite authored Feb 23, 2019
1 parent 920b0bb commit ba708fa
Show file tree
Hide file tree
Showing 13 changed files with 309 additions and 386 deletions.
39 changes: 18 additions & 21 deletions packages/react-noop-renderer/src/createReactNoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -747,36 +747,27 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
return NoopRenderer.findHostInstance(component);
},

flushDeferredPri(timeout: number = Infinity): Array<mixed> {
// The legacy version of this function decremented the timeout before
// returning the new time.
// TODO: Convert tests to use flushUnitsOfWork or flushAndYield instead.
const n = timeout / 5 - 1;

let values = [];
flush(): Array<mixed> {
let values = yieldedValues || [];
yieldedValues = null;
// eslint-disable-next-line no-for-of-loops/no-for-of-loops
for (const value of flushUnitsOfWork(n)) {
for (const value of flushUnitsOfWork(Infinity)) {
values.push(...value);
}
return values;
},

flush(): Array<mixed> {
return ReactNoop.flushUnitsOfWork(Infinity);
},

flushAndYield(
unitsOfWork: number = Infinity,
): Generator<Array<mixed>, void, void> {
return flushUnitsOfWork(unitsOfWork);
},

flushUnitsOfWork(n: number): Array<mixed> {
// TODO: Should only be used via a Jest plugin (like we do with the
// test renderer).
unstable_flushNumberOfYields(n: number): Array<mixed> {
let values = yieldedValues || [];
yieldedValues = null;
// eslint-disable-next-line no-for-of-loops/no-for-of-loops
for (const value of flushUnitsOfWork(n)) {
for (const value of flushUnitsOfWork(Infinity)) {
values.push(...value);
if (values.length >= n) {
break;
}
}
return values;
},
Expand Down Expand Up @@ -847,7 +838,13 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
},

flushExpired(): Array<mixed> {
return ReactNoop.flushUnitsOfWork(0);
let values = yieldedValues || [];
yieldedValues = null;
// eslint-disable-next-line no-for-of-loops/no-for-of-loops
for (const value of flushUnitsOfWork(0)) {
values.push(...value);
}
return values;
},

yield(value: mixed) {
Expand Down
57 changes: 19 additions & 38 deletions packages/react-reconciler/src/ReactFiberScheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -1214,7 +1214,7 @@ function workLoop(isYieldy) {
}
} else {
// Flush asynchronous work until there's a higher priority event
while (nextUnitOfWork !== null && !shouldYieldToRenderer()) {
while (nextUnitOfWork !== null && !shouldYield()) {
nextUnitOfWork = performUnitOfWork(nextUnitOfWork);
}
}
Expand Down Expand Up @@ -2007,7 +2007,7 @@ function onSuspend(
msUntilTimeout: number,
): void {
root.expirationTime = rootExpirationTime;
if (msUntilTimeout === 0 && !shouldYieldToRenderer()) {
if (msUntilTimeout === 0 && !shouldYield()) {
// Don't wait an additional tick. Commit the tree immediately.
root.pendingCommitExpirationTime = suspendedExpirationTime;
root.finishedWork = finishedWork;
Expand Down Expand Up @@ -2204,43 +2204,24 @@ function findHighestPriorityRoot() {
nextFlushedExpirationTime = highestPriorityWork;
}

// TODO: This wrapper exists because many of the older tests (the ones that use
// flushDeferredPri) rely on the number of times `shouldYield` is called. We
// should get rid of it.
let didYield: boolean = false;
function shouldYieldToRenderer() {
if (didYield) {
return true;
}
if (shouldYield()) {
didYield = true;
return true;
}
return false;
}

function performAsyncWork(didTimeout) {
try {
if (didTimeout) {
// The callback timed out. That means at least one update has expired.
// Iterate through the root schedule. If they contain expired work, set
// the next render expiration time to the current time. This has the effect
// of flushing all expired work in a single batch, instead of flushing each
// level one at a time.
if (firstScheduledRoot !== null) {
recomputeCurrentRendererTime();
let root: FiberRoot = firstScheduledRoot;
do {
didExpireAtExpirationTime(root, currentRendererTime);
// The root schedule is circular, so this is never null.
root = (root.nextScheduledRoot: any);
} while (root !== firstScheduledRoot);
}
if (didTimeout) {
// The callback timed out. That means at least one update has expired.
// Iterate through the root schedule. If they contain expired work, set
// the next render expiration time to the current time. This has the effect
// of flushing all expired work in a single batch, instead of flushing each
// level one at a time.
if (firstScheduledRoot !== null) {
recomputeCurrentRendererTime();
let root: FiberRoot = firstScheduledRoot;
do {
didExpireAtExpirationTime(root, currentRendererTime);
// The root schedule is circular, so this is never null.
root = (root.nextScheduledRoot: any);
} while (root !== firstScheduledRoot);
}
performWork(NoWork, true);
} finally {
didYield = false;
}
performWork(NoWork, true);
}

function performSyncWork() {
Expand All @@ -2266,7 +2247,7 @@ function performWork(minExpirationTime: ExpirationTime, isYieldy: boolean) {
nextFlushedRoot !== null &&
nextFlushedExpirationTime !== NoWork &&
minExpirationTime <= nextFlushedExpirationTime &&
!(didYield && currentRendererTime > nextFlushedExpirationTime)
!(shouldYield() && currentRendererTime > nextFlushedExpirationTime)
) {
performWorkOnRoot(
nextFlushedRoot,
Expand Down Expand Up @@ -2414,7 +2395,7 @@ function performWorkOnRoot(
if (finishedWork !== null) {
// We've completed the root. Check the if we should yield one more time
// before committing.
if (!shouldYieldToRenderer()) {
if (!shouldYield()) {
// Still time left. Commit the root.
completeRoot(root, finishedWork, expirationTime);
} else {
Expand Down
Loading

0 comments on commit ba708fa

Please sign in to comment.