diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index f00e5286751b8..ef37cd5ead6f8 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -2628,6 +2628,7 @@ function initSuspenseListRenderState( renderingStartTime: 0, last: lastContentRow, tail: tail, + tailExpiration: 0, tailMode: tailMode, }: SuspenseListRenderState); } else { @@ -2637,6 +2638,7 @@ function initSuspenseListRenderState( renderState.renderingStartTime = 0; renderState.last = lastContentRow; renderState.tail = tail; + renderState.tailExpiration = 0; renderState.tailMode = tailMode; } } diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index 1888c03c910ed..163aedb4d66bf 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -2635,6 +2635,7 @@ function initSuspenseListRenderState( renderingStartTime: 0, last: lastContentRow, tail: tail, + tailExpiration: 0, tailMode: tailMode, lastEffect: lastEffectBeforeRendering, }: SuspenseListRenderState); @@ -2645,6 +2646,7 @@ function initSuspenseListRenderState( renderState.renderingStartTime = 0; renderState.last = lastContentRow; renderState.tail = tail; + renderState.tailExpiration = 0; renderState.tailMode = tailMode; renderState.lastEffect = lastEffectBeforeRendering; } diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js index da1a97cab9312..438126907b691 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js @@ -137,10 +137,9 @@ import { renderDidSuspendDelayIfPossible, renderHasNotSuspendedYet, popRenderLanes, - getRenderTargetTime, } from './ReactFiberWorkLoop.new'; import {createFundamentalStateInstance} from './ReactFiberFundamental.new'; -import {OffscreenLane, SomeRetryLane} from './ReactFiberLane'; +import {OffscreenLane} from './ReactFiberLane'; import {resetChildFibers} from './ReactChildFiber.new'; import {createScopeInstance} from './ReactFiberScope.new'; import {transferActualDuration} from './ReactProfilerTimer.new'; @@ -1067,29 +1066,6 @@ 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); } @@ -1122,11 +1098,10 @@ function completeWork( return null; } } else if ( - // 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. + // The time it took to render last row is greater than time until + // the expiration. now() * 2 - renderState.renderingStartTime > - getRenderTargetTime() && + renderState.tailExpiration && renderLanes !== OffscreenLane ) { // We have now passed our CPU deadline and we'll just give up further @@ -1142,9 +1117,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 = SomeRetryLane; + workInProgress.lanes = renderLanes; if (enableSchedulerTracing) { - markSpawnedWork(SomeRetryLane); + markSpawnedWork(renderLanes); } } } @@ -1169,6 +1144,18 @@ 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; diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.old.js b/packages/react-reconciler/src/ReactFiberCompleteWork.old.js index b85395337eed3..82bc29f7495e5 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.old.js @@ -135,10 +135,9 @@ import { renderDidSuspendDelayIfPossible, renderHasNotSuspendedYet, popRenderLanes, - getRenderTargetTime, } from './ReactFiberWorkLoop.old'; import {createFundamentalStateInstance} from './ReactFiberFundamental.old'; -import {OffscreenLane, SomeRetryLane} from './ReactFiberLane'; +import {OffscreenLane} from './ReactFiberLane'; import {resetChildFibers} from './ReactChildFiber.old'; import {createScopeInstance} from './ReactFiberScope.old'; import {transferActualDuration} from './ReactProfilerTimer.old'; @@ -1050,29 +1049,6 @@ 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); } @@ -1114,11 +1090,10 @@ function completeWork( return null; } } else if ( - // 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. + // The time it took to render last row is greater than time until + // the expiration. now() * 2 - renderState.renderingStartTime > - getRenderTargetTime() && + renderState.tailExpiration && renderLanes !== OffscreenLane ) { // We have now passed our CPU deadline and we'll just give up further @@ -1130,16 +1105,13 @@ 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. 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; + // 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; if (enableSchedulerTracing) { - markSpawnedWork(SomeRetryLane); + markSpawnedWork(renderLanes); } } } @@ -1164,6 +1136,18 @@ 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; diff --git a/packages/react-reconciler/src/ReactFiberLane.js b/packages/react-reconciler/src/ReactFiberLane.js index 8e6a66f9210a8..c18064209cf1b 100644 --- a/packages/react-reconciler/src/ReactFiberLane.js +++ b/packages/react-reconciler/src/ReactFiberLane.js @@ -91,8 +91,6 @@ const TransitionLanes: Lanes = /* */ 0b0000000001111111110 const RetryLanes: Lanes = /* */ 0b0000011110000000000000000000000; -export const SomeRetryLane: Lanes = /* */ 0b0000010000000000000000000000000; - export const SelectiveHydrationLane: Lane = /* */ 0b0000100000000000000000000000000; const NonIdleLanes = /* */ 0b0000111111111111111111111111111; diff --git a/packages/react-reconciler/src/ReactFiberSuspenseComponent.new.js b/packages/react-reconciler/src/ReactFiberSuspenseComponent.new.js index ce49b80419fca..b71569ee6a488 100644 --- a/packages/react-reconciler/src/ReactFiberSuspenseComponent.new.js +++ b/packages/react-reconciler/src/ReactFiberSuspenseComponent.new.js @@ -41,12 +41,14 @@ export type SuspenseListRenderState = {| isBackwards: boolean, // The currently rendering tail row. rendering: null | Fiber, - // The absolute time when we started rendering the most recent tail row. + // The absolute time when we started rendering the 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, |}; diff --git a/packages/react-reconciler/src/ReactFiberSuspenseComponent.old.js b/packages/react-reconciler/src/ReactFiberSuspenseComponent.old.js index 49e586c232327..e151ab0be0088 100644 --- a/packages/react-reconciler/src/ReactFiberSuspenseComponent.old.js +++ b/packages/react-reconciler/src/ReactFiberSuspenseComponent.old.js @@ -41,12 +41,14 @@ export type SuspenseListRenderState = {| isBackwards: boolean, // The currently rendering tail row. rendering: null | Fiber, - // The absolute time when we started rendering the most recent tail row. + // The absolute time when we started rendering the 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. diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index acf2da879d227..3970e125f9ced 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -323,21 +323,6 @@ let mostRecentlyUpdatedRoot: FiberRoot | null = null; let globalMostRecentFallbackTime: number = 0; const FALLBACK_THROTTLE_MS: number = 500; -// 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 | null = null; @@ -597,7 +582,6 @@ 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(); } } @@ -1061,7 +1045,6 @@ export function flushRoot(root: FiberRoot, lanes: Lanes) { markRootExpired(root, lanes); ensureRootIsScheduled(root, now()); if ((executionContext & (RenderContext | CommitContext)) === NoContext) { - resetRenderTimer(); flushSyncCallbackQueue(); } } @@ -1136,7 +1119,6 @@ export function batchedUpdates(fn: A => R, a: A): R { executionContext = prevExecutionContext; if (executionContext === NoContext) { // Flush the immediate callbacks that were scheduled during this batch - resetRenderTimer(); flushSyncCallbackQueue(); } } @@ -1151,7 +1133,6 @@ export function batchedEventUpdates(fn: A => R, a: A): R { executionContext = prevExecutionContext; if (executionContext === NoContext) { // Flush the immediate callbacks that were scheduled during this batch - resetRenderTimer(); flushSyncCallbackQueue(); } } @@ -1180,7 +1161,6 @@ export function discreteUpdates( executionContext = prevExecutionContext; if (executionContext === NoContext) { // Flush the immediate callbacks that were scheduled during this batch - resetRenderTimer(); flushSyncCallbackQueue(); } } @@ -1194,7 +1174,6 @@ export function discreteUpdates( executionContext = prevExecutionContext; if (executionContext === NoContext) { // Flush the immediate callbacks that were scheduled during this batch - resetRenderTimer(); flushSyncCallbackQueue(); } } @@ -1211,7 +1190,6 @@ export function unbatchedUpdates(fn: (a: A) => R, a: A): R { executionContext = prevExecutionContext; if (executionContext === NoContext) { // Flush the immediate callbacks that were scheduled during this batch - resetRenderTimer(); flushSyncCallbackQueue(); } } @@ -1279,7 +1257,6 @@ export function flushControlled(fn: () => mixed): void { executionContext = prevExecutionContext; if (executionContext === NoContext) { // Flush the immediate callbacks that were scheduled during this batch - resetRenderTimer(); flushSyncCallbackQueue(); } } @@ -1290,7 +1267,6 @@ export function flushControlled(fn: () => mixed): void { executionContext = prevExecutionContext; if (executionContext === NoContext) { // Flush the immediate callbacks that were scheduled during this batch - resetRenderTimer(); flushSyncCallbackQueue(); } } @@ -1583,7 +1559,6 @@ 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); } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 4e62a84e9a2ce..4c25ad4117bad 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -308,21 +308,6 @@ let mostRecentlyUpdatedRoot: FiberRoot | null = null; let globalMostRecentFallbackTime: number = 0; const FALLBACK_THROTTLE_MS: number = 500; -// 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 nextEffect: Fiber | null = null; let hasUncaughtError = false; let firstUncaughtError = null; @@ -585,7 +570,6 @@ 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(); } } @@ -1049,7 +1033,6 @@ export function flushRoot(root: FiberRoot, lanes: Lanes) { markRootExpired(root, lanes); ensureRootIsScheduled(root, now()); if ((executionContext & (RenderContext | CommitContext)) === NoContext) { - resetRenderTimer(); flushSyncCallbackQueue(); } } @@ -1124,7 +1107,6 @@ export function batchedUpdates(fn: A => R, a: A): R { executionContext = prevExecutionContext; if (executionContext === NoContext) { // Flush the immediate callbacks that were scheduled during this batch - resetRenderTimer(); flushSyncCallbackQueue(); } } @@ -1139,7 +1121,6 @@ export function batchedEventUpdates(fn: A => R, a: A): R { executionContext = prevExecutionContext; if (executionContext === NoContext) { // Flush the immediate callbacks that were scheduled during this batch - resetRenderTimer(); flushSyncCallbackQueue(); } } @@ -1168,7 +1149,6 @@ export function discreteUpdates( executionContext = prevExecutionContext; if (executionContext === NoContext) { // Flush the immediate callbacks that were scheduled during this batch - resetRenderTimer(); flushSyncCallbackQueue(); } } @@ -1182,7 +1162,6 @@ export function discreteUpdates( executionContext = prevExecutionContext; if (executionContext === NoContext) { // Flush the immediate callbacks that were scheduled during this batch - resetRenderTimer(); flushSyncCallbackQueue(); } } @@ -1199,7 +1178,6 @@ export function unbatchedUpdates(fn: (a: A) => R, a: A): R { executionContext = prevExecutionContext; if (executionContext === NoContext) { // Flush the immediate callbacks that were scheduled during this batch - resetRenderTimer(); flushSyncCallbackQueue(); } } @@ -1267,7 +1245,6 @@ export function flushControlled(fn: () => mixed): void { executionContext = prevExecutionContext; if (executionContext === NoContext) { // Flush the immediate callbacks that were scheduled during this batch - resetRenderTimer(); flushSyncCallbackQueue(); } } @@ -1278,7 +1255,6 @@ export function flushControlled(fn: () => mixed): void { executionContext = prevExecutionContext; if (executionContext === NoContext) { // Flush the immediate callbacks that were scheduled during this batch - resetRenderTimer(); flushSyncCallbackQueue(); } } @@ -1571,7 +1547,6 @@ 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); } diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js index c45a10c99d6d8..5039e8a820e8d 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js @@ -2556,147 +2556,6 @@ describe('ReactSuspenseList', () => { ); }); - // @gate experimental - it('should be able to progressively show CPU expensive rows with two pass rendering', async () => { - function TwoPass({text}) { - const [pass, setPass] = React.useState(0); - React.useLayoutEffect(() => { - Scheduler.unstable_yieldValue('Mount ' + text); - setPass(1); - }, []); - return ; - } - - function Sleep({time, children}) { - Scheduler.unstable_advanceTime(time); - return children; - } - - function App() { - Scheduler.unstable_yieldValue('App'); - return ( - - }> - - - - - }> - - - - - - - - - ); - } - - ReactNoop.render(); - - expect(Scheduler).toFlushAndYieldThrough([ - 'App', - 'First Pass A', - 'Mount A', - 'A', - ]); - expect(ReactNoop).toMatchRenderedOutput(A); - - expect(Scheduler).toFlushAndYieldThrough(['First Pass B', 'Mount B', 'B']); - expect(ReactNoop).toMatchRenderedOutput( - <> - A - B - , - ); - - expect(Scheduler).toFlushAndYield(['C']); - expect(ReactNoop).toMatchRenderedOutput( - <> - A - B - C - , - ); - }); - - // @gate experimental - it('should be able to progressively show rows with two pass rendering and visible', async () => { - function TwoPass({text}) { - const [pass, setPass] = React.useState(0); - React.useLayoutEffect(() => { - Scheduler.unstable_yieldValue('Mount ' + text); - setPass(1); - }, []); - return ; - } - - function Sleep({time, children}) { - Scheduler.unstable_advanceTime(time); - return children; - } - - function App() { - Scheduler.unstable_yieldValue('App'); - return ( - - }> - - - - - }> - - - - - }> - - - - - - ); - } - - ReactNoop.render(); - - expect(Scheduler).toFlushAndYieldThrough([ - 'App', - 'First Pass A', - 'Loading B', - 'Loading C', - 'Mount A', - 'A', - ]); - expect(ReactNoop).toMatchRenderedOutput( - <> - A - Loading B - Loading C - , - ); - - expect(Scheduler).toFlushAndYieldThrough(['First Pass B', 'Mount B', 'B']); - expect(ReactNoop).toMatchRenderedOutput( - <> - A - B - Loading C - , - ); - - expect(Scheduler).toFlushAndYield(['C']); - expect(ReactNoop).toMatchRenderedOutput( - <> - A - B - C - , - ); - }); - // @gate experimental && enableProfilerTimer it('counts the actual duration when profiling a SuspenseList', async () => { // Order of parameters: id, phase, actualDuration, treeBaseDuration diff --git a/packages/react/src/__tests__/ReactProfiler-test.internal.js b/packages/react/src/__tests__/ReactProfiler-test.internal.js index 193f3d1ac7f15..c3d1dbb3cea71 100644 --- a/packages/react/src/__tests__/ReactProfiler-test.internal.js +++ b/packages/react/src/__tests__/ReactProfiler-test.internal.js @@ -294,7 +294,6 @@ describe('Profiler', () => { 'read current time', 'read current time', 'read current time', - 'read current time', ]); // Restore original mock