[Suspense] Change Suspending and Restarting Heuristics (#15769)

* Track most recent commit time of a fallback globally

This value is going to be used to avoid committing too many fallback
states in quick succession. It doesn't really matter where in the tree
that happened.

This means that we now don't really need the concept of SuspenseState
other than has a flag. It could be made cheaper/simpler.

* Change suspense heuristic

This now eagerly commits non-delayed suspended trees, unless they're
only retries in which case they're throttled to 500ms.

* Restart early if we're going to suspend later

* Use the local variable where appropriate

* Make ReactLazy tests less specific on asserting intermediate states

They're not testing the exact states of the suspense boundaries, only
the result. I keep assertions that they're not already resolved early.

* Adjust Profiler tests to the new heuristics

* Update snapshot tests for user timing tests

I also added a blank initial render to ensuree that we cover the suspended
case.

* Adjust Suspense tests to account for new heuristics

Mostly this just means render the Suspense boundary first so that it
becomes an update instead of initial mount.

* Track whether we have a ping on the currently rendering level

If we get a ping on this level but have not yet suspended, we might
still suspend later. In that case we should still restart.

* Add comment about moving markers

We should add this to throwException so we get these markers earlier.
I've had to rewrite tests that test restarting to account for the delayed
restarting heuristic.

Ideally, we should also be able to restart from within throwException if
we're already ready to restart. Right now we wait until the next yield.

* Add test for restarting during throttled retry

* Add test that we don't restart for initial render

* Add Suspense Heuristics as a comment in Throw

Author: sebmarkbage

packages/react-reconciler/src/ReactFiberBeginWork.js

@@ -1413,6 +1413,9 @@ function validateFunctionComponentInDev(workInProgress: Fiber, Component: any) {
   }
 }
 
+// TODO: This is now an empty object. Should we just make it a boolean?
+const SUSPENDED_MARKER: SuspenseState = ({}: any);
+
 function updateSuspenseComponent(
   current,
   workInProgress,
@@ -1440,17 +1443,9 @@ function updateSuspenseComponent(
       (ForceSuspenseFallback: SuspenseContext),
     )
   ) {
-    // This either already captured or is a new mount that was forced into its fallback
-    // state by a parent.
-    const attemptedState: SuspenseState | null = workInProgress.memoizedState;
     // Something in this boundary's subtree already suspended. Switch to
     // rendering the fallback children.
-    nextState = {
-      fallbackExpirationTime:
-        attemptedState !== null
-          ? attemptedState.fallbackExpirationTime
-          : NoWork,
-    };
+    nextState = SUSPENDED_MARKER;
     nextDidTimeout = true;
     workInProgress.effectTag &= ~DidCapture;
   } else {

packages/react-reconciler/src/ReactFiberCommitWork.js

@@ -63,10 +63,6 @@ import invariant from 'shared/invariant';
 import warningWithoutStack from 'shared/warningWithoutStack';
 import warning from 'shared/warning';
 
-import {
-  NoWork,
-  computeAsyncExpirationNoBucket,
-} from './ReactFiberExpirationTime';
 import {onCommitUnmount} from './ReactFiberDevToolsHook';
 import {startPhaseTimer, stopPhaseTimer} from './ReactDebugFiberPerf';
 import {getStackByFiberInDevAndProd} from './ReactCurrentFiber';
@@ -102,8 +98,8 @@ import {
 } from './ReactFiberHostConfig';
 import {
   captureCommitPhaseError,
-  requestCurrentTime,
   resolveRetryThenable,
+  markCommitTimeOfFallback,
 } from './ReactFiberWorkLoop';
 import {
   NoEffect as NoHookEffect,
@@ -1288,16 +1284,7 @@ function commitSuspenseComponent(finishedWork: Fiber) {
   } else {
     newDidTimeout = true;
     primaryChildParent = finishedWork.child;
-    if (newState.fallbackExpirationTime === NoWork) {
-      // If the children had not already timed out, record the time.
-      // This is used to compute the elapsed time during subsequent
-      // attempts to render the children.
-      // We model this as a normal pri expiration time since that's
-      // how we infer start time for updates.
-      newState.fallbackExpirationTime = computeAsyncExpirationNoBucket(
-        requestCurrentTime(),
-      );
-    }
+    markCommitTimeOfFallback();
   }
 
   if (supportsMutation && primaryChildParent !== null) {

packages/react-reconciler/src/ReactFiberCompleteWork.js

@@ -101,7 +101,6 @@ import {
   enableEventAPI,
 } from 'shared/ReactFeatureFlags';
 import {
-  markRenderEventTimeAndConfig,
   renderDidSuspend,
   renderDidSuspendDelayIfPossible,
 } from './ReactFiberWorkLoop';
@@ -702,14 +701,6 @@ function completeWork(
         prevDidTimeout = prevState !== null;
         if (!nextDidTimeout && prevState !== null) {
           // We just switched from the fallback to the normal children.
-
-          // Mark the event time of the switching from fallback to normal children,
-          // based on the start of when we first showed the fallback. This time
-          // was given a normal pri expiration time at the time it was shown.
-          const fallbackExpirationTime: ExpirationTime =
-            prevState.fallbackExpirationTime;
-          markRenderEventTimeAndConfig(fallbackExpirationTime, null);
-
           // Delete the fallback.
           // TODO: Would it be better to store the fallback fragment on
           // the stateNode during the begin phase?
@@ -737,6 +728,13 @@ function completeWork(
         // in the concurrent tree already suspended during this render.
         // This is a known bug.
         if ((workInProgress.mode & BatchedMode) !== NoMode) {
+          // TODO: Move this back to throwException because this is too late
+          // if this is a large tree which is common for initial loads. We
+          // don't know if we should restart a render or not until we get
+          // this marker, and this is too late.
+          // If this render already had a ping or lower pri updates,
+          // and this is the first time we know we're going to suspend we
+          // should be able to immediately restart from within throwException.
           const hasInvisibleChildContext =
             current === null &&
             workInProgress.memoizedProps.unstable_avoidThisFallback !== true;

packages/react-reconciler/src/ReactFiberExpirationTime.js

@@ -83,14 +83,6 @@ export function computeSuspenseExpiration(
   );
 }
 
-// Same as computeAsyncExpiration but without the bucketing logic. This is
-// used to compute timestamps instead of actual expiration times.
-export function computeAsyncExpirationNoBucket(
-  currentTime: ExpirationTime,
-): ExpirationTime {
-  return currentTime - LOW_PRIORITY_EXPIRATION / UNIT_SIZE;
-}
-
 // We intentionally set a higher expiration time for interactive updates in
 // dev than in production.
 //

packages/react-reconciler/src/ReactFiberSuspenseComponent.js

@@ -8,11 +8,9 @@
  */
 
 import type {Fiber} from './ReactFiber';
-import type {ExpirationTime} from './ReactFiberExpirationTime';
 
-export type SuspenseState = {|
-  fallbackExpirationTime: ExpirationTime,
-|};
+// TODO: This is now an empty object. Should we switch this to a boolean?
+export type SuspenseState = {||};
 
 export function shouldCaptureSuspense(
   workInProgress: Fiber,

packages/react-reconciler/src/ReactFiberThrow.js

@@ -275,6 +275,45 @@ function throwException(
 
         // Confirmed that the boundary is in a concurrent mode tree. Continue
         // with the normal suspend path.
+        //
+        // After this we'll use a set of heuristics to determine whether this
+        // render pass will run to completion or restart or "suspend" the commit.
+        // The actual logic for this is spread out in different places.
+        //
+        // This first principle is that if we're going to suspend when we complete
+        // a root, then we should also restart if we get an update or ping that
+        // might unsuspend it, and vice versa. The only reason to suspend is
+        // because you think you might want to restart before committing. However,
+        // it doesn't make sense to restart only while in the period we're suspended.
+        //
+        // Restarting too aggressively is also not good because it starves out any
+        // intermediate loading state. So we use heuristics to determine when.
+
+        // Suspense Heuristics
+        //
+        // If nothing threw a Promise or all the same fallbacks are already showing,
+        // then don't suspend/restart.
+        //
+        // If this is an initial render of a new tree of Suspense boundaries and
+        // those trigger a fallback, then don't suspend/restart. We want to ensure
+        // that we can show the initial loading state as quickly as possible.
+        //
+        // If we hit a "Delayed" case, such as when we'd switch from content back into
+        // a fallback, then we should always suspend/restart. SuspenseConfig applies to
+        // this case. If none is defined, JND is used instead.
+        //
+        // If we're already showing a fallback and it gets "retried", allowing us to show
+        // another level, but there's still an inner boundary that would show a fallback,
+        // then we suspend/restart for 500ms since the last time we showed a fallback
+        // anywhere in the tree. This effectively throttles progressive loading into a
+        // consistent train of commits. This also gives us an opportunity to restart to
+        // get to the completed state slightly earlier.
+        //
+        // If there's ambiguity due to batching it's resolved in preference of:
+        // 1) "delayed", 2) "initial render", 3) "retry".
+        //
+        // We want to ensure that a "busy" state doesn't get force committed. We want to
+        // ensure that new initial loading states can commit as soon as possible.
 
         attachPingListener(root, renderExpirationTime, thenable);