Revert "Use Global Render Timeout for CPU Suspense (#19643)"

gaearon · web-flow · commit 979b115a26f2 · 2020-08-31T19:12:28.000+01:00
This reverts commit d6e4338.
diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js
@@ -2628,6 +2628,7 @@ function initSuspenseListRenderState(
       renderingStartTime: 0,
       last: lastContentRow,
       tail: tail,
+      tailExpiration: 0,
       tailMode: tailMode,
     }: SuspenseListRenderState);
   } else {
@@ -2637,6 +2638,7 @@ function initSuspenseListRenderState(
     renderState.renderingStartTime = 0;
     renderState.last = lastContentRow;
     renderState.tail = tail;
+    renderState.tailExpiration = 0;
     renderState.tailMode = tailMode;
   }
 }
diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js
@@ -2635,6 +2635,7 @@ function initSuspenseListRenderState(
       renderingStartTime: 0,
       last: lastContentRow,
       tail: tail,
+      tailExpiration: 0,
       tailMode: tailMode,
       lastEffect: lastEffectBeforeRendering,
     }: SuspenseListRenderState);
@@ -2645,6 +2646,7 @@ function initSuspenseListRenderState(
     renderState.renderingStartTime = 0;
     renderState.last = lastContentRow;
     renderState.tail = tail;
+    renderState.tailExpiration = 0;
     renderState.tailMode = tailMode;
     renderState.lastEffect = lastEffectBeforeRendering;
   }
diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js
@@ -137,10 +137,9 @@ import {
   renderDidSuspendDelayIfPossible,
   renderHasNotSuspendedYet,
   popRenderLanes,
-  getRenderTargetTime,
 } from './ReactFiberWorkLoop.new';
 import {createFundamentalStateInstance} from './ReactFiberFundamental.new';
-import {OffscreenLane, SomeRetryLane} from './ReactFiberLane';
+import {OffscreenLane} from './ReactFiberLane';
 import {resetChildFibers} from './ReactChildFiber.new';
 import {createScopeInstance} from './ReactFiberScope.new';
 import {transferActualDuration} from './ReactProfilerTimer.new';
@@ -1067,29 +1066,6 @@ function completeWork(
               row = row.sibling;
             }
           }
-
-          if (renderState.tail !== null && now() > getRenderTargetTime()) {
-            // We have already passed our CPU deadline but we still have rows
-            // left in the tail. We'll just give up further attempts to render
-            // the main content and only render fallbacks.
-            workInProgress.effectTag |= DidCapture;
-            didSuspendAlready = true;
-
-            cutOffTailIfNeeded(renderState, false);
-
-            // Since nothing actually suspended, there will nothing to ping this
-            // to get it started back up to attempt the next item. While in terms
-            // of priority this work has the same priority as this current render,
-            // it's not part of the same transition once the transition has
-            // committed. If it's sync, we still want to yield so that it can be
-            // painted. Conceptually, this is really the same as pinging.
-            // We can use any RetryLane even if it's the one currently rendering
-            // since we're leaving it behind on this node.
-            workInProgress.lanes = SomeRetryLane;
-            if (enableSchedulerTracing) {
-              markSpawnedWork(SomeRetryLane);
-            }
-          }
         } else {
           cutOffTailIfNeeded(renderState, false);
         }
@@ -1122,11 +1098,10 @@ function completeWork(
               return null;
             }
           } else if (
-            // The time it took to render last row is greater than the remaining
-            // time we have to render. So rendering one more row would likely
-            // exceed it.
+            // The time it took to render last row is greater than time until
+            // the expiration.
             now() * 2 - renderState.renderingStartTime >
-              getRenderTargetTime() &&
+              renderState.tailExpiration &&
             renderLanes !== OffscreenLane
           ) {
             // We have now passed our CPU deadline and we'll just give up further
@@ -1142,9 +1117,9 @@ function completeWork(
             // them, then they really have the same priority as this render.
             // So we'll pick it back up the very next render pass once we've had
             // an opportunity to yield for paint.
-            workInProgress.lanes = SomeRetryLane;
+            workInProgress.lanes = renderLanes;
             if (enableSchedulerTracing) {
-              markSpawnedWork(SomeRetryLane);
+              markSpawnedWork(renderLanes);
             }
           }
         }
@@ -1169,6 +1144,18 @@ function completeWork(
 
       if (renderState.tail !== null) {
         // We still have tail rows to render.
+        if (renderState.tailExpiration === 0) {
+          // Heuristic for how long we're willing to spend rendering rows
+          // until we just give up and show what we have so far.
+          const TAIL_EXPIRATION_TIMEOUT_MS = 500;
+          renderState.tailExpiration = now() + TAIL_EXPIRATION_TIMEOUT_MS;
+          // TODO: This is meant to mimic the train model or JND but this
+          // is a per component value. It should really be since the start
+          // of the total render or last commit. Consider using something like
+          // globalMostRecentFallbackTime. That doesn't account for being
+          // suspended for part of the time or when it's a new render.
+          // It should probably use a global start time value instead.
+        }
         // Pop a row.
         const next = renderState.tail;
         renderState.rendering = next;
diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.old.js b/packages/react-reconciler/src/ReactFiberCompleteWork.old.js
@@ -135,10 +135,9 @@ import {
   renderDidSuspendDelayIfPossible,
   renderHasNotSuspendedYet,
   popRenderLanes,
-  getRenderTargetTime,
 } from './ReactFiberWorkLoop.old';
 import {createFundamentalStateInstance} from './ReactFiberFundamental.old';
-import {OffscreenLane, SomeRetryLane} from './ReactFiberLane';
+import {OffscreenLane} from './ReactFiberLane';
 import {resetChildFibers} from './ReactChildFiber.old';
 import {createScopeInstance} from './ReactFiberScope.old';
 import {transferActualDuration} from './ReactProfilerTimer.old';
@@ -1050,29 +1049,6 @@ function completeWork(
               row = row.sibling;
             }
           }
-
-          if (renderState.tail !== null && now() > getRenderTargetTime()) {
-            // We have already passed our CPU deadline but we still have rows
-            // left in the tail. We'll just give up further attempts to render
-            // the main content and only render fallbacks.
-            workInProgress.effectTag |= DidCapture;
-            didSuspendAlready = true;
-
-            cutOffTailIfNeeded(renderState, false);
-
-            // Since nothing actually suspended, there will nothing to ping this
-            // to get it started back up to attempt the next item. While in terms
-            // of priority this work has the same priority as this current render,
-            // it's not part of the same transition once the transition has
-            // committed. If it's sync, we still want to yield so that it can be
-            // painted. Conceptually, this is really the same as pinging.
-            // We can use any RetryLane even if it's the one currently rendering
-            // since we're leaving it behind on this node.
-            workInProgress.lanes = SomeRetryLane;
-            if (enableSchedulerTracing) {
-              markSpawnedWork(SomeRetryLane);
-            }
-          }
         } else {
           cutOffTailIfNeeded(renderState, false);
         }
@@ -1114,11 +1090,10 @@ function completeWork(
               return null;
             }
           } else if (
-            // The time it took to render last row is greater than the remaining
-            // time we have to render. So rendering one more row would likely
-            // exceed it.
+            // The time it took to render last row is greater than time until
+            // the expiration.
             now() * 2 - renderState.renderingStartTime >
-              getRenderTargetTime() &&
+              renderState.tailExpiration &&
             renderLanes !== OffscreenLane
           ) {
             // We have now passed our CPU deadline and we'll just give up further
@@ -1130,16 +1105,13 @@ function completeWork(
             cutOffTailIfNeeded(renderState, false);
 
             // Since nothing actually suspended, there will nothing to ping this
-            // to get it started back up to attempt the next item. While in terms
-            // of priority this work has the same priority as this current render,
-            // it's not part of the same transition once the transition has
-            // committed. If it's sync, we still want to yield so that it can be
-            // painted. Conceptually, this is really the same as pinging.
-            // We can use any RetryLane even if it's the one currently rendering
-            // since we're leaving it behind on this node.
-            workInProgress.lanes = SomeRetryLane;
+            // to get it started back up to attempt the next item. If we can show
+            // them, then they really have the same priority as this render.
+            // So we'll pick it back up the very next render pass once we've had
+            // an opportunity to yield for paint.
+            workInProgress.lanes = renderLanes;
             if (enableSchedulerTracing) {
-              markSpawnedWork(SomeRetryLane);
+              markSpawnedWork(renderLanes);
             }
           }
         }
@@ -1164,6 +1136,18 @@ function completeWork(
 
       if (renderState.tail !== null) {
         // We still have tail rows to render.
+        if (renderState.tailExpiration === 0) {
+          // Heuristic for how long we're willing to spend rendering rows
+          // until we just give up and show what we have so far.
+          const TAIL_EXPIRATION_TIMEOUT_MS = 500;
+          renderState.tailExpiration = now() + TAIL_EXPIRATION_TIMEOUT_MS;
+          // TODO: This is meant to mimic the train model or JND but this
+          // is a per component value. It should really be since the start
+          // of the total render or last commit. Consider using something like
+          // globalMostRecentFallbackTime. That doesn't account for being
+          // suspended for part of the time or when it's a new render.
+          // It should probably use a global start time value instead.
+        }
         // Pop a row.
         const next = renderState.tail;
         renderState.rendering = next;
diff --git a/packages/react-reconciler/src/ReactFiberLane.js b/packages/react-reconciler/src/ReactFiberLane.js
@@ -91,8 +91,6 @@ const TransitionLanes: Lanes = /*                       */ 0b0000000001111111110
 
 const RetryLanes: Lanes = /*                            */ 0b0000011110000000000000000000000;
 
-export const SomeRetryLane: Lanes = /*                  */ 0b0000010000000000000000000000000;
-
 export const SelectiveHydrationLane: Lane = /*          */ 0b0000100000000000000000000000000;
 
 const NonIdleLanes = /*                                 */ 0b0000111111111111111111111111111;
diff --git a/packages/react-reconciler/src/ReactFiberSuspenseComponent.new.js b/packages/react-reconciler/src/ReactFiberSuspenseComponent.new.js
@@ -41,12 +41,14 @@ export type SuspenseListRenderState = {|
   isBackwards: boolean,
   // The currently rendering tail row.
   rendering: null | Fiber,
-  // The absolute time when we started rendering the most recent tail row.
+  // The absolute time when we started rendering the tail row.
   renderingStartTime: number,
   // The last of the already rendered children.
   last: null | Fiber,
   // Remaining rows on the tail of the list.
   tail: null | Fiber,
+  // The absolute time in ms that we'll expire the tail rendering.
+  tailExpiration: number,
   // Tail insertions setting.
   tailMode: SuspenseListTailMode,
 |};
diff --git a/packages/react-reconciler/src/ReactFiberSuspenseComponent.old.js b/packages/react-reconciler/src/ReactFiberSuspenseComponent.old.js
@@ -41,12 +41,14 @@ export type SuspenseListRenderState = {|
   isBackwards: boolean,
   // The currently rendering tail row.
   rendering: null | Fiber,
-  // The absolute time when we started rendering the most recent tail row.
+  // The absolute time when we started rendering the tail row.
   renderingStartTime: number,
   // The last of the already rendered children.
   last: null | Fiber,
   // Remaining rows on the tail of the list.
   tail: null | Fiber,
+  // The absolute time in ms that we'll expire the tail rendering.
+  tailExpiration: number,
   // Tail insertions setting.
   tailMode: SuspenseListTailMode,
   // Last Effect before we rendered the "rendering" item.
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
@@ -323,21 +323,6 @@ let mostRecentlyUpdatedRoot: FiberRoot | null = null;
 let globalMostRecentFallbackTime: number = 0;
 const FALLBACK_THROTTLE_MS: number = 500;
 
-// The absolute time for when we should start giving up on rendering
-// more and prefer CPU suspense heuristics instead.
-let workInProgressRootRenderTargetTime: number = Infinity;
-// How long a render is supposed to take before we start following CPU
-// suspense heuristics and opt out of rendering more content.
-const RENDER_TIMEOUT_MS = 500;
-
-function resetRenderTimer() {
-  workInProgressRootRenderTargetTime = now() + RENDER_TIMEOUT_MS;
-}
-
-export function getRenderTargetTime(): number {
-  return workInProgressRootRenderTargetTime;
-}
-
 let hasUncaughtError = false;
 let firstUncaughtError = null;
 let legacyErrorBoundariesThatAlreadyFailed: Set<mixed> | null = null;
@@ -597,7 +582,6 @@ export function scheduleUpdateOnFiber(
         // scheduleCallbackForFiber to preserve the ability to schedule a callback
         // without immediately flushing it. We only do this for user-initiated
         // updates, to preserve historical behavior of legacy mode.
-        resetRenderTimer();
         flushSyncCallbackQueue();
       }
     }
@@ -1061,7 +1045,6 @@ export function flushRoot(root: FiberRoot, lanes: Lanes) {
   markRootExpired(root, lanes);
   ensureRootIsScheduled(root, now());
   if ((executionContext & (RenderContext | CommitContext)) === NoContext) {
-    resetRenderTimer();
     flushSyncCallbackQueue();
   }
 }
@@ -1136,7 +1119,6 @@ export function batchedUpdates<A, R>(fn: A => R, a: A): R {
     executionContext = prevExecutionContext;
     if (executionContext === NoContext) {
       // Flush the immediate callbacks that were scheduled during this batch
-      resetRenderTimer();
       flushSyncCallbackQueue();
     }
   }
@@ -1151,7 +1133,6 @@ export function batchedEventUpdates<A, R>(fn: A => R, a: A): R {
     executionContext = prevExecutionContext;
     if (executionContext === NoContext) {
       // Flush the immediate callbacks that were scheduled during this batch
-      resetRenderTimer();
       flushSyncCallbackQueue();
     }
   }
@@ -1180,7 +1161,6 @@ export function discreteUpdates<A, B, C, D, R>(
       executionContext = prevExecutionContext;
       if (executionContext === NoContext) {
         // Flush the immediate callbacks that were scheduled during this batch
-        resetRenderTimer();
         flushSyncCallbackQueue();
       }
     }
@@ -1194,7 +1174,6 @@ export function discreteUpdates<A, B, C, D, R>(
       executionContext = prevExecutionContext;
       if (executionContext === NoContext) {
         // Flush the immediate callbacks that were scheduled during this batch
-        resetRenderTimer();
         flushSyncCallbackQueue();
       }
     }
@@ -1211,7 +1190,6 @@ export function unbatchedUpdates<A, R>(fn: (a: A) => R, a: A): R {
     executionContext = prevExecutionContext;
     if (executionContext === NoContext) {
       // Flush the immediate callbacks that were scheduled during this batch
-      resetRenderTimer();
       flushSyncCallbackQueue();
     }
   }
@@ -1279,7 +1257,6 @@ export function flushControlled(fn: () => mixed): void {
       executionContext = prevExecutionContext;
       if (executionContext === NoContext) {
         // Flush the immediate callbacks that were scheduled during this batch
-        resetRenderTimer();
         flushSyncCallbackQueue();
       }
     }
@@ -1290,7 +1267,6 @@ export function flushControlled(fn: () => mixed): void {
       executionContext = prevExecutionContext;
       if (executionContext === NoContext) {
         // Flush the immediate callbacks that were scheduled during this batch
-        resetRenderTimer();
         flushSyncCallbackQueue();
       }
     }
@@ -1583,7 +1559,6 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) {
   // If the root or lanes have changed, throw out the existing stack
   // and prepare a fresh one. Otherwise we'll continue where we left off.
   if (workInProgressRoot !== root || workInProgressRootRenderLanes !== lanes) {
-    resetRenderTimer();
     prepareFreshStack(root, lanes);
     startWorkOnPendingInteractions(root, lanes);
   }
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js
diff --git a/packages/react/src/__tests__/ReactProfiler-test.internal.js b/packages/react/src/__tests__/ReactProfiler-test.internal.js

Original file line number	Diff line number	Diff line change
`@@ -323,21 +323,6 @@ let mostRecentlyUpdatedRoot: FiberRoot \| null = null;`
`323`	`323`	`let globalMostRecentFallbackTime: number = 0;`
`324`	`324`	`const FALLBACK_THROTTLE_MS: number = 500;`
`325`	`325`
`326`		`-// The absolute time for when we should start giving up on rendering`
`327`		`-// more and prefer CPU suspense heuristics instead.`
`328`		`-let workInProgressRootRenderTargetTime: number = Infinity;`
`329`		`-// How long a render is supposed to take before we start following CPU`
`330`		`-// suspense heuristics and opt out of rendering more content.`
`331`		`-const RENDER_TIMEOUT_MS = 500;`
`332`		`-`
`333`		`-function resetRenderTimer() {`
`334`		`- workInProgressRootRenderTargetTime = now() + RENDER_TIMEOUT_MS;`
`335`		`-}`
`336`		`-`
`337`		`-export function getRenderTargetTime(): number {`
`338`		`- return workInProgressRootRenderTargetTime;`
`339`		`-}`
`340`		`-`
`341`	`326`	`let hasUncaughtError = false;`
`342`	`327`	`let firstUncaughtError = null;`
`343`	`328`	`let legacyErrorBoundariesThatAlreadyFailed: Set<mixed> \| null = null;`
`@@ -597,7 +582,6 @@ export function scheduleUpdateOnFiber(`
`597`	`582`	`// scheduleCallbackForFiber to preserve the ability to schedule a callback`
`598`	`583`	`// without immediately flushing it. We only do this for user-initiated`
`599`	`584`	`// updates, to preserve historical behavior of legacy mode.`
`600`		`- resetRenderTimer();`
`601`	`585`	`flushSyncCallbackQueue();`
`602`	`586`	`}`
`603`	`587`	`}`
`@@ -1061,7 +1045,6 @@ export function flushRoot(root: FiberRoot, lanes: Lanes) {`
`1061`	`1045`	`markRootExpired(root, lanes);`
`1062`	`1046`	`ensureRootIsScheduled(root, now());`
`1063`	`1047`	`if ((executionContext & (RenderContext \| CommitContext)) === NoContext) {`
`1064`		`- resetRenderTimer();`
`1065`	`1048`	`flushSyncCallbackQueue();`
`1066`	`1049`	`}`
`1067`	`1050`	`}`
`@@ -1136,7 +1119,6 @@ export function batchedUpdates<A, R>(fn: A => R, a: A): R {`
`1136`	`1119`	`executionContext = prevExecutionContext;`
`1137`	`1120`	`if (executionContext === NoContext) {`
`1138`	`1121`	`// Flush the immediate callbacks that were scheduled during this batch`
`1139`		`- resetRenderTimer();`
`1140`	`1122`	`flushSyncCallbackQueue();`
`1141`	`1123`	`}`
`1142`	`1124`	`}`
`@@ -1151,7 +1133,6 @@ export function batchedEventUpdates<A, R>(fn: A => R, a: A): R {`
`1151`	`1133`	`executionContext = prevExecutionContext;`
`1152`	`1134`	`if (executionContext === NoContext) {`
`1153`	`1135`	`// Flush the immediate callbacks that were scheduled during this batch`
`1154`		`- resetRenderTimer();`
`1155`	`1136`	`flushSyncCallbackQueue();`
`1156`	`1137`	`}`
`1157`	`1138`	`}`
`@@ -1180,7 +1161,6 @@ export function discreteUpdates<A, B, C, D, R>(`
`1180`	`1161`	`executionContext = prevExecutionContext;`
`1181`	`1162`	`if (executionContext === NoContext) {`
`1182`	`1163`	`// Flush the immediate callbacks that were scheduled during this batch`
`1183`		`- resetRenderTimer();`
`1184`	`1164`	`flushSyncCallbackQueue();`
`1185`	`1165`	`}`
`1186`	`1166`	`}`
`@@ -1194,7 +1174,6 @@ export function discreteUpdates<A, B, C, D, R>(`
`1194`	`1174`	`executionContext = prevExecutionContext;`
`1195`	`1175`	`if (executionContext === NoContext) {`
`1196`	`1176`	`// Flush the immediate callbacks that were scheduled during this batch`
`1197`		`- resetRenderTimer();`
`1198`	`1177`	`flushSyncCallbackQueue();`
`1199`	`1178`	`}`
`1200`	`1179`	`}`
`@@ -1211,7 +1190,6 @@ export function unbatchedUpdates<A, R>(fn: (a: A) => R, a: A): R {`
`1211`	`1190`	`executionContext = prevExecutionContext;`
`1212`	`1191`	`if (executionContext === NoContext) {`
`1213`	`1192`	`// Flush the immediate callbacks that were scheduled during this batch`
`1214`		`- resetRenderTimer();`
`1215`	`1193`	`flushSyncCallbackQueue();`
`1216`	`1194`	`}`
`1217`	`1195`	`}`
`@@ -1279,7 +1257,6 @@ export function flushControlled(fn: () => mixed): void {`
`1279`	`1257`	`executionContext = prevExecutionContext;`
`1280`	`1258`	`if (executionContext === NoContext) {`
`1281`	`1259`	`// Flush the immediate callbacks that were scheduled during this batch`
`1282`		`- resetRenderTimer();`
`1283`	`1260`	`flushSyncCallbackQueue();`
`1284`	`1261`	`}`
`1285`	`1262`	`}`
`@@ -1290,7 +1267,6 @@ export function flushControlled(fn: () => mixed): void {`
`1290`	`1267`	`executionContext = prevExecutionContext;`
`1291`	`1268`	`if (executionContext === NoContext) {`
`1292`	`1269`	`// Flush the immediate callbacks that were scheduled during this batch`
`1293`		`- resetRenderTimer();`
`1294`	`1270`	`flushSyncCallbackQueue();`
`1295`	`1271`	`}`
`1296`	`1272`	`}`
`@@ -1583,7 +1559,6 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) {`
`1583`	`1559`	`// If the root or lanes have changed, throw out the existing stack`
`1584`	`1560`	`// and prepare a fresh one. Otherwise we'll continue where we left off.`
`1585`	`1561`	`if (workInProgressRoot !== root \|\| workInProgressRootRenderLanes !== lanes) {`
`1586`		`- resetRenderTimer();`
`1587`	`1562`	`prepareFreshStack(root, lanes);`
`1588`	`1563`	`startWorkOnPendingInteractions(root, lanes);`
`1589`	`1564`	`}`