@samitouri / QOS-React-2 / commits / d77d12510b

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.

Sebastian Markbåge committed Jun 21, 2019 at 18:05 UTC d77d12510b1a1c37484d771a323e0a02cbeb9ba7

6 files changed +205 -15

packages/react-reconciler/src/ReactFiberBeginWork.js

+6 -3

@@ -173,7 +173,7 @@ import {

173 isSimpleFunctionComponent,

174 } from './ReactFiber';

175 import {

176 - markDidDeprioritizeIdleSubtree,

176 + markSpawnedWork,

177 requestCurrentTime,

178 retryTimedOutBoundary,

179 } from './ReactFiberWorkLoop';

@@ -1006,7 +1006,7 @@ function updateHostComponent(current, workInProgress, renderExpirationTime) {

1006 shouldDeprioritizeSubtree(type, nextProps)

1007 ) {

1008 if (enableSchedulerTracing) {

1009 - markDidDeprioritizeIdleSubtree();

1009 + markSpawnedWork(Never);

1010 }

1011 // Schedule this fiber to re-render at offscreen priority. Then bailout.

1012 workInProgress.expirationTime = workInProgress.childExpirationTime = Never;

@@ -2069,6 +2069,7 @@ function updateSuspenseListComponent(

2069 rendering: null,

2070 last: null,

2071 tail: null,

2072 + tailExpiration: 0,

2073 };

2074 } else {

2075 let didForceFallback =

@@ -2124,6 +2125,7 @@ function updateSuspenseListComponent(

2125 rendering: null,

2126 last: lastContentRow,

2127 tail: tail,

2128 + tailExpiration: 0,

2129 };

2130 } else {

2131 suspenseListState.tail = tail;

@@ -2164,6 +2166,7 @@ function updateSuspenseListComponent(

2166 rendering: null,

2167 last: null,

2168 tail: tail,

2169 + tailExpiration: 0,

2170 };

2171 } else {

2172 suspenseListState.isBackwards = true;

@@ -2595,7 +2598,7 @@ function beginWork(

2598 shouldDeprioritizeSubtree(workInProgress.type, newProps)

2599 ) {

2600 if (enableSchedulerTracing) {

2598 - markDidDeprioritizeIdleSubtree();

2601 + markSpawnedWork(Never);

2602 }

2603 // Schedule this fiber to re-render at offscreen priority. Then bailout.

2604 workInProgress.expirationTime = workInProgress.childExpirationTime = Never;

packages/react-reconciler/src/ReactFiberCompleteWork.js

+34 -3

@@ -24,6 +24,8 @@ import type {

24 } from './ReactFiberSuspenseComponent';

25 import type {SuspenseContext} from './ReactFiberSuspenseContext';

27 +import {now} from './SchedulerWithReactIntegration';

28 +

29 import {

30 IndeterminateComponent,

31 FunctionComponent,

@@ -111,7 +113,7 @@ import {

113 enableEventAPI,

114 } from 'shared/ReactFeatureFlags';

115 import {

114 - markDidDeprioritizeIdleSubtree,

116 + markSpawnedWork,

117 renderDidSuspend,

118 renderDidSuspendDelayIfPossible,

119 } from './ReactFiberWorkLoop';

@@ -121,6 +123,7 @@ import {

123 } from './ReactFiberEvents';

124 import getComponentName from 'shared/getComponentName';

125 import warning from 'shared/warning';

126 +import {Never} from './ReactFiberExpirationTime';

127

128 function markUpdate(workInProgress: Fiber) {

129 // Tag the fiber with an update effect. This turns a Placement into

@@ -908,7 +911,7 @@ function completeWork(

911 'This is probably a bug in React.',

912 );

913 if (enableSchedulerTracing) {

911 - markDidDeprioritizeIdleSubtree();

914 + markSpawnedWork(Never);

915 }

916 skipPastDehydratedSuspenseInstance(workInProgress);

917 } else if ((workInProgress.effectTag & DidCapture) === NoEffect) {

@@ -958,7 +961,28 @@ function completeWork(

961 // Append the rendered row to the child list.

962 let rendered = suspenseListState.rendering;

963 if (!suspenseListState.didSuspend) {

961 - suspenseListState.didSuspend = isShowingAnyFallbacks(rendered);

964 + if (

965 + now() > suspenseListState.tailExpiration &&

966 + renderExpirationTime > Never

967 + ) {

968 + // We have now passed our CPU deadline and we'll just give up further

969 + // attempts to render the main content and only render fallbacks.

970 + // The assumption is that this is usually faster.

971 + suspenseListState.didSuspend = true;

972 + // Since nothing actually suspended, there will nothing to ping this

973 + // to get it started back up to attempt the next item. If we can show

974 + // them, then they really have the same priority as this render.

975 + // So we'll pick it back up the very next render pass once we've had

976 + // an opportunity to yield for paint.

977 +

978 + const nextPriority = renderExpirationTime - 1;

979 + workInProgress.expirationTime = workInProgress.childExpirationTime = nextPriority;

980 + if (enableSchedulerTracing) {

981 + markSpawnedWork(nextPriority);

982 + }

983 + } else {

984 + suspenseListState.didSuspend = isShowingAnyFallbacks(rendered);

985 + }

986 }

987 if (suspenseListState.isBackwards) {

988 // The effect list of the backwards tail will have been added

@@ -981,6 +1005,13 @@ function completeWork(

1005

1006 if (suspenseListState !== null && suspenseListState.tail !== null) {

1007 // We still have tail rows to render.

1008 + if (suspenseListState.tailExpiration === 0) {

1009 + // Heuristic for how long we're willing to spend rendering rows

1010 + // until we just give up and show what we have so far.

1011 + const TAIL_EXPIRATION_TIMEOUT_MS = 500;

1012 + suspenseListState.tailExpiration =

1013 + now() + TAIL_EXPIRATION_TIMEOUT_MS;

1014 + }

1015 // Pop a row.

1016 let next = suspenseListState.tail;

1017 suspenseListState.rendering = next;

packages/react-reconciler/src/ReactFiberSuspenseComponent.js

@@ -22,6 +22,8 @@ export type SuspenseListState = {|

22 last: null | Fiber,

23 // Remaining rows on the tail of the list.

24 tail: null | Fiber,

25 + // The absolute time in ms that we'll expire the tail rendering.

26 + tailExpiration: number,

27 |};

29 export function shouldCaptureSuspense(

packages/react-reconciler/src/ReactFiberWorkLoop.js

+22 -9

@@ -246,9 +246,11 @@ let nestedPassiveUpdateCount: number = 0;

246

247 let interruptedBy: Fiber | null = null;

248

249 -// Marks the need to reschedule pending interactions at Never priority during the commit phase.

250 -// This enables them to be traced accross hidden boundaries or suspended SSR hydration.

251 -let didDeprioritizeIdleSubtree: boolean = false;

249 +// Marks the need to reschedule pending interactions at these expiration times

250 +// during the commit phase. This enables them to be traced across components

251 +// that spawn new work during render. E.g. hidden boundaries, suspended SSR

252 +// hydration or SuspenseList.

253 +let spawnedWorkDuringRender: null | Array<ExpirationTime> = null;

254

255 // Expiration times are computed by adding to the current time (the start

256 // time). However, if two updates are scheduled within the same event, we

@@ -785,7 +787,7 @@ function prepareFreshStack(root, expirationTime) {

787 workInProgressRootHasPendingPing = false;

788

789 if (enableSchedulerTracing) {

788 - didDeprioritizeIdleSubtree = false;

790 + spawnedWorkDuringRender = null;

791 }

792

793 if (__DEV__) {

@@ -1707,9 +1709,16 @@ function commitRootImpl(root) {

1709 );

1710

1711 if (enableSchedulerTracing) {

1710 - if (didDeprioritizeIdleSubtree) {

1711 - didDeprioritizeIdleSubtree = false;

1712 - scheduleInteractions(root, Never, root.memoizedInteractions);

1712 + if (spawnedWorkDuringRender !== null) {

1713 + const expirationTimes = spawnedWorkDuringRender;

1714 + spawnedWorkDuringRender = null;

1715 + for (let i = 0; i < expirationTimes.length; i++) {

1716 + scheduleInteractions(

1717 + root,

1718 + expirationTimes[i],

1719 + root.memoizedInteractions,

1720 + );

1721 + }

1722 }

1723 }

1724

@@ -2532,11 +2541,15 @@ function computeThreadID(root, expirationTime) {

2541 return expirationTime * 1000 + root.interactionThreadID;

2542 }

2543

2535 -export function markDidDeprioritizeIdleSubtree() {

2544 +export function markSpawnedWork(expirationTime: ExpirationTime) {

2545 if (!enableSchedulerTracing) {

2546 return;

2547 }

2539 - didDeprioritizeIdleSubtree = true;

2548 + if (spawnedWorkDuringRender === null) {

2549 + spawnedWorkDuringRender = [expirationTime];

2550 + } else {

2551 + spawnedWorkDuringRender.push(expirationTime);

2552 + }

2553 }

2554

2555 function scheduleInteractions(root, expirationTime, interactions) {

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

+62

@@ -1027,4 +1027,66 @@ describe('ReactSuspenseList', () => {

1027 </Fragment>,

1028 );

1029 });

1030 +

1031 + it('switches to rendering fallbacks if the tail takes long CPU time', async () => {

1032 + function Foo() {

1033 + return (

1034 + <SuspenseList revealOrder="forwards">

1035 + <Suspense fallback={<Text text="Loading A" />}>

1036 + <Text text="A" />

1037 + </Suspense>

1038 + <Suspense fallback={<Text text="Loading B" />}>

1039 + <Text text="B" />

1040 + </Suspense>

1041 + <Suspense fallback={<Text text="Loading C" />}>

1042 + <Text text="C" />

1043 + </Suspense>

1044 + </SuspenseList>

1045 + );

1046 + }

1047 +

1048 + // This render is only CPU bound. Nothing suspends.

1049 + ReactNoop.render(<Foo />);

1050 +

1051 + expect(Scheduler).toFlushAndYieldThrough(['A']);

1052 +

1053 + Scheduler.advanceTime(300);

1054 + jest.advanceTimersByTime(300);

1055 +

1056 + expect(Scheduler).toFlushAndYieldThrough(['B']);

1057 +

1058 + Scheduler.advanceTime(300);

1059 + jest.advanceTimersByTime(300);

1060 +

1061 + // We've still not been able to show anything on the screen even though

1062 + // we have two items ready.

1063 + expect(ReactNoop).toMatchRenderedOutput(null);

1064 +

1065 + // Time has now elapsed for so long that we're just going to give up

1066 + // rendering the rest of the content. So that we can at least show

1067 + // something.

1068 + expect(Scheduler).toFlushAndYieldThrough([

1069 + 'Loading C',

1070 + 'C', // I'll flush through into the next render so that the first commits.

1071 + ]);

1072 +

1073 + expect(ReactNoop).toMatchRenderedOutput(

1074 + <Fragment>

1075 + A

1076 + B

1077 + Loading C

1078 + </Fragment>,

1079 + );

1080 +

1081 + // Then we do a second pass to commit the last item.

1082 + expect(Scheduler).toFlushAndYield([]);

1083 +

1084 + expect(ReactNoop).toMatchRenderedOutput(

1085 + <Fragment>

1086 + A

1087 + B

1088 + C

1089 + </Fragment>,

1090 + );

1091 + });

1092 });

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

+79

@@ -507,6 +507,85 @@ describe('ReactDOMTracing', () => {

507 );

508 });

509 });

510 +

511 + it('should properly trace interactions through a multi-pass SuspenseList render', () => {

512 + const SuspenseList = React.unstable_SuspenseList;

513 + const Suspense = React.Suspense;

514 + function Text({text}) {

515 + Scheduler.yieldValue(text);

516 + React.useEffect(() => {

517 + Scheduler.yieldValue('Commit ' + text);

518 + });

519 + return {text};

520 + }

521 + function App() {

522 + return (

523 + <SuspenseList revealOrder="forwards">

524 + <Suspense fallback={<Text text="Loading A" />}>

525 + <Text text="A" />

526 + </Suspense>

527 + <Suspense fallback={<Text text="Loading B" />}>

528 + <Text text="B" />

529 + </Suspense>

530 + <Suspense fallback={<Text text="Loading C" />}>

531 + <Text text="C" />

532 + </Suspense>

533 + </SuspenseList>

534 + );

535 + }

536 +

537 + const container = document.createElement('div');

538 + const root = ReactDOM.unstable_createRoot(container);

539 +

540 + let interaction;

541 + SchedulerTracing.unstable_trace('initialization', 0, () => {

542 + interaction = Array.from(SchedulerTracing.unstable_getCurrent())[0];

543 + // This render is only CPU bound. Nothing suspends.

544 + root.render(<App />);

545 + });

546 +

547 + expect(Scheduler).toFlushAndYieldThrough(['A']);

548 +

549 + Scheduler.advanceTime(300);

550 + jest.advanceTimersByTime(300);

551 +

552 + expect(Scheduler).toFlushAndYieldThrough(['B']);

553 +

554 + Scheduler.advanceTime(300);

555 + jest.advanceTimersByTime(300);

556 +

557 + // Time has now elapsed for so long that we're just going to give up

558 + // rendering the rest of the content. So that we can at least show

559 + // something.

560 + expect(Scheduler).toFlushAndYieldThrough([

561 + 'Loading C',

562 + 'Commit A',

563 + 'Commit B',

564 + 'Commit Loading C',

565 + ]);

566 +

567 + // Schedule an unrelated low priority update that shouldn't be included

568 + // in the previous interaction. This is meant to ensure that we don't

569 + // rely on the whole tree completing to cover up bugs.

570 + Scheduler.unstable_runWithPriority(

571 + Scheduler.unstable_IdlePriority,

572 + () => root.render(<App />),

573 + );

574 +

575 + expect(onInteractionTraced).toHaveBeenCalledTimes(1);

576 + expect(onInteractionTraced).toHaveBeenLastNotifiedOfInteraction(

577 + interaction,

578 + );

579 + expect(onInteractionScheduledWorkCompleted).not.toHaveBeenCalled();

580 +

581 + // Then we do a second pass to commit the last item.

582 + expect(Scheduler).toFlushAndYieldThrough(['C', 'Commit C']);

583 +

584 + expect(onInteractionScheduledWorkCompleted).toHaveBeenCalledTimes(1);

585 + expect(

586 + onInteractionScheduledWorkCompleted,

587 + ).toHaveBeenLastNotifiedOfInteraction(interaction);

588 + });

589 });

590

591 describe('hydration', () => {