Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Use Global Render Timeout for CPU Suspense #19643

Merged
merged 5 commits into from Aug 20, 2020
Merged

Use Global Render Timeout for CPU Suspense #19643

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2abbea2
Use Retry lane for resuming CPU suspended work
sebmarkbage Aug 19, 2020
80de8e3
Use a global render timeout for CPU suspense heuristics
sebmarkbage Aug 19, 2020
c39f837
Fix profiler test since we're now reading time more often
sebmarkbage Aug 19, 2020
1593a1f
Sync to new reconciler
sebmarkbage Aug 19, 2020
2b8b3fc
Test synchronously rerendering should not render more rows
sebmarkbage Aug 18, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 0 additions & 2 deletions packages/react-reconciler/src/ReactFiberBeginWork.new.js
View file
Open in desktop
Expand Up @@ -2644,7 +2644,6 @@ function initSuspenseListRenderState(
renderingStartTime: 0,
last: lastContentRow,
tail: tail,
tailExpiration: 0,
tailMode: tailMode,
lastEffect: lastEffectBeforeRendering,
}: SuspenseListRenderState);
Expand All @@ -2655,7 +2654,6 @@ function initSuspenseListRenderState(
renderState.renderingStartTime = 0;
renderState.last = lastContentRow;
renderState.tail = tail;
renderState.tailExpiration = 0;
renderState.tailMode = tailMode;
renderState.lastEffect = lastEffectBeforeRendering;
}
Expand Down
2 changes: 0 additions & 2 deletions packages/react-reconciler/src/ReactFiberBeginWork.old.js
View file
Open in desktop
Expand Up @@ -2635,7 +2635,6 @@ function initSuspenseListRenderState(
renderingStartTime: 0,
last: lastContentRow,
tail: tail,
tailExpiration: 0,
tailMode: tailMode,
lastEffect: lastEffectBeforeRendering,
}: SuspenseListRenderState);
Expand All @@ -2646,7 +2645,6 @@ function initSuspenseListRenderState(
renderState.renderingStartTime = 0;
renderState.last = lastContentRow;
renderState.tail = tail;
renderState.tailExpiration = 0;
renderState.tailMode = tailMode;
renderState.lastEffect = lastEffectBeforeRendering;
}
Expand Down
49 changes: 31 additions & 18 deletions packages/react-reconciler/src/ReactFiberCompleteWork.new.js
View file
Open in desktop
Expand Up @@ -137,9 +137,10 @@ import {
renderDidSuspendDelayIfPossible,
renderHasNotSuspendedYet,
popRenderLanes,
getRenderTargetTime,
} from './ReactFiberWorkLoop.new';
import {createFundamentalStateInstance} from './ReactFiberFundamental.new';
import {OffscreenLane} from './ReactFiberLane';
import {OffscreenLane, SomeRetryLane} from './ReactFiberLane';
import {resetChildFibers} from './ReactChildFiber.new';
import {createScopeInstance} from './ReactFiberScope.new';
import {transferActualDuration} from './ReactProfilerTimer.new';
Expand Down Expand Up @@ -1076,6 +1077,29 @@ 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);
}
Expand Down Expand Up @@ -1117,10 +1141,11 @@ function completeWork(
return null;
}
} else if (
// The time it took to render last row is greater than time until
// the expiration.
// 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.
now() * 2 - renderState.renderingStartTime >
renderState.tailExpiration &&
getRenderTargetTime() &&
renderLanes !== OffscreenLane
) {
// We have now passed our CPU deadline and we'll just give up further
Expand All @@ -1136,9 +1161,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 = renderLanes;
workInProgress.lanes = SomeRetryLane;
if (enableSchedulerTracing) {
markSpawnedWork(renderLanes);
markSpawnedWork(SomeRetryLane);
}
}
}
Expand All @@ -1163,18 +1188,6 @@ 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;
Expand Down
60 changes: 38 additions & 22 deletions packages/react-reconciler/src/ReactFiberCompleteWork.old.js
View file
Open in desktop
Expand Up @@ -135,9 +135,10 @@ import {
renderDidSuspendDelayIfPossible,
renderHasNotSuspendedYet,
popRenderLanes,
getRenderTargetTime,
} from './ReactFiberWorkLoop.old';
import {createFundamentalStateInstance} from './ReactFiberFundamental.old';
import {OffscreenLane} from './ReactFiberLane';
import {OffscreenLane, SomeRetryLane} from './ReactFiberLane';
import {resetChildFibers} from './ReactChildFiber.old';
import {createScopeInstance} from './ReactFiberScope.old';
import {transferActualDuration} from './ReactProfilerTimer.old';
Expand Down Expand Up @@ -1049,6 +1050,29 @@ 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);
}
Expand Down Expand Up @@ -1090,10 +1114,11 @@ function completeWork(
return null;
}
} else if (
// The time it took to render last row is greater than time until
// the expiration.
// 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.
now() * 2 - renderState.renderingStartTime >
renderState.tailExpiration &&
getRenderTargetTime() &&
renderLanes !== OffscreenLane
) {
// We have now passed our CPU deadline and we'll just give up further
Expand All @@ -1105,13 +1130,16 @@ 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. 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;
// 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(renderLanes);
markSpawnedWork(SomeRetryLane);
}
}
}
Expand All @@ -1136,18 +1164,6 @@ 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;
Expand Down
2 changes: 2 additions & 0 deletions packages/react-reconciler/src/ReactFiberLane.js
View file
Open in desktop
Expand Up @@ -97,6 +97,8 @@ const TransitionLongLanes: Lanes = /* */ 0b0000000001111000000

const RetryLanes: Lanes = /* */ 0b0000011110000000000000000000000;

export const SomeRetryLane: Lanes = /* */ 0b0000010000000000000000000000000;

export const SelectiveHydrationLane: Lane = /* */ 0b0000100000000000000000000000000;

const NonIdleLanes = /* */ 0b0000111111111111111111111111111;
Expand Down
4 changes: 1 addition & 3 deletions packages/react-reconciler/src/ReactFiberSuspenseComponent.new.js
View file
Open in desktop
Expand Up @@ -41,14 +41,12 @@ export type SuspenseListRenderState = {|
isBackwards: boolean,
// The currently rendering tail row.
rendering: null | Fiber,
// The absolute time when we started rendering the tail row.
// The absolute time when we started rendering the most recent 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.
Expand Down
4 changes: 1 addition & 3 deletions packages/react-reconciler/src/ReactFiberSuspenseComponent.old.js
View file
Open in desktop
Expand Up @@ -41,14 +41,12 @@ export type SuspenseListRenderState = {|
isBackwards: boolean,
// The currently rendering tail row.
rendering: null | Fiber,
// The absolute time when we started rendering the tail row.
// The absolute time when we started rendering the most recent 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.
Expand Down
25 changes: 25 additions & 0 deletions packages/react-reconciler/src/ReactFiberWorkLoop.new.js
View file
Open in desktop
Expand Up @@ -328,6 +328,21 @@ let globalMostRecentFallbackTime: number = 0;
const FALLBACK_THROTTLE_MS: number = 500;
const DEFAULT_TIMEOUT_MS: number = 5000;

// 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;
Expand Down Expand Up @@ -603,6 +618,7 @@ 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();
}
}
Expand Down Expand Up @@ -1111,6 +1127,7 @@ export function flushRoot(root: FiberRoot, lanes: Lanes) {
markRootExpired(root, lanes);
ensureRootIsScheduled(root, now());
if ((executionContext & (RenderContext | CommitContext)) === NoContext) {
resetRenderTimer();
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we could put this call inside flushSyncCallbackQueue? Since that's the entry point for synchronous work.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not all of them check for NoContext, so I was hesitant to do that without digging into why that is first.

flushSyncCallbackQueue();
}
}
Expand Down Expand Up @@ -1185,6 +1202,7 @@ 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();
}
}
Expand All @@ -1199,6 +1217,7 @@ 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();
}
}
Expand Down Expand Up @@ -1227,6 +1246,7 @@ 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();
}
}
Expand All @@ -1240,6 +1260,7 @@ 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();
}
}
Expand All @@ -1256,6 +1277,7 @@ 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();
}
}
Expand Down Expand Up @@ -1323,6 +1345,7 @@ export function flushControlled(fn: () => mixed): void {
executionContext = prevExecutionContext;
if (executionContext === NoContext) {
// Flush the immediate callbacks that were scheduled during this batch
resetRenderTimer();
flushSyncCallbackQueue();
}
}
Expand All @@ -1333,6 +1356,7 @@ export function flushControlled(fn: () => mixed): void {
executionContext = prevExecutionContext;
if (executionContext === NoContext) {
// Flush the immediate callbacks that were scheduled during this batch
resetRenderTimer();
flushSyncCallbackQueue();
}
}
Expand Down Expand Up @@ -1651,6 +1675,7 @@ 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);
}
Expand Down