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.
facebook · Aug 14, 2020 · 9abc278 · 9abc278
1 parent fe6d052
commit 9abc278
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 30 deletions.
diff --git a/packages/react/src/__tests__/ReactProfiler-test.internal.js b/packages/react/src/__tests__/ReactProfiler-test.internal.js
@@ -2481,15 +2481,11 @@ describe('Profiler', () => {
         // Errors that happen inside of a subscriber should throw,
         throwInOnWorkStarted = true;
         expect(Scheduler).toFlushAndThrow('Expected error onWorkStarted');
-        // Rendering was interrupted by the error that was thrown
-        expect(Scheduler).toHaveYielded([]);
-        // Rendering continues in the next task
-        expect(Scheduler).toFlushAndYield(['Component:text']);
         throwInOnWorkStarted = false;
+        // Rendering was interrupted by the error that was thrown, then
+        // continued and finished in the next task.
+        expect(Scheduler).toHaveYielded(['Component:text']);
         expect(onWorkStarted).toHaveBeenCalled();
-
-        // But the React work should have still been processed.
-        expect(Scheduler).toFlushAndYield([]);
         const tree = renderer.toTree();
         expect(tree.type).toBe(Component);
         expect(tree.props.children).toBe('text');

diff --git a/packages/scheduler/src/Scheduler.js b/packages/scheduler/src/Scheduler.js
@@ -393,21 +393,6 @@ function unstable_getCurrentPriorityLevel() {
   return currentPriorityLevel;
 }
 
-function unstable_shouldYield() {
-  const currentTime = getCurrentTime();
-  advanceTimers(currentTime);
-  const firstTask = peek(taskQueue);
-  return (
-    (firstTask !== currentTask &&
-      currentTask !== null &&
-      firstTask !== null &&
-      firstTask.callback !== null &&
-      firstTask.startTime <= currentTime &&
-      firstTask.expirationTime < currentTask.expirationTime) ||
-    shouldYieldToHost()
-  );
-}
-
 const unstable_requestPaint = requestPaint;
 
 export {
@@ -422,7 +407,7 @@ export {
   unstable_cancelCallback,
   unstable_wrapCallback,
   unstable_getCurrentPriorityLevel,
-  unstable_shouldYield,
+  shouldYieldToHost as unstable_shouldYield,
   unstable_requestPaint,
   unstable_continueExecution,
   unstable_pauseExecution,

diff --git a/packages/scheduler/src/__tests__/Scheduler-test.js b/packages/scheduler/src/__tests__/Scheduler-test.js
@@ -249,7 +249,7 @@ describe('Scheduler', () => {
   });
 
   it(
-    'continuations are interrupted by higher priority work scheduled ' +
+    'continuations do not block higher priority work scheduled ' +
       'inside an executing callback',
     () => {
       const tasks = [
@@ -272,8 +272,8 @@ describe('Scheduler', () => {
               Scheduler.unstable_yieldValue('High pri');
             });
           }
-          if (tasks.length > 0 && shouldYield()) {
-            Scheduler.unstable_yieldValue('Yield!');
+          if (tasks.length > 0) {
+            // Return a continuation
             return work;
           }
         }
@@ -283,9 +283,8 @@ describe('Scheduler', () => {
         'A',
         'B',
         'Schedule high pri',
-        // Even though there's time left in the frame, the low pri callback
-        // should yield to the high pri callback
-        'Yield!',
+        // The high pri callback should fire before the continuation of the
+        // lower pri work
         'High pri',
         // Continue low pri work
         'C',
@@ -662,7 +661,7 @@ describe('Scheduler', () => {
           const [label, ms] = task;
           Scheduler.unstable_advanceTime(ms);
           Scheduler.unstable_yieldValue(label);
-          if (tasks.length > 0 && shouldYield()) {
+          if (tasks.length > 0) {
             return work;
           }
         }