Skip to content

Commit

Permalink
Fix Logging of Immediately Resolved Promises (facebook#31610)
Browse files Browse the repository at this point in the history
This avoid re-emitting the yellow "Event" log when we ping inside the
original event. Instead of treating events as repeated when we get
repeated updates, we treat them as repeated if we've ever logged out
this event before.

Additionally, in the case the prerender sibling flag is on we need to
ensure that if a render gets interrupted when it has been suspended we
treat that as "Prewarm" instead of "Interrupted Render".

Before:
<img width="539" alt="Screenshot 2024-11-19 at 2 39 44 PM"
src="https://github.com/user-attachments/assets/190ca50c-5168-40d8-a6fd-6b9a583af1f0">

After:

<img width="1004" alt="Screenshot 2024-11-21 at 4 53 16 PM"
src="https://github.com/user-attachments/assets/0c441ada-1ed1-412c-8935-aaf040c25dfe">
  • Loading branch information
sebmarkbage authored Nov 21, 2024
1 parent c11c951 commit a9f14cb
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 26 deletions.
17 changes: 13 additions & 4 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,6 @@ import {
startProfilerTimer,
stopProfilerTimerIfRunningAndRecordDuration,
stopProfilerTimerIfRunningAndRecordIncompleteDuration,
markUpdateAsRepeat,
trackSuspendedTime,
startYieldTimer,
yieldStartTime,
Expand Down Expand Up @@ -927,6 +926,7 @@ export function performWorkOnRoot(
// We've returned from yielding to the event loop. Let's log the time it took.
const yieldEndTime = now();
switch (yieldReason) {
case SuspendedOnImmediate:
case SuspendedOnData:
logSuspendedYieldTime(yieldStartTime, yieldEndTime, yieldedFiber);
break;
Expand Down Expand Up @@ -1009,7 +1009,6 @@ export function performWorkOnRoot(
setCurrentTrackFromLanes(lanes);
logInconsistentRender(renderStartTime, renderEndTime);
finalizeRender(lanes, renderEndTime);
markUpdateAsRepeat(lanes);
}
// A store was mutated in an interleaved event. Render again,
// synchronously, to block further mutations.
Expand All @@ -1036,7 +1035,6 @@ export function performWorkOnRoot(
setCurrentTrackFromLanes(lanes);
logErroredRenderPhase(renderStartTime, renderEndTime);
finalizeRender(lanes, renderEndTime);
markUpdateAsRepeat(lanes);
}
lanes = errorRetryLanes;
exitStatus = recoverFromConcurrentError(
Expand Down Expand Up @@ -1740,7 +1738,18 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes): Fiber {
previousRenderStartTime > 0
) {
setCurrentTrackFromLanes(workInProgressRootRenderLanes);
logInterruptedRenderPhase(previousRenderStartTime, renderStartTime);
if (
workInProgressRootExitStatus === RootSuspended ||
workInProgressRootExitStatus === RootSuspendedWithDelay
) {
// If the root was already suspended when it got interrupted and restarted,
// 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);
} else {
logInterruptedRenderPhase(previousRenderStartTime, renderStartTime);
}
finalizeRender(workInProgressRootRenderLanes, renderStartTime);
}

Expand Down
42 changes: 20 additions & 22 deletions packages/react-reconciler/src/ReactProfilerTimer.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,12 @@ export function startUpdateTimerByLane(lane: Lane): void {
blockingUpdateTime = now();
const newEventTime = resolveEventTimeStamp();
const newEventType = resolveEventType();
blockingEventIsRepeat =
newEventTime === blockingEventTime &&
newEventType === blockingEventType;
if (
newEventTime !== blockingEventTime ||
newEventType !== blockingEventType
) {
blockingEventIsRepeat = false;
}
blockingEventTime = newEventTime;
blockingEventType = newEventType;
}
Expand All @@ -92,29 +95,19 @@ export function startUpdateTimerByLane(lane: Lane): void {
if (transitionStartTime < 0) {
const newEventTime = resolveEventTimeStamp();
const newEventType = resolveEventType();
transitionEventIsRepeat =
newEventTime === transitionEventTime &&
newEventType === transitionEventType;
if (
newEventTime !== transitionEventTime ||
newEventType !== transitionEventType
) {
transitionEventIsRepeat = false;
}
transitionEventTime = newEventTime;
transitionEventType = newEventType;
}
}
}
}

export function markUpdateAsRepeat(lanes: Lanes): void {
if (!enableProfilerTimer || !enableComponentPerformanceTrack) {
return;
}
// We're about to do a retry of this render. It is not a new update, so treat this
// as a repeat within the same event.
if (includesSyncLane(lanes) || includesBlockingLane(lanes)) {
blockingEventIsRepeat = true;
} else if (includesTransitionLane(lanes)) {
transitionEventIsRepeat = true;
}
}

export function trackSuspendedTime(lanes: Lanes, renderEndTime: number) {
if (!enableProfilerTimer || !enableComponentPerformanceTrack) {
return;
Expand All @@ -129,6 +122,7 @@ export function trackSuspendedTime(lanes: Lanes, renderEndTime: number) {
export function clearBlockingTimers(): void {
blockingUpdateTime = -1.1;
blockingSuspendedTime = -1.1;
blockingEventIsRepeat = true;
}

export function startAsyncTransitionTimer(): void {
Expand All @@ -139,9 +133,12 @@ export function startAsyncTransitionTimer(): void {
transitionStartTime = now();
const newEventTime = resolveEventTimeStamp();
const newEventType = resolveEventType();
transitionEventIsRepeat =
newEventTime === transitionEventTime &&
newEventType === transitionEventType;
if (
newEventTime !== transitionEventTime ||
newEventType !== transitionEventType
) {
transitionEventIsRepeat = false;
}
transitionEventTime = newEventTime;
transitionEventType = newEventType;
}
Expand Down Expand Up @@ -173,6 +170,7 @@ export function clearTransitionTimers(): void {
transitionStartTime = -1.1;
transitionUpdateTime = -1.1;
transitionSuspendedTime = -1.1;
transitionEventIsRepeat = true;
}

export function clampBlockingTimers(finalTime: number): void {
Expand Down

0 comments on commit a9f14cb

Please sign in to comment.