Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Fiber] Move Profiler onPostCommit processing of passive effect durations to plain passive effect #30966

Merged
merged 2 commits into from
Sep 16, 2024
Merged

[Fiber] Move Profiler onPostCommit processing of passive effect durations to plain passive effect #30966

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
816f86f
Extract Profiler onPostCommit to CommitEffects
sebmarkbage Sep 15, 2024
08139d7
Move onPostCommit processing of passive effect durations to plain pas…
sebmarkbage Sep 15, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions packages/react-reconciler/src/ReactFiberBeginWork.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1026,6 +1026,10 @@ function updateProfiler(
workInProgress.flags |= Update;

if (enableProfilerCommitHooks) {
// Schedule a passive effect for this Profiler to call onPostCommit hooks.
// This effect should be scheduled even if there is no onPostCommit callback for this Profiler,
// because the effect is also where times bubble to parent Profilers.
workInProgress.flags |= Passive;
// Reset effect durations for the next eventual effect phase.
// These are reset during render to allow the DevTools commit hook a chance to read them,
const stateNode = workInProgress.stateNode;
Expand Down Expand Up @@ -3700,6 +3704,10 @@ function attemptEarlyBailoutIfNoScheduledUpdate(
}

if (enableProfilerCommitHooks) {
// Schedule a passive effect for this Profiler to call onPostCommit hooks.
// This effect should be scheduled even if there is no onPostCommit callback for this Profiler,
// because the effect is also where times bubble to parent Profilers.
workInProgress.flags |= Passive;
// Reset effect durations for the next eventual effect phase.
// These are reset during render to allow the DevTools commit hook a chance to read them,
const stateNode = workInProgress.stateNode;
Expand Down
53 changes: 51 additions & 2 deletions packages/react-reconciler/src/ReactFiberCommitEffects.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -881,7 +881,7 @@ function commitProfiler(
commitTime: number,
effectDuration: number,
) {
const {onCommit, onRender} = finishedWork.memoizedProps;
const {id, onCommit, onRender} = finishedWork.memoizedProps;

let phase = current === null ? 'mount' : 'update';
if (enableProfilerNestedUpdatePhase) {
Expand All @@ -892,7 +892,7 @@ function commitProfiler(

if (typeof onRender === 'function') {
onRender(
finishedWork.memoizedProps.id,
id,
phase,
finishedWork.actualDuration,
finishedWork.treeBaseDuration,
Expand Down Expand Up @@ -938,3 +938,52 @@ export function commitProfilerUpdate(
}
}
}

function commitProfilerPostCommitImpl(
finishedWork: Fiber,
current: Fiber | null,
commitTime: number,
passiveEffectDuration: number,
): void {
const {id, onPostCommit} = finishedWork.memoizedProps;

let phase = current === null ? 'mount' : 'update';
if (enableProfilerNestedUpdatePhase) {
if (isCurrentUpdateNested()) {
phase = 'nested-update';
}
}

if (typeof onPostCommit === 'function') {
onPostCommit(id, phase, passiveEffectDuration, commitTime);
}
}

export function commitProfilerPostCommit(
finishedWork: Fiber,
current: Fiber | null,
commitTime: number,
passiveEffectDuration: number,
) {
try {
if (__DEV__) {
runWithFiberInDEV(
finishedWork,
commitProfilerPostCommitImpl,
finishedWork,
current,
commitTime,
passiveEffectDuration,
);
} else {
commitProfilerPostCommitImpl(
finishedWork,
current,
commitTime,
passiveEffectDuration,
);
}
} catch (error) {
captureCommitPhaseError(finishedWork, finishedWork.return, error);
}
}
116 changes: 47 additions & 69 deletions packages/react-reconciler/src/ReactFiberCommitWork.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@ import {
enablePersistedModeClonedFlag,
enableProfilerTimer,
enableProfilerCommitHooks,
enableProfilerNestedUpdatePhase,
enableSchedulingProfiler,
enableSuspenseCallback,
enableScopeAPI,
Expand Down Expand Up @@ -100,7 +99,6 @@ import {
Cloned,
} from './ReactFiberFlags';
import {
isCurrentUpdateNested,
getCommitTime,
recordLayoutEffectDuration,
startLayoutEffectTimer,
Expand Down Expand Up @@ -137,7 +135,6 @@ import {
captureCommitPhaseError,
resolveRetryWakeable,
markCommitTimeOfFallback,
enqueuePendingPassiveProfilerEffect,
restorePendingUpdaters,
addTransitionStartCallbackToPendingTransition,
addTransitionProgressCallbackToPendingTransition,
Expand Down Expand Up @@ -193,6 +190,7 @@ import {
safelyDetachRef,
safelyCallDestroy,
commitProfilerUpdate,
commitProfilerPostCommit,
commitRootCallbacks,
} from './ReactFiberCommitEffects';
import {
Expand Down Expand Up @@ -394,62 +392,6 @@ function commitBeforeMutationEffectsDeletion(deletion: Fiber) {
}
}

export function commitPassiveEffectDurations(
finishedRoot: FiberRoot,
finishedWork: Fiber,
): void {
if (
enableProfilerTimer &&
enableProfilerCommitHooks &&
getExecutionContext() & CommitContext
) {
// Only Profilers with work in their subtree will have an Update effect scheduled.
if ((finishedWork.flags & Update) !== NoFlags) {
switch (finishedWork.tag) {
case Profiler: {
const {passiveEffectDuration} = finishedWork.stateNode;
const {id, onPostCommit} = finishedWork.memoizedProps;

// This value will still reflect the previous commit phase.
// It does not get reset until the start of the next commit phase.
const commitTime = getCommitTime();

let phase = finishedWork.alternate === null ? 'mount' : 'update';
if (enableProfilerNestedUpdatePhase) {
if (isCurrentUpdateNested()) {
phase = 'nested-update';
}
}

if (typeof onPostCommit === 'function') {
onPostCommit(id, phase, passiveEffectDuration, commitTime);
}

// Bubble times to the next nearest ancestor Profiler.
// After we process that Profiler, we'll bubble further up.
let parentFiber = finishedWork.return;
outer: while (parentFiber !== null) {
switch (parentFiber.tag) {
case HostRoot:
const root = parentFiber.stateNode;
root.passiveEffectDuration += passiveEffectDuration;
break outer;
case Profiler:
const parentStateNode = parentFiber.stateNode;
parentStateNode.passiveEffectDuration += passiveEffectDuration;
break outer;
}
parentFiber = parentFiber.return;
}
break;
}
default:
break;
}
}
}
}

function commitLayoutEffectOnFiber(
finishedRoot: FiberRoot,
current: Fiber | null,
Expand Down Expand Up @@ -557,11 +499,6 @@ function commitLayoutEffectOnFiber(
effectDuration,
);

// Schedule a passive effect for this Profiler to call onPostCommit hooks.
// This effect should be scheduled even if there is no onPostCommit callback for this Profiler,
// because the effect is also where times bubble to parent Profilers.
enqueuePendingPassiveProfilerEffect(finishedWork);

// Propagate layout effect durations to the next nearest Profiler ancestor.
// Do not reset these values until the next render so DevTools has a chance to read them first.
let parentFiber = finishedWork.return;
Expand Down Expand Up @@ -2475,11 +2412,6 @@ export function reappearLayoutEffects(
effectDuration,
);

// Schedule a passive effect for this Profiler to call onPostCommit hooks.
// This effect should be scheduled even if there is no onPostCommit callback for this Profiler,
// because the effect is also where times bubble to parent Profilers.
enqueuePendingPassiveProfilerEffect(finishedWork);

// Propagate layout effect durations to the next nearest Profiler ancestor.
// Do not reset these values until the next render so DevTools has a chance to read them first.
let parentFiber = finishedWork.return;
Expand Down Expand Up @@ -2824,6 +2756,52 @@ function commitPassiveMountOnFiber(
}
break;
}
case Profiler: {
recursivelyTraversePassiveMountEffects(
finishedRoot,
finishedWork,
committedLanes,
committedTransitions,
);

// Only Profilers with work in their subtree will have a Passive effect scheduled.
if (flags & Passive) {
if (
enableProfilerTimer &&
enableProfilerCommitHooks &&
getExecutionContext() & CommitContext
) {
const {passiveEffectDuration} = finishedWork.stateNode;

commitProfilerPostCommit(
finishedWork,
finishedWork.alternate,
// This value will still reflect the previous commit phase.
// It does not get reset until the start of the next commit phase.
getCommitTime(),
passiveEffectDuration,
);

// Bubble times to the next nearest ancestor Profiler.
// After we process that Profiler, we'll bubble further up.
let parentFiber = finishedWork.return;
outer: while (parentFiber !== null) {
switch (parentFiber.tag) {
case HostRoot:
const root = parentFiber.stateNode;
root.passiveEffectDuration += passiveEffectDuration;
break outer;
case Profiler:
const parentStateNode = parentFiber.stateNode;
parentStateNode.passiveEffectDuration += passiveEffectDuration;
break outer;
}
parentFiber = parentFiber.return;
}
}
}
break;
}
case LegacyHiddenComponent: {
if (enableLegacyHidden) {
recursivelyTraversePassiveMountEffects(
Expand Down
25 changes: 0 additions & 25 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,6 @@ import {
commitBeforeMutationEffects,
commitLayoutEffects,
commitMutationEffects,
commitPassiveEffectDurations,
commitPassiveMountEffects,
commitPassiveUnmountEffects,
disappearLayoutEffects,
Expand Down Expand Up @@ -580,7 +579,6 @@ let legacyErrorBoundariesThatAlreadyFailed: Set<mixed> | null = null;
let rootDoesHavePassiveEffects: boolean = false;
let rootWithPendingPassiveEffects: FiberRoot | null = null;
let pendingPassiveEffectsLanes: Lanes = NoLanes;
let pendingPassiveProfilerEffects: Array<Fiber> = [];
let pendingPassiveEffectsRemainingLanes: Lanes = NoLanes;
let pendingPassiveTransitions: Array<Transition> | null = null;

Expand Down Expand Up @@ -3475,19 +3473,6 @@ export function flushPassiveEffects(): boolean {
return false;
}

export function enqueuePendingPassiveProfilerEffect(fiber: Fiber): void {
if (enableProfilerTimer && enableProfilerCommitHooks) {
pendingPassiveProfilerEffects.push(fiber);
if (!rootDoesHavePassiveEffects) {
rootDoesHavePassiveEffects = true;
scheduleCallback(NormalSchedulerPriority, () => {
flushPassiveEffects();
return null;
});
}
}
}

function flushPassiveEffectsImpl() {
if (rootWithPendingPassiveEffects === null) {
return false;
Expand Down Expand Up @@ -3528,16 +3513,6 @@ function flushPassiveEffectsImpl() {
commitPassiveUnmountEffects(root.current);
commitPassiveMountEffects(root, root.current, lanes, transitions);

// TODO: Move to commitPassiveMountEffects
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is fulfilling this TODO.

if (enableProfilerTimer && enableProfilerCommitHooks) {
const profilerEffects = pendingPassiveProfilerEffects;
pendingPassiveProfilerEffects = [];
for (let i = 0; i < profilerEffects.length; i++) {
const fiber = ((profilerEffects[i]: any): Fiber);
commitPassiveEffectDurations(root, fiber);
}
}

if (__DEV__) {
if (enableDebugTracing) {
logPassiveEffectsStopped();
Expand Down
Loading