Expire rendering the tail of SuspenseList after a timeout (#15946)

* Expire rendering the tail of SuspenseList after a timeout

This is the first Suspense feature that isn't actually dependent on IO.

The thinking here is that it's normal for a SuspenseList to show loading
states, and it'll be designed to handle it one at a time.

However, sometimes there are lists with really big items that take a long
time to CPU render. Since data can become available as we do that, it is
likely that we have all the data and become CPU bound.

In that case, the list would naively just render until the end and then
display all items at once. I think that's actually what you want for fast
lists. However, for slow ones (like News Feed), you're better off showing
a few rows at a time.

It's not necessarily one at a time because if you can do many in a short
period of time and fit them all on the screen, then it's better to do them
all at once than pop them in one at a time very quickly.

Therefore, I use a heuristic of trying to render as many rows as I can in
500ms before giving up.

This timer starts before the first row of the tail and we only check it
after. This ensures that we always make a little progress each attempt.
An alternative approach could be to start the time before doing the head
of the list but we don't want that being slow prevent us from making
further progress.

Currently, I disable this optimization at Never priority because there's
nothing intermediate that becomes visible anyway.

* Fix tracing through a SuspenseList

This ensures that we can spawn new work during render through arbitrary
priorities.

We'll need this for other features too.

Since each priority can commit separately we need to use an array to
include the current interactions on each priority.

Author: sebmarkbage

packages/react-reconciler/src/ReactFiberBeginWork.js

@@ -173,7 +173,7 @@ import {
   isSimpleFunctionComponent,
 } from './ReactFiber';
 import {
-  markDidDeprioritizeIdleSubtree,
+  markSpawnedWork,
   requestCurrentTime,
   retryTimedOutBoundary,
 } from './ReactFiberWorkLoop';
@@ -1006,7 +1006,7 @@ function updateHostComponent(current, workInProgress, renderExpirationTime) {
     shouldDeprioritizeSubtree(type, nextProps)
   ) {
     if (enableSchedulerTracing) {
-      markDidDeprioritizeIdleSubtree();
+      markSpawnedWork(Never);
     }
     // Schedule this fiber to re-render at offscreen priority. Then bailout.
     workInProgress.expirationTime = workInProgress.childExpirationTime = Never;
@@ -2069,6 +2069,7 @@ function updateSuspenseListComponent(
       rendering: null,
       last: null,
       tail: null,
+      tailExpiration: 0,
     };
   } else {
     let didForceFallback =
@@ -2124,6 +2125,7 @@ function updateSuspenseListComponent(
             rendering: null,
             last: lastContentRow,
             tail: tail,
+            tailExpiration: 0,
           };
         } else {
           suspenseListState.tail = tail;
@@ -2164,6 +2166,7 @@ function updateSuspenseListComponent(
             rendering: null,
             last: null,
             tail: tail,
+            tailExpiration: 0,
           };
         } else {
           suspenseListState.isBackwards = true;
@@ -2595,7 +2598,7 @@ function beginWork(
             shouldDeprioritizeSubtree(workInProgress.type, newProps)
           ) {
             if (enableSchedulerTracing) {
-              markDidDeprioritizeIdleSubtree();
+              markSpawnedWork(Never);
             }
             // Schedule this fiber to re-render at offscreen priority. Then bailout.
             workInProgress.expirationTime = workInProgress.childExpirationTime = Never;

packages/react-reconciler/src/ReactFiberCompleteWork.js

@@ -24,6 +24,8 @@ import type {
 } from './ReactFiberSuspenseComponent';
 import type {SuspenseContext} from './ReactFiberSuspenseContext';
 
+import {now} from './SchedulerWithReactIntegration';
+
 import {
   IndeterminateComponent,
   FunctionComponent,
@@ -111,7 +113,7 @@ import {
   enableEventAPI,
 } from 'shared/ReactFeatureFlags';
 import {
-  markDidDeprioritizeIdleSubtree,
+  markSpawnedWork,
   renderDidSuspend,
   renderDidSuspendDelayIfPossible,
 } from './ReactFiberWorkLoop';
@@ -121,6 +123,7 @@ import {
 } from './ReactFiberEvents';
 import getComponentName from 'shared/getComponentName';
 import warning from 'shared/warning';
+import {Never} from './ReactFiberExpirationTime';
 
 function markUpdate(workInProgress: Fiber) {
   // Tag the fiber with an update effect. This turns a Placement into
@@ -908,7 +911,7 @@ function completeWork(
               'This is probably a bug in React.',
           );
           if (enableSchedulerTracing) {
-            markDidDeprioritizeIdleSubtree();
+            markSpawnedWork(Never);
           }
           skipPastDehydratedSuspenseInstance(workInProgress);
         } else if ((workInProgress.effectTag & DidCapture) === NoEffect) {
@@ -958,7 +961,28 @@ function completeWork(
           // Append the rendered row to the child list.
           let rendered = suspenseListState.rendering;
           if (!suspenseListState.didSuspend) {
-            suspenseListState.didSuspend = isShowingAnyFallbacks(rendered);
+            if (
+              now() > suspenseListState.tailExpiration &&
+              renderExpirationTime > Never
+            ) {
+              // We have now passed our CPU deadline and we'll just give up further
+              // attempts to render the main content and only render fallbacks.
+              // The assumption is that this is usually faster.
+              suspenseListState.didSuspend = true;
+              // Since nothing actually suspended, there will nothing to ping this
+              // 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.
+
+              const nextPriority = renderExpirationTime - 1;
+              workInProgress.expirationTime = workInProgress.childExpirationTime = nextPriority;
+              if (enableSchedulerTracing) {
+                markSpawnedWork(nextPriority);
+              }
+            } else {
+              suspenseListState.didSuspend = isShowingAnyFallbacks(rendered);
+            }
           }
           if (suspenseListState.isBackwards) {
             // The effect list of the backwards tail will have been added
@@ -981,6 +1005,13 @@ function completeWork(
 
         if (suspenseListState !== null && suspenseListState.tail !== null) {
           // We still have tail rows to render.
+          if (suspenseListState.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;
+            suspenseListState.tailExpiration =
+              now() + TAIL_EXPIRATION_TIMEOUT_MS;
+          }
           // Pop a row.
           let next = suspenseListState.tail;
           suspenseListState.rendering = next;

packages/react-reconciler/src/ReactFiberSuspenseComponent.js

@@ -22,6 +22,8 @@ export type SuspenseListState = {|
   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,
 |};
 
 export function shouldCaptureSuspense(

packages/react-reconciler/src/ReactFiberWorkLoop.js

@@ -246,9 +246,11 @@ let nestedPassiveUpdateCount: number = 0;
 
 let interruptedBy: Fiber | null = null;
 
-// Marks the need to reschedule pending interactions at Never priority during the commit phase.
-// This enables them to be traced accross hidden boundaries or suspended SSR hydration.
-let didDeprioritizeIdleSubtree: boolean = false;
+// Marks the need to reschedule pending interactions at these expiration times
+// during the commit phase. This enables them to be traced across components
+// that spawn new work during render. E.g. hidden boundaries, suspended SSR
+// hydration or SuspenseList.
+let spawnedWorkDuringRender: null | Array<ExpirationTime> = null;
 
 // Expiration times are computed by adding to the current time (the start
 // time). However, if two updates are scheduled within the same event, we
@@ -785,7 +787,7 @@ function prepareFreshStack(root, expirationTime) {
   workInProgressRootHasPendingPing = false;
 
   if (enableSchedulerTracing) {
-    didDeprioritizeIdleSubtree = false;
+    spawnedWorkDuringRender = null;
   }
 
   if (__DEV__) {
@@ -1707,9 +1709,16 @@ function commitRootImpl(root) {
     );
 
     if (enableSchedulerTracing) {
-      if (didDeprioritizeIdleSubtree) {
-        didDeprioritizeIdleSubtree = false;
-        scheduleInteractions(root, Never, root.memoizedInteractions);
+      if (spawnedWorkDuringRender !== null) {
+        const expirationTimes = spawnedWorkDuringRender;
+        spawnedWorkDuringRender = null;
+        for (let i = 0; i < expirationTimes.length; i++) {
+          scheduleInteractions(
+            root,
+            expirationTimes[i],
+            root.memoizedInteractions,
+          );
+        }
       }
     }
 
@@ -2532,11 +2541,15 @@ function computeThreadID(root, expirationTime) {
   return expirationTime * 1000 + root.interactionThreadID;
 }
 
-export function markDidDeprioritizeIdleSubtree() {
+export function markSpawnedWork(expirationTime: ExpirationTime) {
   if (!enableSchedulerTracing) {
     return;
   }
-  didDeprioritizeIdleSubtree = true;
+  if (spawnedWorkDuringRender === null) {
+    spawnedWorkDuringRender = [expirationTime];
+  } else {
+    spawnedWorkDuringRender.push(expirationTime);
+  }
 }
 
 function scheduleInteractions(root, expirationTime, interactions) {

packages/react-reconciler/src/__tests__/ReactSuspenseList-test.internal.js

@@ -1027,4 +1027,66 @@ describe('ReactSuspenseList', () => {
       </Fragment>,
     );
   });
+
+  it('switches to rendering fallbacks if the tail takes long CPU time', async () => {
+    function Foo() {
+      return (
+        <SuspenseList revealOrder="forwards">
+          <Suspense fallback={<Text text="Loading A" />}>
+            <Text text="A" />
+          </Suspense>
+          <Suspense fallback={<Text text="Loading B" />}>
+            <Text text="B" />
+          </Suspense>
+          <Suspense fallback={<Text text="Loading C" />}>
+            <Text text="C" />
+          </Suspense>
+        </SuspenseList>
+      );
+    }
+
+    // This render is only CPU bound. Nothing suspends.
+    ReactNoop.render(<Foo />);
+
+    expect(Scheduler).toFlushAndYieldThrough(['A']);
+
+    Scheduler.advanceTime(300);
+    jest.advanceTimersByTime(300);
+
+    expect(Scheduler).toFlushAndYieldThrough(['B']);
+
+    Scheduler.advanceTime(300);
+    jest.advanceTimersByTime(300);
+
+    // We've still not been able to show anything on the screen even though
+    // we have two items ready.
+    expect(ReactNoop).toMatchRenderedOutput(null);
+
+    // Time has now elapsed for so long that we're just going to give up
+    // rendering the rest of the content. So that we can at least show
+    // something.
+    expect(Scheduler).toFlushAndYieldThrough([
+      'Loading C',
+      'C', // I'll flush through into the next render so that the first commits.
+    ]);
+
+    expect(ReactNoop).toMatchRenderedOutput(
+      <Fragment>
+        <span>A</span>
+        <span>B</span>
+        <span>Loading C</span>
+      </Fragment>,
+    );
+
+    // Then we do a second pass to commit the last item.
+    expect(Scheduler).toFlushAndYield([]);
+
+    expect(ReactNoop).toMatchRenderedOutput(
+      <Fragment>
+        <span>A</span>
+        <span>B</span>
+        <span>C</span>
+      </Fragment>,
+    );
+  });
 });

packages/react/src/__tests__/ReactDOMTracing-test.internal.js

@@ -507,6 +507,85 @@ describe('ReactDOMTracing', () => {
           );
         });
       });
+
+      it('should properly trace interactions through a multi-pass SuspenseList render', () => {
+        const SuspenseList = React.unstable_SuspenseList;
+        const Suspense = React.Suspense;
+        function Text({text}) {
+          Scheduler.yieldValue(text);
+          React.useEffect(() => {
+            Scheduler.yieldValue('Commit ' + text);
+          });
+          return <span>{text}</span>;
+        }
+        function App() {
+          return (
+            <SuspenseList revealOrder="forwards">
+              <Suspense fallback={<Text text="Loading A" />}>
+                <Text text="A" />
+              </Suspense>
+              <Suspense fallback={<Text text="Loading B" />}>
+                <Text text="B" />
+              </Suspense>
+              <Suspense fallback={<Text text="Loading C" />}>
+                <Text text="C" />
+              </Suspense>
+            </SuspenseList>
+          );
+        }
+
+        const container = document.createElement('div');
+        const root = ReactDOM.unstable_createRoot(container);
+
+        let interaction;
+        SchedulerTracing.unstable_trace('initialization', 0, () => {
+          interaction = Array.from(SchedulerTracing.unstable_getCurrent())[0];
+          // This render is only CPU bound. Nothing suspends.
+          root.render(<App />);
+        });
+
+        expect(Scheduler).toFlushAndYieldThrough(['A']);
+
+        Scheduler.advanceTime(300);
+        jest.advanceTimersByTime(300);
+
+        expect(Scheduler).toFlushAndYieldThrough(['B']);
+
+        Scheduler.advanceTime(300);
+        jest.advanceTimersByTime(300);
+
+        // Time has now elapsed for so long that we're just going to give up
+        // rendering the rest of the content. So that we can at least show
+        // something.
+        expect(Scheduler).toFlushAndYieldThrough([
+          'Loading C',
+          'Commit A',
+          'Commit B',
+          'Commit Loading C',
+        ]);
+
+        // Schedule an unrelated low priority update that shouldn't be included
+        // in the previous interaction. This is meant to ensure that we don't
+        // rely on the whole tree completing to cover up bugs.
+        Scheduler.unstable_runWithPriority(
+          Scheduler.unstable_IdlePriority,
+          () => root.render(<App />),
+        );
+
+        expect(onInteractionTraced).toHaveBeenCalledTimes(1);
+        expect(onInteractionTraced).toHaveBeenLastNotifiedOfInteraction(
+          interaction,
+        );
+        expect(onInteractionScheduledWorkCompleted).not.toHaveBeenCalled();
+
+        // Then we do a second pass to commit the last item.
+        expect(Scheduler).toFlushAndYieldThrough(['C', 'Commit C']);
+
+        expect(onInteractionScheduledWorkCompleted).toHaveBeenCalledTimes(1);
+        expect(
+          onInteractionScheduledWorkCompleted,
+        ).toHaveBeenLastNotifiedOfInteraction(interaction);
+      });
     });
 
     describe('hydration', () => {