From d1dd7feabc63bf8ca61e9b3f4d78245a29ebbe9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Sat, 14 Dec 2024 13:46:21 -0500 Subject: [PATCH 1/3] [Fiber] Schedule client renders using non-hydration lane (#31776) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Related to #31752. When hydrating, we have two different ways of handling a Suspense boundary that the server has already given up on and decided to client render. If we have already hydrated the parent and then later this happens, then we'll use the retry lane like any ping. If we discover that it was already in client-render mode when we discover the Suspense boundary for the first time, then schedule a default lane to let us first finish the current render and then upgrade the priority to sync to try to client render this boundary as soon as possible since we're holding back content. We used to use the `DefaultHydrationLane` for this but this is not really a Hydration. It's actually a client render. If we get any other updates flowing in from above at the same time we might as well do them in the same pass instead of two passes. So this should be considered more like any update. This also means that visually the client render pass now gets painted as a render instead of a hydration. This show the flow of a shell being hydrated at the default priority, then a Suspense boundary being discovered and hydrated at Idle and then an inner boundary being discovered as client rendered which gets upgraded to default. Screenshot 2024-12-14 at 12 13 57 AM --- packages/react-reconciler/src/ReactFiberBeginWork.js | 12 +++++------- packages/shared/ReactFeatureFlags.js | 4 ++-- 2 files changed, 7 insertions(+), 9 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 119129d506c09..0ae0e668d22ac 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -111,6 +111,7 @@ import { disableLegacyMode, disableDefaultPropsExceptForClasses, enableOwnerStacks, + enableHydrationLaneScheduling, } from 'shared/ReactFeatureFlags'; import isArray from 'shared/isArray'; import shallowEqual from 'shared/shallowEqual'; @@ -146,6 +147,7 @@ import { NoLane, NoLanes, OffscreenLane, + DefaultLane, DefaultHydrationLane, SomeRetryLane, includesSomeLane, @@ -2646,14 +2648,10 @@ function mountDehydratedSuspenseComponent( // have timed out. In theory we could render it in this pass but it would have the // wrong priority associated with it and will prevent hydration of parent path. // Instead, we'll leave work left on it to render it in a separate commit. - - // TODO This time should be the time at which the server rendered response that is - // a parent to this boundary was displayed. However, since we currently don't have - // a protocol to transfer that time, we'll just estimate it by using the current - // time. This will mean that Suspense timeouts are slightly shifted to later than - // they should be. // Schedule a normal pri update to render this content. - workInProgress.lanes = laneToLanes(DefaultHydrationLane); + workInProgress.lanes = laneToLanes( + enableHydrationLaneScheduling ? DefaultLane : DefaultHydrationLane, + ); } else { // We'll continue hydrating the rest at offscreen priority since we'll already // be showing the right content coming from the server, it is no rush. diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index f28d123e61e5b..afac564cad28a 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -22,6 +22,8 @@ // when it rolls out to prod. We should remove these as soon as possible. // ----------------------------------------------------------------------------- +export const enableHydrationLaneScheduling = true; + // ----------------------------------------------------------------------------- // Land or remove (moderate effort) // @@ -111,8 +113,6 @@ export const enableSuspenseAvoidThisFallback = false; export const enableCPUSuspense = __EXPERIMENTAL__; -export const enableHydrationLaneScheduling = true; - // Test this at Meta before enabling. export const enableNoCloningMemoCache = false; From c32780eeb4c44e138d09a35da841926f512d3b07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Sat, 14 Dec 2024 13:49:47 -0500 Subject: [PATCH 2/3] [Fiber] Highlight hydration and offscreen render phases (#31752) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This highlights the render phase as the tertiary color (green) when we're render a hydration lane or offscreen lane. I call the "Render" phase "Hydrated" instead in this case. For the offscreen case we don't currently have a differentiation between hydrated or activity. I just called that "Prepared". Even for the hydration case where there's no discovered client rendered boundaries it's more like it's preparing for an interaction rather than blocking one. Where as for the other lanes the hydration might block something. Screenshot 2024-12-12 at 11 23 14 PM In a follow up I'd like to color the components in the Components tree green if they were hydrated but not the ones that was actually client rendered e.g. due to a mismatch or forced client rendering so you can tell the difference. Unfortunately, the current signals we have for this get reset earlier in the commit phase than when we log these. Another thing is that a failed hydration should probably be colored red even though it ends up committing successfully. I.e. a recoverable error. --- .../react-reconciler/src/ReactFiberLane.js | 12 ++++ .../src/ReactFiberPerformanceTrack.js | 70 ++++++++++++++++--- .../src/ReactFiberWorkLoop.js | 29 ++++++-- 3 files changed, 94 insertions(+), 17 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberLane.js b/packages/react-reconciler/src/ReactFiberLane.js index ee3a0a3df2b60..4c96207bd6696 100644 --- a/packages/react-reconciler/src/ReactFiberLane.js +++ b/packages/react-reconciler/src/ReactFiberLane.js @@ -621,6 +621,18 @@ export function includesTransitionLane(lanes: Lanes): boolean { return (lanes & TransitionLanes) !== NoLanes; } +export function includesOnlyHydrationLanes(lanes: Lanes): boolean { + return (lanes & HydrationLanes) === lanes; +} + +export function includesOnlyOffscreenLanes(lanes: Lanes): boolean { + return (lanes & OffscreenLane) === lanes; +} + +export function includesOnlyHydrationOrOffscreenLanes(lanes: Lanes): boolean { + return (lanes & (HydrationLanes | OffscreenLane)) === lanes; +} + export function includesBlockingLane(lanes: Lanes): boolean { const SyncDefaultLanes = InputContinuousHydrationLane | diff --git a/packages/react-reconciler/src/ReactFiberPerformanceTrack.js b/packages/react-reconciler/src/ReactFiberPerformanceTrack.js index 56470bb0e55d6..7e9e246d8beb2 100644 --- a/packages/react-reconciler/src/ReactFiberPerformanceTrack.js +++ b/packages/react-reconciler/src/ReactFiberPerformanceTrack.js @@ -9,9 +9,16 @@ import type {Fiber} from './ReactInternalTypes'; +import type {Lanes} from './ReactFiberLane'; + import getComponentNameFromFiber from './getComponentNameFromFiber'; -import {getGroupNameOfHighestPriorityLane} from './ReactFiberLane'; +import { + getGroupNameOfHighestPriorityLane, + includesOnlyHydrationLanes, + includesOnlyOffscreenLanes, + includesOnlyHydrationOrOffscreenLanes, +} from './ReactFiberLane'; import {enableProfilerTimer} from 'shared/ReactFeatureFlags'; @@ -51,7 +58,7 @@ const reusableLaneOptions = { }, }; -export function setCurrentTrackFromLanes(lanes: number): void { +export function setCurrentTrackFromLanes(lanes: Lanes): void { reusableLaneDevToolDetails.track = getGroupNameOfHighestPriorityLane(lanes); } @@ -223,6 +230,7 @@ export function logBlockingStart( eventType: null | string, eventIsRepeat: boolean, renderStartTime: number, + lanes: Lanes, ): void { if (supportsUserTiming) { reusableLaneDevToolDetails.track = 'Blocking'; @@ -240,7 +248,11 @@ export function logBlockingStart( } if (updateTime > 0) { // Log the time from when we called setState until we started rendering. - reusableLaneDevToolDetails.color = 'primary-light'; + reusableLaneDevToolDetails.color = includesOnlyHydrationOrOffscreenLanes( + lanes, + ) + ? 'tertiary-light' + : 'primary-light'; reusableLaneOptions.start = updateTime; reusableLaneOptions.end = renderStartTime; performance.measure('Blocked', reusableLaneOptions); @@ -292,33 +304,65 @@ export function logTransitionStart( } } -export function logRenderPhase(startTime: number, endTime: number): void { +export function logRenderPhase( + startTime: number, + endTime: number, + lanes: Lanes, +): void { if (supportsUserTiming) { - reusableLaneDevToolDetails.color = 'primary-dark'; + reusableLaneDevToolDetails.color = includesOnlyHydrationOrOffscreenLanes( + lanes, + ) + ? 'tertiary-dark' + : 'primary-dark'; reusableLaneOptions.start = startTime; reusableLaneOptions.end = endTime; - performance.measure('Render', reusableLaneOptions); + performance.measure( + includesOnlyOffscreenLanes(lanes) + ? 'Prepared' + : includesOnlyHydrationLanes(lanes) + ? 'Hydrated' + : 'Render', + reusableLaneOptions, + ); } } export function logInterruptedRenderPhase( startTime: number, endTime: number, + lanes: Lanes, ): void { if (supportsUserTiming) { - reusableLaneDevToolDetails.color = 'primary-dark'; + reusableLaneDevToolDetails.color = includesOnlyHydrationOrOffscreenLanes( + lanes, + ) + ? 'tertiary-dark' + : 'primary-dark'; reusableLaneOptions.start = startTime; reusableLaneOptions.end = endTime; - performance.measure('Interrupted Render', reusableLaneOptions); + performance.measure( + includesOnlyOffscreenLanes(lanes) + ? 'Prewarm' + : includesOnlyHydrationLanes(lanes) + ? 'Interrupted Hydration' + : 'Interrupted Render', + reusableLaneOptions, + ); } } export function logSuspendedRenderPhase( startTime: number, endTime: number, + lanes: Lanes, ): void { if (supportsUserTiming) { - reusableLaneDevToolDetails.color = 'primary-dark'; + reusableLaneDevToolDetails.color = includesOnlyHydrationOrOffscreenLanes( + lanes, + ) + ? 'tertiary-dark' + : 'primary-dark'; reusableLaneOptions.start = startTime; reusableLaneOptions.end = endTime; performance.measure('Prewarm', reusableLaneOptions); @@ -328,10 +372,15 @@ export function logSuspendedRenderPhase( export function logSuspendedWithDelayPhase( startTime: number, endTime: number, + lanes: Lanes, ): void { // This means the render was suspended and cannot commit until it gets unblocked. if (supportsUserTiming) { - reusableLaneDevToolDetails.color = 'primary-dark'; + reusableLaneDevToolDetails.color = includesOnlyHydrationOrOffscreenLanes( + lanes, + ) + ? 'tertiary-dark' + : 'primary-dark'; reusableLaneOptions.start = startTime; reusableLaneOptions.end = endTime; performance.measure('Suspended', reusableLaneOptions); @@ -341,6 +390,7 @@ export function logSuspendedWithDelayPhase( export function logErroredRenderPhase( startTime: number, endTime: number, + lanes: Lanes, ): void { if (supportsUserTiming) { reusableLaneDevToolDetails.color = 'error'; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index f812d16c9c65a..dd637d687da4f 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -1035,7 +1035,7 @@ export function performWorkOnRoot( if (errorRetryLanes !== NoLanes) { if (enableProfilerTimer && enableComponentPerformanceTrack) { setCurrentTrackFromLanes(lanes); - logErroredRenderPhase(renderStartTime, renderEndTime); + logErroredRenderPhase(renderStartTime, renderEndTime, lanes); finalizeRender(lanes, renderEndTime); } lanes = errorRetryLanes; @@ -1066,7 +1066,7 @@ export function performWorkOnRoot( if (exitStatus === RootFatalErrored) { if (enableProfilerTimer && enableComponentPerformanceTrack) { setCurrentTrackFromLanes(lanes); - logErroredRenderPhase(renderStartTime, renderEndTime); + logErroredRenderPhase(renderStartTime, renderEndTime, lanes); finalizeRender(lanes, renderEndTime); } prepareFreshStack(root, NoLanes); @@ -1207,7 +1207,7 @@ function finishConcurrentRender( // until we receive more data. if (enableProfilerTimer && enableComponentPerformanceTrack) { setCurrentTrackFromLanes(lanes); - logSuspendedRenderPhase(renderStartTime, renderEndTime); + logSuspendedRenderPhase(renderStartTime, renderEndTime, lanes); finalizeRender(lanes, renderEndTime); trackSuspendedTime(lanes, renderEndTime); } @@ -1757,9 +1757,17 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { // then this is considered a prewarm and not an interrupted render because // we couldn't have shown anything anyway so it's not a bad thing that we // got interrupted. - logSuspendedRenderPhase(previousRenderStartTime, renderStartTime); + logSuspendedRenderPhase( + previousRenderStartTime, + renderStartTime, + lanes, + ); } else { - logInterruptedRenderPhase(previousRenderStartTime, renderStartTime); + logInterruptedRenderPhase( + previousRenderStartTime, + renderStartTime, + lanes, + ); } finalizeRender(workInProgressRootRenderLanes, renderStartTime); } @@ -1783,6 +1791,7 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { : clampedUpdateTime >= 0 ? clampedUpdateTime : renderStartTime, + lanes, ); } logBlockingStart( @@ -1791,6 +1800,7 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { blockingEventType, blockingEventIsRepeat, renderStartTime, + lanes, ); clearBlockingTimers(); } @@ -1817,6 +1827,7 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { : clampedUpdateTime >= 0 ? clampedUpdateTime : renderStartTime, + lanes, ); } logTransitionStart( @@ -3202,9 +3213,13 @@ function commitRootImpl( // Log the previous render phase once we commit. I.e. we weren't interrupted. setCurrentTrackFromLanes(lanes); if (exitStatus === RootErrored) { - logErroredRenderPhase(completedRenderStartTime, completedRenderEndTime); + logErroredRenderPhase( + completedRenderStartTime, + completedRenderEndTime, + lanes, + ); } else { - logRenderPhase(completedRenderStartTime, completedRenderEndTime); + logRenderPhase(completedRenderStartTime, completedRenderEndTime, lanes); } } From c80b336d23aa472b5e5910268138ac0447d6ae19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sebastian=20Markb=C3=A5ge?= Date: Sat, 14 Dec 2024 16:17:06 -0500 Subject: [PATCH 3/3] Implement requestPaint in the actual scheduler (#31784) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When implementing passive effects we did a pretty massive oversight. While the passive effect is scheduled into its own scheduler task, the scheduler doesn't always yield to the browser if it has time left. That means that if you have a fast commit phase, it might try to squeeze in the passive effects in the same frame but those then might end being very heavy. We had `requestPaint()` for this but that was only implemented for the `isInputPending` experiment. It wasn't thought we needed it for the regular scheduler because it yields "every frame" anyway - but it doesn't yield every task. While the `isInputPending` experiment showed that it wasn't actually any significant impact, and it was better to keep shorter yield time anyway. Which is why we deleted the code. Whatever small win it did see in some cases might have been actually due to this issue rather than anything to do with `isInputPending` at all. As you can see in https://github.com/facebook/react/pull/31782 we do have this implemented in the mock scheduler and a lot of behavior that we assert assumes that this works. So this just implements yielding after `requestPaint` is called. Before: Screenshot 2024-12-14 at 3 40 24 PM After: Screenshot 2024-12-14 at 3 41 25 PM Notice how in after the native task is split into two. It might not always actually paint and the native scheduler might make the same mistake and think it has enough time left but it's at least less likely to. We do have another way to do this. When we yield a continuation we also yield to the native browser. This is to enable the Suspense Optimization (currently disabled) to work. We could do the same for passive effects and, in fact, I have a branch that does but because that requires a lot more tests to be fixed it's a lot more invasive of a change. The nice thing about this approach is that this is not even running in tests at all and the tests we do have assert that this is the behavior already. 😬 --- packages/scheduler/src/__tests__/Scheduler-test.js | 4 ++-- packages/scheduler/src/forks/Scheduler.js | 11 ++++++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/scheduler/src/__tests__/Scheduler-test.js b/packages/scheduler/src/__tests__/Scheduler-test.js index 5af59a24a24f5..9e0813e461e61 100644 --- a/packages/scheduler/src/__tests__/Scheduler-test.js +++ b/packages/scheduler/src/__tests__/Scheduler-test.js @@ -183,7 +183,7 @@ describe('SchedulerBrowser', () => { it('task with continuation', () => { scheduleCallback(NormalPriority, () => { runtime.log('Task'); - // Request paint so that we yield at the end of the frame interval + // Request paint so that we yield immediately requestPaint(); while (!Scheduler.unstable_shouldYield()) { runtime.advanceTime(1); @@ -199,7 +199,7 @@ describe('SchedulerBrowser', () => { runtime.assertLog([ 'Message Event', 'Task', - gate(flags => (flags.www ? 'Yield at 10ms' : 'Yield at 5ms')), + 'Yield at 0ms', 'Post Message', ]); diff --git a/packages/scheduler/src/forks/Scheduler.js b/packages/scheduler/src/forks/Scheduler.js index a785bec95320c..0e85e0a99b7b0 100644 --- a/packages/scheduler/src/forks/Scheduler.js +++ b/packages/scheduler/src/forks/Scheduler.js @@ -93,6 +93,8 @@ var isPerformingWork = false; var isHostCallbackScheduled = false; var isHostTimeoutScheduled = false; +var needsPaint = false; + // Capture local references to native APIs, in case a polyfill overrides them. const localSetTimeout = typeof setTimeout === 'function' ? setTimeout : null; const localClearTimeout = @@ -456,6 +458,10 @@ let frameInterval = frameYieldMs; let startTime = -1; function shouldYieldToHost(): boolean { + if (needsPaint) { + // Yield now. + return true; + } const timeElapsed = getCurrentTime() - startTime; if (timeElapsed < frameInterval) { // The main thread has only been blocked for a really short amount of time; @@ -466,7 +472,9 @@ function shouldYieldToHost(): boolean { return true; } -function requestPaint() {} +function requestPaint() { + needsPaint = true; +} function forceFrameRate(fps: number) { if (fps < 0 || fps > 125) { @@ -486,6 +494,7 @@ function forceFrameRate(fps: number) { } const performWorkUntilDeadline = () => { + needsPaint = false; if (isMessageLoopRunning) { const currentTime = getCurrentTime(); // Keep track of the start time so we can measure how long the main thread