From 6cf85c7efae8977ecd4a5739568447931c0b69e5 Mon Sep 17 00:00:00 2001 From: Sebastian Markbage Date: Wed, 13 Nov 2024 16:52:01 -0500 Subject: [PATCH] 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 | 12 +++- .../src/ReactFiberWorkLoop.js | 26 ++++++-- .../src/ReactProfilerTimer.js | 65 +++++++------------ 3 files changed, 56 insertions(+), 47 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberPerformanceTrack.js b/packages/react-reconciler/src/ReactFiberPerformanceTrack.js index 3d52dcf4e1073..73cad82321fda 100644 --- a/packages/react-reconciler/src/ReactFiberPerformanceTrack.js +++ b/packages/react-reconciler/src/ReactFiberPerformanceTrack.js @@ -104,6 +104,7 @@ export function logBlockingStart( updateTime: number, eventTime: number, eventType: null | string, + eventIsRepeat: boolean, renderStartTime: number, ): void { if (supportsUserTiming) { @@ -114,7 +115,10 @@ export function logBlockingStart( reusableComponentOptions.start = eventTime; reusableComponentOptions.end = updateTime > 0 ? updateTime : renderStartTime; - performance.measure(eventType, reusableComponentOptions); + performance.measure( + eventIsRepeat ? '' : eventType, + reusableComponentOptions, + ); } if (updateTime > 0) { // Log the time from when we called setState until we started rendering. @@ -131,6 +135,7 @@ export function logTransitionStart( updateTime: number, eventTime: number, eventType: null | string, + eventIsRepeat: boolean, renderStartTime: number, ): void { if (supportsUserTiming) { @@ -145,7 +150,10 @@ export function logTransitionStart( : updateTime > 0 ? updateTime : renderStartTime; - performance.measure(eventType, reusableComponentOptions); + performance.measure( + eventIsRepeat ? '' : eventType, + reusableComponentOptions, + ); } 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 5f7f80d5f3aef..d7f77f929e6e0 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; }