From 69612413eff3327616fbffd3a08d062353efba4a Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Tue, 12 Nov 2024 23:09:39 -0500 Subject: [PATCH 1/4] Save the last committed/interrupted time That way we can clamp a new update within the same event which otherwise would have en event time before the old render. --- .../src/ReactProfilerTimer.js | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/packages/react-reconciler/src/ReactProfilerTimer.js b/packages/react-reconciler/src/ReactProfilerTimer.js index e59b5cb8e56ed..55bb89c26acb0 100644 --- a/packages/react-reconciler/src/ReactProfilerTimer.js +++ b/packages/react-reconciler/src/ReactProfilerTimer.js @@ -36,10 +36,12 @@ export let componentEffectDuration: number = -0; export let componentEffectStartTime: number = -1.1; export let componentEffectEndTime: number = -1.1; +let blockingClampTime: number = -0; export let blockingUpdateTime: number = -1.1; // First sync setState scheduled. export let blockingEventTime: number = -1.1; // Event timeStamp of the first setState. export let blockingEventType: null | string = null; // Event type of the first setState. // TODO: This should really be one per Transition lane. +let transitionClampTime: number = -0; export let transitionStartTime: number = -1.1; // First startTransition call before setState. export let transitionUpdateTime: number = -1.1; // First transition setState scheduled. export let transitionEventTime: number = -1.1; // Event timeStamp of the first transition. @@ -54,6 +56,11 @@ export function startUpdateTimerByLane(lane: Lane): void { blockingUpdateTime = now(); blockingEventTime = resolveEventTimeStamp(); blockingEventType = resolveEventType(); + if (blockingEventTime < blockingClampTime) { + // We've already rendered within this event and we need to clamp the new update + // to the end of that render. + blockingEventTime = blockingClampTime; + } } } else if (isTransitionLane(lane)) { if (transitionUpdateTime < 0) { @@ -61,6 +68,11 @@ export function startUpdateTimerByLane(lane: Lane): void { if (transitionStartTime < 0) { transitionEventTime = resolveEventTimeStamp(); transitionEventType = resolveEventType(); + if (transitionEventTime < transitionClampTime) { + // We've already rendered within this event and we need to clamp the new update + // to the end of that render. + transitionEventTime = transitionClampTime; + } } } } @@ -78,6 +90,11 @@ export function startAsyncTransitionTimer(): void { transitionStartTime = now(); transitionEventTime = resolveEventTimeStamp(); transitionEventType = resolveEventType(); + if (transitionEventTime < transitionClampTime) { + // We've already rendered within this event and we need to clamp the new update + // to the end of that render. + transitionEventTime = transitionClampTime; + } } } @@ -121,6 +138,8 @@ export function clampBlockingTimers(finalTime: number): void { if (blockingEventTime >= 0 && blockingEventTime < finalTime) { blockingEventTime = finalTime; } + // Save this for later in case another update comes in later with an early event time. + blockingClampTime = finalTime; } export function clampTransitionTimers(finalTime: number): void { @@ -139,6 +158,8 @@ export function clampTransitionTimers(finalTime: number): void { if (transitionEventTime >= 0 && transitionEventTime < finalTime) { transitionEventTime = finalTime; } + // Save this for later in case another update comes in later with an early event time. + transitionClampTime = finalTime; } export function pushNestedEffectDurations(): number { From 4b3b123824cdfe8e7a339ba90d2107ab2bd7bb4a Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 13 Nov 2024 00:29:56 -0500 Subject: [PATCH 2/4] Finalize render when root did not complete or suspended with delay This is a special case which ends with no on-going renders but also doesn't commit. So we need to finalize the render so that we know that we can't. This should probably be handled more like a suspended yield instead. --- packages/react-reconciler/src/ReactFiberWorkLoop.js | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 6d2ec2d0c8e61..f20bfd44a2f9e 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -938,6 +938,9 @@ export function performWorkOnRoot( } break; } else if (exitStatus === RootDidNotComplete) { + if (enableProfilerTimer && enableComponentPerformanceTrack) { + finalizeRender(lanes, now()); + } // The render unwound without completing the tree. This happens in special // cases where need to exit the current render without producing a // consistent tree or committing. @@ -1130,6 +1133,9 @@ function finishConcurrentRender( // This is a transition, so we should exit without committing a // placeholder and without scheduling a timeout. Delay indefinitely // until we receive more data. + if (enableProfilerTimer && enableComponentPerformanceTrack) { + finalizeRender(lanes, now()); + } const didAttemptEntireTree = !workInProgressRootDidSkipSuspendedSiblings; markRootSuspended( From 613a8de798b912ba55748af63e989a53c09e4463 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 13 Nov 2024 16:52:01 -0500 Subject: [PATCH 3/4] Reset the event name to "" if we're doing more than one render within the event We do this by applying the clamp lazily and not resetting the event info between renders. --- .../src/ReactFiberPerformanceTrack.js | 6 +- .../src/ReactFiberWorkLoop.js | 26 ++++++-- .../src/ReactProfilerTimer.js | 65 +++++++------------ 3 files changed, 50 insertions(+), 47 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberPerformanceTrack.js b/packages/react-reconciler/src/ReactFiberPerformanceTrack.js index e1d75149b2425..953e6ed30ac63 100644 --- a/packages/react-reconciler/src/ReactFiberPerformanceTrack.js +++ b/packages/react-reconciler/src/ReactFiberPerformanceTrack.js @@ -118,6 +118,7 @@ export function logBlockingStart( updateTime: number, eventTime: number, eventType: null | string, + eventIsRepeat: boolean, renderStartTime: number, ): void { if (supportsUserTiming) { @@ -127,7 +128,7 @@ export function logBlockingStart( reusableLaneDevToolDetails.color = 'secondary-dark'; reusableLaneOptions.start = eventTime; reusableLaneOptions.end = updateTime > 0 ? updateTime : renderStartTime; - performance.measure(eventType, reusableLaneOptions); + performance.measure(eventIsRepeat ? '' : eventType, reusableLaneOptions); } if (updateTime > 0) { // Log the time from when we called setState until we started rendering. @@ -144,6 +145,7 @@ export function logTransitionStart( updateTime: number, eventTime: number, eventType: null | string, + eventIsRepeat: boolean, renderStartTime: number, ): void { if (supportsUserTiming) { @@ -158,7 +160,7 @@ export function logTransitionStart( : updateTime > 0 ? updateTime : renderStartTime; - performance.measure(eventType, reusableLaneOptions); + performance.measure(eventIsRepeat ? '' : eventType, reusableLaneOptions); } if (startTime > 0) { // Log the time from when we started an async transition until we called setState or started rendering. diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index f20bfd44a2f9e..b5e594a15b3cc 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -229,13 +229,17 @@ import { } from './ReactFiberConcurrentUpdates'; import { + blockingClampTime, blockingUpdateTime, blockingEventTime, blockingEventType, + blockingEventIsRepeat, + transitionClampTime, transitionStartTime, transitionUpdateTime, transitionEventTime, transitionEventType, + transitionEventIsRepeat, clearBlockingTimers, clearTransitionTimers, clampBlockingTimers, @@ -1661,19 +1665,31 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber { if (includesSyncLane(lanes) || includesBlockingLane(lanes)) { logBlockingStart( - blockingUpdateTime, - blockingEventTime, + blockingUpdateTime >= 0 && blockingUpdateTime < blockingClampTime + ? blockingClampTime + : blockingUpdateTime, + blockingEventTime >= 0 && blockingEventTime < blockingClampTime + ? blockingClampTime + : blockingEventTime, blockingEventType, + blockingEventIsRepeat, renderStartTime, ); clearBlockingTimers(); } if (includesTransitionLane(lanes)) { logTransitionStart( - transitionStartTime, - transitionUpdateTime, - transitionEventTime, + transitionStartTime >= 0 && transitionStartTime < transitionClampTime + ? transitionClampTime + : transitionStartTime, + transitionUpdateTime >= 0 && transitionUpdateTime < transitionClampTime + ? transitionClampTime + : transitionUpdateTime, + transitionEventTime >= 0 && transitionEventTime < transitionClampTime + ? transitionClampTime + : transitionEventTime, transitionEventType, + transitionEventIsRepeat, renderStartTime, ); clearTransitionTimers(); diff --git a/packages/react-reconciler/src/ReactProfilerTimer.js b/packages/react-reconciler/src/ReactProfilerTimer.js index 55bb89c26acb0..93869c85148a8 100644 --- a/packages/react-reconciler/src/ReactProfilerTimer.js +++ b/packages/react-reconciler/src/ReactProfilerTimer.js @@ -36,16 +36,18 @@ export let componentEffectDuration: number = -0; export let componentEffectStartTime: number = -1.1; export let componentEffectEndTime: number = -1.1; -let blockingClampTime: number = -0; +export let blockingClampTime: number = -0; export let blockingUpdateTime: number = -1.1; // First sync setState scheduled. export let blockingEventTime: number = -1.1; // Event timeStamp of the first setState. export let blockingEventType: null | string = null; // Event type of the first setState. +export let blockingEventIsRepeat: boolean = false; // TODO: This should really be one per Transition lane. -let transitionClampTime: number = -0; +export let transitionClampTime: number = -0; export let transitionStartTime: number = -1.1; // First startTransition call before setState. export let transitionUpdateTime: number = -1.1; // First transition setState scheduled. export let transitionEventTime: number = -1.1; // Event timeStamp of the first transition. export let transitionEventType: null | string = null; // Event type of the first transition. +export let transitionEventIsRepeat: boolean = false; export function startUpdateTimerByLane(lane: Lane): void { if (!enableProfilerTimer || !enableComponentPerformanceTrack) { @@ -54,25 +56,25 @@ export function startUpdateTimerByLane(lane: Lane): void { if (isSyncLane(lane) || isBlockingLane(lane)) { if (blockingUpdateTime < 0) { blockingUpdateTime = now(); - blockingEventTime = resolveEventTimeStamp(); - blockingEventType = resolveEventType(); - if (blockingEventTime < blockingClampTime) { - // We've already rendered within this event and we need to clamp the new update - // to the end of that render. - blockingEventTime = blockingClampTime; - } + const newEventTime = resolveEventTimeStamp(); + const newEventType = resolveEventType(); + blockingEventIsRepeat = + newEventTime === blockingEventTime && + newEventType === blockingEventType; + blockingEventTime = newEventTime; + blockingEventType = newEventType; } } else if (isTransitionLane(lane)) { if (transitionUpdateTime < 0) { transitionUpdateTime = now(); if (transitionStartTime < 0) { - transitionEventTime = resolveEventTimeStamp(); - transitionEventType = resolveEventType(); - if (transitionEventTime < transitionClampTime) { - // We've already rendered within this event and we need to clamp the new update - // to the end of that render. - transitionEventTime = transitionClampTime; - } + const newEventTime = resolveEventTimeStamp(); + const newEventType = resolveEventType(); + transitionEventIsRepeat = + newEventTime === transitionEventTime && + newEventType === transitionEventType; + transitionEventTime = newEventTime; + transitionEventType = newEventType; } } } @@ -88,13 +90,13 @@ export function startAsyncTransitionTimer(): void { } if (transitionStartTime < 0 && transitionUpdateTime < 0) { transitionStartTime = now(); - transitionEventTime = resolveEventTimeStamp(); - transitionEventType = resolveEventType(); - if (transitionEventTime < transitionClampTime) { - // We've already rendered within this event and we need to clamp the new update - // to the end of that render. - transitionEventTime = transitionClampTime; - } + const newEventTime = resolveEventTimeStamp(); + const newEventType = resolveEventType(); + transitionEventIsRepeat = + newEventTime === transitionEventTime && + newEventType === transitionEventType; + transitionEventTime = newEventTime; + transitionEventType = newEventType; } } @@ -132,13 +134,6 @@ export function clampBlockingTimers(finalTime: number): void { // If we had new updates come in while we were still rendering or committing, we don't want // those update times to create overlapping tracks in the performance timeline so we clamp // them to the end of the commit phase. - if (blockingUpdateTime >= 0 && blockingUpdateTime < finalTime) { - blockingUpdateTime = finalTime; - } - if (blockingEventTime >= 0 && blockingEventTime < finalTime) { - blockingEventTime = finalTime; - } - // Save this for later in case another update comes in later with an early event time. blockingClampTime = finalTime; } @@ -149,16 +144,6 @@ export function clampTransitionTimers(finalTime: number): void { // If we had new updates come in while we were still rendering or committing, we don't want // those update times to create overlapping tracks in the performance timeline so we clamp // them to the end of the commit phase. - if (transitionStartTime >= 0 && transitionStartTime < finalTime) { - transitionStartTime = finalTime; - } - if (transitionUpdateTime >= 0 && transitionUpdateTime < finalTime) { - transitionUpdateTime = finalTime; - } - if (transitionEventTime >= 0 && transitionEventTime < finalTime) { - transitionEventTime = finalTime; - } - // Save this for later in case another update comes in later with an early event time. transitionClampTime = finalTime; } From d1d95a5f0ed267ce817e469370501b9e85713cc4 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 13 Nov 2024 20:45:55 -0500 Subject: [PATCH 4/4] Track current event at the beginning of a scheduled task That way we can ignore this event when we log where an update happened. --- packages/react-art/src/ReactFiberConfigART.js | 2 ++ .../src/client/ReactFiberConfigDOM.js | 9 +++++++-- .../src/ReactFiberConfigFabric.js | 2 ++ .../src/ReactFiberConfigNative.js | 2 ++ .../react-noop-renderer/src/createReactNoop.js | 2 ++ .../src/ReactFiberRootScheduler.js | 14 ++++++++++++++ .../react-reconciler/src/ReactFiberWorkLoop.js | 6 ++++++ .../ReactFiberHostContext-test.internal.js | 1 + .../src/forks/ReactFiberConfig.custom.js | 1 + .../src/ReactFiberConfigTestHost.js | 3 ++- 10 files changed, 39 insertions(+), 3 deletions(-) diff --git a/packages/react-art/src/ReactFiberConfigART.js b/packages/react-art/src/ReactFiberConfigART.js index 33bbe055724d9..b88d6008c4bd2 100644 --- a/packages/react-art/src/ReactFiberConfigART.js +++ b/packages/react-art/src/ReactFiberConfigART.js @@ -363,6 +363,8 @@ export function resolveUpdatePriority(): EventPriority { return currentUpdatePriority || DefaultEventPriority; } +export function trackSchedulerEvent(): void {} + export function resolveEventType(): null | string { return null; } diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index af53a2a6acf47..e3f6ff56f6ecf 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -606,14 +606,19 @@ export function shouldAttemptEagerTransition(): boolean { return false; } +let schedulerEvent: void | Event = undefined; +export function trackSchedulerEvent(): void { + schedulerEvent = window.event; +} + export function resolveEventType(): null | string { const event = window.event; - return event ? event.type : null; + return event && event !== schedulerEvent ? event.type : null; } export function resolveEventTimeStamp(): number { const event = window.event; - return event ? event.timeStamp : -1.1; + return event && event !== schedulerEvent ? event.timeStamp : -1.1; } export const isPrimaryRenderer = true; diff --git a/packages/react-native-renderer/src/ReactFiberConfigFabric.js b/packages/react-native-renderer/src/ReactFiberConfigFabric.js index 10f7c6ecd84a3..2b0997d2e8b3f 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigFabric.js +++ b/packages/react-native-renderer/src/ReactFiberConfigFabric.js @@ -372,6 +372,8 @@ export function resolveUpdatePriority(): EventPriority { return DefaultEventPriority; } +export function trackSchedulerEvent(): void {} + export function resolveEventType(): null | string { return null; } diff --git a/packages/react-native-renderer/src/ReactFiberConfigNative.js b/packages/react-native-renderer/src/ReactFiberConfigNative.js index 1dc8c627b1ccb..774a84a7d63e9 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigNative.js +++ b/packages/react-native-renderer/src/ReactFiberConfigNative.js @@ -288,6 +288,8 @@ export function resolveUpdatePriority(): EventPriority { return DefaultEventPriority; } +export function trackSchedulerEvent(): void {} + export function resolveEventType(): null | string { return null; } diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 4c1bf05e54046..2e24c07932cac 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -531,6 +531,8 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { return currentEventPriority; }, + trackSchedulerEvent(): void {}, + resolveEventType(): null | string { return null; }, diff --git a/packages/react-reconciler/src/ReactFiberRootScheduler.js b/packages/react-reconciler/src/ReactFiberRootScheduler.js index 1f529d9e4df31..8756a9c89009c 100644 --- a/packages/react-reconciler/src/ReactFiberRootScheduler.js +++ b/packages/react-reconciler/src/ReactFiberRootScheduler.js @@ -18,6 +18,7 @@ import { disableSchedulerTimeoutInWorkLoop, enableProfilerTimer, enableProfilerNestedUpdatePhase, + enableComponentPerformanceTrack, enableSiblingPrerendering, } from 'shared/ReactFeatureFlags'; import { @@ -64,6 +65,7 @@ import { supportsMicrotasks, scheduleMicrotask, shouldAttemptEagerTransition, + trackSchedulerEvent, } from './ReactFiberConfig'; import ReactSharedInternals from 'shared/ReactSharedInternals'; @@ -225,6 +227,12 @@ function flushSyncWorkAcrossRoots_impl( } function processRootScheduleInMicrotask() { + if (enableProfilerTimer && enableComponentPerformanceTrack) { + // Track the currently executing event if there is one so we can ignore this + // event when logging events. + trackSchedulerEvent(); + } + // This function is always called inside a microtask. It should never be // called synchronously. didScheduleMicrotask = false; @@ -428,6 +436,12 @@ function performWorkOnRootViaSchedulerTask( resetNestedUpdateFlag(); } + if (enableProfilerTimer && enableComponentPerformanceTrack) { + // Track the currently executing event if there is one so we can ignore this + // event when logging events. + trackSchedulerEvent(); + } + // Flush any pending passive effects before deciding which lanes to work on, // in case they schedule additional work. const originalCallbackNode = root.callbackNode; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index b5e594a15b3cc..36189aea53ca4 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -90,6 +90,7 @@ import { setCurrentUpdatePriority, getCurrentUpdatePriority, resolveUpdatePriority, + trackSchedulerEvent, } from './ReactFiberConfig'; import {createWorkInProgress, resetWorkInProgress} from './ReactFiber'; @@ -3161,6 +3162,11 @@ function commitRootImpl( // with setTimeout pendingPassiveTransitions = transitions; scheduleCallback(NormalSchedulerPriority, () => { + if (enableProfilerTimer && enableComponentPerformanceTrack) { + // Track the currently executing event if there is one so we can ignore this + // event when logging events. + trackSchedulerEvent(); + } flushPassiveEffects(true); // This render triggered passive effects: release the root cache pool // *after* passive effects fire to avoid freeing a cache pool that may diff --git a/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js b/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js index 60726514474fb..fbbe9a3bf71af 100644 --- a/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactFiberHostContext-test.internal.js @@ -83,6 +83,7 @@ describe('ReactFiberHostContext', () => { } return DefaultEventPriority; }, + trackSchedulerEvent: function () {}, resolveEventType: function () { return null; }, diff --git a/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js b/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js index 1f525d9a05c52..1ed0210e90ee5 100644 --- a/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js +++ b/packages/react-reconciler/src/forks/ReactFiberConfig.custom.js @@ -73,6 +73,7 @@ export const getInstanceFromScope = $$$config.getInstanceFromScope; export const setCurrentUpdatePriority = $$$config.setCurrentUpdatePriority; export const getCurrentUpdatePriority = $$$config.getCurrentUpdatePriority; export const resolveUpdatePriority = $$$config.resolveUpdatePriority; +export const trackSchedulerEvent = $$$config.trackSchedulerEvent; export const resolveEventType = $$$config.resolveEventType; export const resolveEventTimeStamp = $$$config.resolveEventTimeStamp; export const shouldAttemptEagerTransition = diff --git a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js index cb38a985eb800..eaa0332fe8b29 100644 --- a/packages/react-test-renderer/src/ReactFiberConfigTestHost.js +++ b/packages/react-test-renderer/src/ReactFiberConfigTestHost.js @@ -224,10 +224,11 @@ export function resolveUpdatePriority(): EventPriority { } return DefaultEventPriority; } + +export function trackSchedulerEvent(): void {} export function resolveEventType(): null | string { return null; } - export function resolveEventTimeStamp(): number { return -1.1; }