Skip to content

Commit 3d77108

Browse files
committed
Adjust SuspenseList CPU bound heuristic
In SuspenseList we switch to rendering fallbacks (or stop rendering further rows in the case of tail="collapsed/hidden") if it takes more than 500ms to render the list. The limit of 500ms is similar to the train model and designed to be short enough to be in the not noticeable range. This works well if each row is small because we time the 500ms range well. However, if we have a few large rows then we're likely to exceed the limit by a lot. E.g. two 480ms rows hits almost a second instead of 500ms. This PR adjusts the heuristic to instead compute whether something has expired based on the render time of the last row. I.e. if we think rendering one more row would exceed the timeout, then we don't attempt. This still works well for small rows and bails earlier for large rows. The expiration is still based on the start of the list rather than the start of the render. It should probably be based on the start of the render but that's a bigger change and needs some thought.
1 parent b64938e commit 3d77108

File tree

5 files changed

+19
-7
lines changed

5 files changed

+19
-7
lines changed

packages/react-reconciler/src/ReactFiberBeginWork.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2358,6 +2358,7 @@ function initSuspenseListRenderState(
23582358
workInProgress.memoizedState = ({
23592359
isBackwards: isBackwards,
23602360
rendering: null,
2361+
renderingStartTime: 0,
23612362
last: lastContentRow,
23622363
tail: tail,
23632364
tailExpiration: 0,
@@ -2368,6 +2369,7 @@ function initSuspenseListRenderState(
23682369
// We can reuse the existing object from previous renders.
23692370
renderState.isBackwards = isBackwards;
23702371
renderState.rendering = null;
2372+
renderState.renderingStartTime = 0;
23712373
renderState.last = lastContentRow;
23722374
renderState.tail = tail;
23732375
renderState.tailExpiration = 0;

packages/react-reconciler/src/ReactFiberCompleteWork.js

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1114,7 +1114,8 @@ function completeWork(
11141114
return null;
11151115
}
11161116
} else if (
1117-
now() > renderState.tailExpiration &&
1117+
now() * 2 - renderState.renderingStartTime >
1118+
renderState.tailExpiration &&
11181119
renderExpirationTime > Never
11191120
) {
11201121
// We have now passed our CPU deadline and we'll just give up further
@@ -1164,12 +1165,19 @@ function completeWork(
11641165
// until we just give up and show what we have so far.
11651166
const TAIL_EXPIRATION_TIMEOUT_MS = 500;
11661167
renderState.tailExpiration = now() + TAIL_EXPIRATION_TIMEOUT_MS;
1168+
// TODO: This is meant to mimic the train model or JND but this
1169+
// is a per component value. It should really be since the start
1170+
// of the total render or last commit. Consider using something like
1171+
// globalMostRecentFallbackTime. That doesn't account for being
1172+
// suspended for part of the time or when it's a new render.
1173+
// It should probably use a global start time value instead.
11671174
}
11681175
// Pop a row.
11691176
let next = renderState.tail;
11701177
renderState.rendering = next;
11711178
renderState.tail = next.sibling;
11721179
renderState.lastEffect = workInProgress.lastEffect;
1180+
renderState.renderingStartTime = now();
11731181
next.sibling = null;
11741182

11751183
// Restore the context.

packages/react-reconciler/src/ReactFiberSuspenseComponent.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ export type SuspenseListRenderState = {|
4747
isBackwards: boolean,
4848
// The currently rendering tail row.
4949
rendering: null | Fiber,
50+
// The absolute time when we started rendering the tail row.
51+
renderingStartTime: number,
5052
// The last of the already rendered children.
5153
last: null | Fiber,
5254
// Remaining rows on the tail of the list.

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1233,8 +1233,8 @@ describe('ReactSuspenseList', () => {
12331233

12341234
expect(Scheduler).toFlushAndYieldThrough(['A']);
12351235

1236-
Scheduler.unstable_advanceTime(300);
1237-
jest.advanceTimersByTime(300);
1236+
Scheduler.unstable_advanceTime(200);
1237+
jest.advanceTimersByTime(200);
12381238

12391239
expect(Scheduler).toFlushAndYieldThrough(['B']);
12401240

@@ -1407,8 +1407,8 @@ describe('ReactSuspenseList', () => {
14071407

14081408
expect(Scheduler).toFlushAndYieldThrough(['A']);
14091409

1410-
Scheduler.unstable_advanceTime(300);
1411-
jest.advanceTimersByTime(300);
1410+
Scheduler.unstable_advanceTime(200);
1411+
jest.advanceTimersByTime(200);
14121412

14131413
expect(Scheduler).toFlushAndYieldThrough(['B']);
14141414

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -561,8 +561,8 @@ describe('ReactDOMTracing', () => {
561561

562562
expect(Scheduler).toFlushAndYieldThrough(['A']);
563563

564-
Scheduler.unstable_advanceTime(300);
565-
jest.advanceTimersByTime(300);
564+
Scheduler.unstable_advanceTime(200);
565+
jest.advanceTimersByTime(200);
566566

567567
expect(Scheduler).toFlushAndYieldThrough(['B']);
568568

0 commit comments

Comments
 (0)