Remove wasteful checks from `shouldYield`
`shouldYield` will currently return `true` if there's a higher priority task in the Scheduler queue. Since we yield every 5ms anyway, this doesn't really have any practical benefit. On the contrary, the extra checks on every `shouldYield` call are wasteful.
Andrew Clark committed
Aug 13, 2020 at 11:49 UTC
9abc2785cb070148d64fae81e523246b90b92016
3 files changed
+10
-30
packages/react/src/__tests__/ReactProfiler-test.internal.js
+3
-7
@@ -2481,15 +2481,11 @@ describe('Profiler', () => {
2481
// Errors that happen inside of a subscriber should throw,
2482
throwInOnWorkStarted = true;
2483
expect(Scheduler).toFlushAndThrow('Expected error onWorkStarted');
2484
- // Rendering was interrupted by the error that was thrown
2485
- expect(Scheduler).toHaveYielded([]);
2486
- // Rendering continues in the next task
2487
- expect(Scheduler).toFlushAndYield(['Component:text']);
2484
throwInOnWorkStarted = false;
2485
+ // Rendering was interrupted by the error that was thrown, then
2486
+ // continued and finished in the next task.
2487
+ expect(Scheduler).toHaveYielded(['Component:text']);
2488
expect(onWorkStarted).toHaveBeenCalled();
2490
-
2491
- // But the React work should have still been processed.
2492
- expect(Scheduler).toFlushAndYield([]);
2489
const tree = renderer.toTree();
2490
expect(tree.type).toBe(Component);
2491
expect(tree.props.children).toBe('text');
packages/scheduler/src/Scheduler.js
+1
-16
@@ -393,21 +393,6 @@ function unstable_getCurrentPriorityLevel() {
393
return currentPriorityLevel;
394
}
395
396
-function unstable_shouldYield() {
397
- const currentTime = getCurrentTime();
398
- advanceTimers(currentTime);
399
- const firstTask = peek(taskQueue);
400
- return (
401
- (firstTask !== currentTask &&
402
- currentTask !== null &&
403
- firstTask !== null &&
404
- firstTask.callback !== null &&
405
- firstTask.startTime <= currentTime &&
406
- firstTask.expirationTime < currentTask.expirationTime) ||
407
- shouldYieldToHost()
408
- );
409
-}
410
-
396
const unstable_requestPaint = requestPaint;
397
398
export {
@@ -422,7 +407,7 @@ export {
407
unstable_cancelCallback,
408
unstable_wrapCallback,
409
unstable_getCurrentPriorityLevel,
425
- unstable_shouldYield,
410
+ shouldYieldToHost as unstable_shouldYield,
411
unstable_requestPaint,
412
unstable_continueExecution,
413
unstable_pauseExecution,
packages/scheduler/src/__tests__/Scheduler-test.js
+6
-7
@@ -249,7 +249,7 @@ describe('Scheduler', () => {
249
});
250
251
it(
252
- 'continuations are interrupted by higher priority work scheduled ' +
252
+ 'continuations do not block higher priority work scheduled ' +
253
'inside an executing callback',
254
() => {
255
const tasks = [
@@ -272,8 +272,8 @@ describe('Scheduler', () => {
272
Scheduler.unstable_yieldValue('High pri');
273
});
274
}
275
- if (tasks.length > 0 && shouldYield()) {
276
- Scheduler.unstable_yieldValue('Yield!');
275
+ if (tasks.length > 0) {
276
+ // Return a continuation
277
return work;
278
}
279
}
@@ -283,9 +283,8 @@ describe('Scheduler', () => {
283
'A',
284
'B',
285
'Schedule high pri',
286
- // Even though there's time left in the frame, the low pri callback
287
- // should yield to the high pri callback
288
- 'Yield!',
286
+ // The high pri callback should fire before the continuation of the
287
+ // lower pri work
288
'High pri',
289
// Continue low pri work
290
'C',
@@ -662,7 +661,7 @@ describe('Scheduler', () => {
661
const [label, ms] = task;
662
Scheduler.unstable_advanceTime(ms);
663
Scheduler.unstable_yieldValue(label);
665
- if (tasks.length > 0 && shouldYield()) {
664
+ if (tasks.length > 0) {
665
return work;
666
}
667
}