From 0e8a5aff3ddf0f863839a924738f958fd940e3be Mon Sep 17 00:00:00 2001 From: Brian Vaughn Date: Thu, 21 Oct 2021 14:41:44 -0400 Subject: [PATCH] Scheduling Profiler: Add marks for component effects (mount and unmount) (#22578) DevTools (and its profilers) should not require users to be familiar with React internals. Although the scheduling profiler includes a CPU sample flame graph, it's there for advanced use cases and shouldn't be required to identify common performance issues. This PR proposes adding new marks around component effects. This will enable users to identify components with slow effect create/destroy functions without requiring them to dig through the call stack. (Once #22529 lands, these new marks will also include component stacks, making them more useful still.) For example, here's a profile with a long-running effect. Without this change, it's not clear why the effects phase takes so long. After this change, it's more clear why that the phase is longer because of a specific component. We may consider adding similar marks around render phase hooks like useState, useReducer, useMemo. I avoided doing that in this PR because it would be a pretty pervasive change to the ReactFiberHooks file. Note that this change should have no effect on production bundles since everything is guarded behind a profiling feature flag. Going to tag more people than I normally would for this pR, since it touches both reconciler and DevTools packages. Feel free to ignore though if you don't have strong feelings. Note that although this PR adds new marks to the scheduling profiler, it's done in a way that's backwards compatible for older profiles. --- .../src/EventTooltip.js | 21 +- .../content-views/ComponentMeasuresView.js | 57 +++- .../src/content-views/constants.js | 8 - .../__tests__/preprocessData-test.internal.js | 62 +++-- .../src/import-worker/preprocessData.js | 181 +++++++++++-- .../src/types.js | 8 + .../src/ReactFiberCommitWork.new.js | 62 ++++- .../src/ReactFiberCommitWork.old.js | 62 ++++- .../src/SchedulingProfiler.js | 72 ++++++ .../SchedulingProfiler-test.internal.js | 243 ++++++++++++++---- 10 files changed, 649 insertions(+), 127 deletions(-) diff --git a/packages/react-devtools-scheduling-profiler/src/EventTooltip.js b/packages/react-devtools-scheduling-profiler/src/EventTooltip.js index 9152c7de8f56e..d3ca5d5625f13 100644 --- a/packages/react-devtools-scheduling-profiler/src/EventTooltip.js +++ b/packages/react-devtools-scheduling-profiler/src/EventTooltip.js @@ -140,9 +140,26 @@ const TooltipReactComponentMeasure = ({ }: {| componentMeasure: ReactComponentMeasure, |}) => { - const {componentName, duration, timestamp, warning} = componentMeasure; + const {componentName, duration, timestamp, type, warning} = componentMeasure; - const label = `${componentName} rendered`; + let label = componentName; + switch (type) { + case 'render': + label += ' rendered'; + break; + case 'layout-effect-mount': + label += ' mounted layout effect'; + break; + case 'layout-effect-unmount': + label += ' unmounted layout effect'; + break; + case 'passive-effect-mount': + label += ' mounted passive effect'; + break; + case 'passive-effect-unmount': + label += ' unmounted passive effect'; + break; + } return ( <> diff --git a/packages/react-devtools-scheduling-profiler/src/content-views/ComponentMeasuresView.js b/packages/react-devtools-scheduling-profiler/src/content-views/ComponentMeasuresView.js index c7de8f8897c5e..a241498a05a05 100644 --- a/packages/react-devtools-scheduling-profiler/src/content-views/ComponentMeasuresView.js +++ b/packages/react-devtools-scheduling-profiler/src/content-views/ComponentMeasuresView.js @@ -76,7 +76,13 @@ export class ComponentMeasuresView extends View { showHoverHighlight: boolean, ): boolean { const {frame} = this; - const {componentName, duration, timestamp, warning} = componentMeasure; + const { + componentName, + duration, + timestamp, + type, + warning, + } = componentMeasure; const xStart = timestampToPosition(timestamp, scaleFactor, frame); const xStop = timestampToPosition(timestamp + duration, scaleFactor, frame); @@ -96,6 +102,9 @@ export class ComponentMeasuresView extends View { return false; // Too small to render at this zoom level } + let textFillStyle = ((null: any): string); + let typeLabel = ((null: any): string); + const drawableRect = intersectionOfRects(componentMeasureRect, rect); context.beginPath(); if (warning !== null) { @@ -103,9 +112,43 @@ export class ComponentMeasuresView extends View { ? COLORS.WARNING_BACKGROUND_HOVER : COLORS.WARNING_BACKGROUND; } else { - context.fillStyle = showHoverHighlight - ? COLORS.REACT_COMPONENT_MEASURE_HOVER - : COLORS.REACT_COMPONENT_MEASURE; + switch (type) { + case 'render': + context.fillStyle = showHoverHighlight + ? COLORS.REACT_RENDER_HOVER + : COLORS.REACT_RENDER; + textFillStyle = COLORS.REACT_RENDER_TEXT; + typeLabel = 'rendered'; + break; + case 'layout-effect-mount': + context.fillStyle = showHoverHighlight + ? COLORS.REACT_LAYOUT_EFFECTS_HOVER + : COLORS.REACT_LAYOUT_EFFECTS; + textFillStyle = COLORS.REACT_LAYOUT_EFFECTS_TEXT; + typeLabel = 'mounted layout effect'; + break; + case 'layout-effect-unmount': + context.fillStyle = showHoverHighlight + ? COLORS.REACT_LAYOUT_EFFECTS_HOVER + : COLORS.REACT_LAYOUT_EFFECTS; + textFillStyle = COLORS.REACT_LAYOUT_EFFECTS_TEXT; + typeLabel = 'unmounted layout effect'; + break; + case 'passive-effect-mount': + context.fillStyle = showHoverHighlight + ? COLORS.REACT_PASSIVE_EFFECTS_HOVER + : COLORS.REACT_PASSIVE_EFFECTS; + textFillStyle = COLORS.REACT_PASSIVE_EFFECTS_TEXT; + typeLabel = 'mounted passive effect'; + break; + case 'passive-effect-unmount': + context.fillStyle = showHoverHighlight + ? COLORS.REACT_PASSIVE_EFFECTS_HOVER + : COLORS.REACT_PASSIVE_EFFECTS; + textFillStyle = COLORS.REACT_PASSIVE_EFFECTS_TEXT; + typeLabel = 'unmounted passive effect'; + break; + } } context.fillRect( drawableRect.origin.x, @@ -114,9 +157,11 @@ export class ComponentMeasuresView extends View { drawableRect.size.height, ); - const label = `${componentName} rendered - ${formatDuration(duration)}`; + const label = `${componentName} ${typeLabel} - ${formatDuration(duration)}`; - drawText(label, context, componentMeasureRect, drawableRect); + drawText(label, context, componentMeasureRect, drawableRect, { + fillStyle: textFillStyle, + }); return true; } diff --git a/packages/react-devtools-scheduling-profiler/src/content-views/constants.js b/packages/react-devtools-scheduling-profiler/src/content-views/constants.js index 4d896d1967e96..d0895d2aff8e7 100644 --- a/packages/react-devtools-scheduling-profiler/src/content-views/constants.js +++ b/packages/react-devtools-scheduling-profiler/src/content-views/constants.js @@ -59,8 +59,6 @@ export let COLORS = { PRIORITY_LABEL: '', USER_TIMING: '', USER_TIMING_HOVER: '', - REACT_COMPONENT_MEASURE: '', - REACT_COMPONENT_MEASURE_HOVER: '', REACT_IDLE: '', REACT_IDLE_HOVER: '', REACT_RENDER: '', @@ -150,12 +148,6 @@ export function updateColorsToMatchTheme(element: Element): boolean { USER_TIMING_HOVER: computedStyle.getPropertyValue( '--color-scheduling-profiler-user-timing-hover', ), - REACT_COMPONENT_MEASURE: computedStyle.getPropertyValue( - '--color-scheduling-profiler-react-render', - ), - REACT_COMPONENT_MEASURE_HOVER: computedStyle.getPropertyValue( - '--color-scheduling-profiler-react-render-hover', - ), REACT_IDLE: computedStyle.getPropertyValue( '--color-scheduling-profiler-react-idle', ), diff --git a/packages/react-devtools-scheduling-profiler/src/import-worker/__tests__/preprocessData-test.internal.js b/packages/react-devtools-scheduling-profiler/src/import-worker/__tests__/preprocessData-test.internal.js index f8690dc57447c..30cba586615c6 100644 --- a/packages/react-devtools-scheduling-profiler/src/import-worker/__tests__/preprocessData-test.internal.js +++ b/packages/react-devtools-scheduling-profiler/src/import-worker/__tests__/preprocessData-test.internal.js @@ -840,7 +840,7 @@ describe('preprocessData', () => { Object { "batchUID": 0, "depth": 0, - "duration": 0.0019999999999999983, + "duration": 0.004, "lanes": Array [ 4, ], @@ -852,11 +852,11 @@ describe('preprocessData', () => { Object { "batchUID": 1, "depth": 0, - "duration": 0.010000000000000002, + "duration": 0.009999999999999998, "lanes": Array [ 4, ], - "timestamp": 0.019, + "timestamp": 0.021, "type": "render-idle", }, Object { @@ -866,37 +866,37 @@ describe('preprocessData', () => { "lanes": Array [ 4, ], - "timestamp": 0.019, + "timestamp": 0.021, "type": "render", }, Object { "batchUID": 1, "depth": 0, - "duration": 0.006000000000000002, + "duration": 0.005999999999999998, "lanes": Array [ 4, ], - "timestamp": 0.023, + "timestamp": 0.025, "type": "commit", }, Object { "batchUID": 1, "depth": 1, - "duration": 0.0010000000000000009, + "duration": 0.0009999999999999974, "lanes": Array [ 4, ], - "timestamp": 0.027, + "timestamp": 0.029, "type": "layout-effects", }, Object { "batchUID": 1, "depth": 0, - "duration": 0.0010000000000000009, + "duration": 0.0030000000000000027, "lanes": Array [ 4, ], - "timestamp": 0.03, + "timestamp": 0.032, "type": "passive-effects", }, ], @@ -906,16 +906,32 @@ describe('preprocessData', () => { "componentName": "App", "duration": 0.001, "timestamp": 0.006, + "type": "render", + "warning": null, + }, + Object { + "componentName": "App", + "duration": 0.0019999999999999983, + "timestamp": 0.017, + "type": "passive-effect-mount", "warning": null, }, Object { "componentName": "App", "duration": 0.0010000000000000009, - "timestamp": 0.02, + "timestamp": 0.022, + "type": "render", + "warning": null, + }, + Object { + "componentName": "App", + "duration": 0.0010000000000000009, + "timestamp": 0.033, + "type": "passive-effect-mount", "warning": null, }, ], - "duration": 0.031, + "duration": 0.035, "flamechart": Array [], "internalModuleSourceToRanges": Map {}, "laneToLabelMap": Map { @@ -1000,7 +1016,7 @@ describe('preprocessData', () => { Object { "batchUID": 0, "depth": 0, - "duration": 0.0019999999999999983, + "duration": 0.004, "lanes": Array [ 4, ], @@ -1010,11 +1026,11 @@ describe('preprocessData', () => { Object { "batchUID": 1, "depth": 0, - "duration": 0.010000000000000002, + "duration": 0.009999999999999998, "lanes": Array [ 4, ], - "timestamp": 0.019, + "timestamp": 0.021, "type": "render-idle", }, Object { @@ -1024,37 +1040,37 @@ describe('preprocessData', () => { "lanes": Array [ 4, ], - "timestamp": 0.019, + "timestamp": 0.021, "type": "render", }, Object { "batchUID": 1, "depth": 0, - "duration": 0.006000000000000002, + "duration": 0.005999999999999998, "lanes": Array [ 4, ], - "timestamp": 0.023, + "timestamp": 0.025, "type": "commit", }, Object { "batchUID": 1, "depth": 1, - "duration": 0.0010000000000000009, + "duration": 0.0009999999999999974, "lanes": Array [ 4, ], - "timestamp": 0.027, + "timestamp": 0.029, "type": "layout-effects", }, Object { "batchUID": 1, "depth": 0, - "duration": 0.0010000000000000009, + "duration": 0.0030000000000000027, "lanes": Array [ 4, ], - "timestamp": 0.03, + "timestamp": 0.032, "type": "passive-effects", }, ], @@ -1108,7 +1124,7 @@ describe('preprocessData', () => { "lanes": Array [ 4, ], - "timestamp": 0.017, + "timestamp": 0.018, "type": "schedule-state-update", "warning": null, }, diff --git a/packages/react-devtools-scheduling-profiler/src/import-worker/preprocessData.js b/packages/react-devtools-scheduling-profiler/src/import-worker/preprocessData.js index 6c41de1e3e279..e6bcfc178d003 100644 --- a/packages/react-devtools-scheduling-profiler/src/import-worker/preprocessData.js +++ b/packages/react-devtools-scheduling-profiler/src/import-worker/preprocessData.js @@ -22,6 +22,7 @@ import type { Phase, ReactLane, ReactComponentMeasure, + ReactComponentMeasureType, ReactMeasure, ReactMeasureType, ReactProfilerData, @@ -484,31 +485,13 @@ function processTimelineEvent( } else if (name.startsWith('--react-lane-labels-')) { const [laneLabelTuplesString] = name.substr(20).split('-'); updateLaneToLabelMap(currentProfilerData, laneLabelTuplesString); - } else if (name.startsWith('--component-render-start-')) { - const [componentName] = name.substr(25).split('-'); - - if (state.currentReactComponentMeasure !== null) { - console.error( - 'Render started while another render in progress:', - state.currentReactComponentMeasure, - ); - } - - state.currentReactComponentMeasure = { - componentName, - timestamp: startTime, - duration: 0, - warning: null, - }; - } else if (name === '--component-render-stop') { - if (state.currentReactComponentMeasure !== null) { - const componentMeasure = state.currentReactComponentMeasure; - componentMeasure.duration = startTime - componentMeasure.timestamp; - - state.currentReactComponentMeasure = null; - - currentProfilerData.componentMeasures.push(componentMeasure); - } + } else if (name.startsWith('--component-')) { + processReactComponentMeasure( + name, + startTime, + currentProfilerData, + state, + ); } else if (name.startsWith('--schedule-render-')) { const [laneBitmaskString] = name.substr(18).split('-'); @@ -868,6 +851,154 @@ function processTimelineEvent( } } +function assertNoOverlappingComponentMeasure(state: ProcessorState) { + if (state.currentReactComponentMeasure !== null) { + console.error( + 'Component measure started while another measure in progress:', + state.currentReactComponentMeasure, + ); + } +} + +function assertCurrentComponentMeasureType( + state: ProcessorState, + type: ReactComponentMeasureType, +): void { + if (state.currentReactComponentMeasure === null) { + console.error( + `Component measure type "${type}" stopped while no measure was in progress`, + ); + } else if (state.currentReactComponentMeasure.type !== type) { + console.error( + `Component measure type "${type}" stopped while type ${state.currentReactComponentMeasure.type} in progress`, + ); + } +} + +function processReactComponentMeasure( + name: string, + startTime: Milliseconds, + currentProfilerData: ReactProfilerData, + state: ProcessorState, +): void { + if (name.startsWith('--component-render-start-')) { + const [componentName] = name.substr(25).split('-'); + + assertNoOverlappingComponentMeasure(state); + + state.currentReactComponentMeasure = { + componentName, + timestamp: startTime, + duration: 0, + type: 'render', + warning: null, + }; + } else if (name === '--component-render-stop') { + assertCurrentComponentMeasureType(state, 'render'); + + if (state.currentReactComponentMeasure !== null) { + const componentMeasure = state.currentReactComponentMeasure; + componentMeasure.duration = startTime - componentMeasure.timestamp; + + state.currentReactComponentMeasure = null; + + currentProfilerData.componentMeasures.push(componentMeasure); + } + } else if (name.startsWith('--component-layout-effect-mount-start-')) { + const [componentName] = name.substr(38).split('-'); + + assertNoOverlappingComponentMeasure(state); + + state.currentReactComponentMeasure = { + componentName, + timestamp: startTime, + duration: 0, + type: 'layout-effect-mount', + warning: null, + }; + } else if (name === '--component-layout-effect-mount-stop') { + assertCurrentComponentMeasureType(state, 'layout-effect-mount'); + + if (state.currentReactComponentMeasure !== null) { + const componentMeasure = state.currentReactComponentMeasure; + componentMeasure.duration = startTime - componentMeasure.timestamp; + + state.currentReactComponentMeasure = null; + + currentProfilerData.componentMeasures.push(componentMeasure); + } + } else if (name.startsWith('--component-layout-effect-unmount-start-')) { + const [componentName] = name.substr(40).split('-'); + + assertNoOverlappingComponentMeasure(state); + + state.currentReactComponentMeasure = { + componentName, + timestamp: startTime, + duration: 0, + type: 'layout-effect-unmount', + warning: null, + }; + } else if (name === '--component-layout-effect-unmount-stop') { + assertCurrentComponentMeasureType(state, 'layout-effect-unmount'); + + if (state.currentReactComponentMeasure !== null) { + const componentMeasure = state.currentReactComponentMeasure; + componentMeasure.duration = startTime - componentMeasure.timestamp; + + state.currentReactComponentMeasure = null; + + currentProfilerData.componentMeasures.push(componentMeasure); + } + } else if (name.startsWith('--component-passive-effect-mount-start-')) { + const [componentName] = name.substr(39).split('-'); + + assertNoOverlappingComponentMeasure(state); + + state.currentReactComponentMeasure = { + componentName, + timestamp: startTime, + duration: 0, + type: 'passive-effect-mount', + warning: null, + }; + } else if (name === '--component-passive-effect-mount-stop') { + assertCurrentComponentMeasureType(state, 'passive-effect-mount'); + + if (state.currentReactComponentMeasure !== null) { + const componentMeasure = state.currentReactComponentMeasure; + componentMeasure.duration = startTime - componentMeasure.timestamp; + + state.currentReactComponentMeasure = null; + + currentProfilerData.componentMeasures.push(componentMeasure); + } + } else if (name.startsWith('--component-passive-effect-unmount-start-')) { + const [componentName] = name.substr(41).split('-'); + + assertNoOverlappingComponentMeasure(state); + + state.currentReactComponentMeasure = { + componentName, + timestamp: startTime, + duration: 0, + type: 'passive-effect-unmount', + warning: null, + }; + } else if (name === '--component-passive-effect-unmount-stop') { + assertCurrentComponentMeasureType(state, 'passive-effect-unmount'); + + if (state.currentReactComponentMeasure !== null) { + const componentMeasure = state.currentReactComponentMeasure; + componentMeasure.duration = startTime - componentMeasure.timestamp; + + state.currentReactComponentMeasure = null; + + currentProfilerData.componentMeasures.push(componentMeasure); + } + } +} + function preprocessFlamechart(rawData: TimelineEvent[]): Flamechart { let parsedData; try { diff --git a/packages/react-devtools-scheduling-profiler/src/types.js b/packages/react-devtools-scheduling-profiler/src/types.js index e5b14e7897ace..cf504229b01c5 100644 --- a/packages/react-devtools-scheduling-profiler/src/types.js +++ b/packages/react-devtools-scheduling-profiler/src/types.js @@ -119,10 +119,18 @@ export type NetworkMeasure = {| url: string, |}; +export type ReactComponentMeasureType = + | 'render' + | 'layout-effect-mount' + | 'layout-effect-unmount' + | 'passive-effect-mount' + | 'passive-effect-unmount'; + export type ReactComponentMeasure = {| +componentName: string, duration: Milliseconds, +timestamp: Milliseconds, + +type: ReactComponentMeasureType, warning: string | null, |}; diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index 95c2f3753bfd7..235bd74361730 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -30,6 +30,7 @@ import { enableProfilerTimer, enableProfilerCommitHooks, enableProfilerNestedUpdatePhase, + enableSchedulingProfiler, enableSuspenseServerRenderer, enableSuspenseCallback, enableScopeAPI, @@ -142,6 +143,16 @@ import { import {didWarnAboutReassigningProps} from './ReactFiberBeginWork.new'; import {doesFiberContain} from './ReactFiberTreeReflection'; import {invokeGuardedCallback, clearCaughtError} from 'shared/ReactErrorUtils'; +import { + markComponentPassiveEffectMountStarted, + markComponentPassiveEffectMountStopped, + markComponentPassiveEffectUnmountStarted, + markComponentPassiveEffectUnmountStopped, + markComponentLayoutEffectMountStarted, + markComponentLayoutEffectMountStopped, + markComponentLayoutEffectUnmountStarted, + markComponentLayoutEffectUnmountStopped, +} from './SchedulingProfiler'; let didWarnAboutUndefinedSnapshotBeforeUpdate: Set | null = null; if (__DEV__) { @@ -512,7 +523,23 @@ function commitHookEffectListUnmount( const destroy = effect.destroy; effect.destroy = undefined; if (destroy !== undefined) { + if (enableSchedulingProfiler) { + if ((flags & HookPassive) !== NoHookEffect) { + markComponentPassiveEffectUnmountStarted(finishedWork); + } else if ((flags & HookLayout) !== NoHookEffect) { + markComponentLayoutEffectUnmountStarted(finishedWork); + } + } + safelyCallDestroy(finishedWork, nearestMountedAncestor, destroy); + + if (enableSchedulingProfiler) { + if ((flags & HookPassive) !== NoHookEffect) { + markComponentPassiveEffectUnmountStopped(); + } else if ((flags & HookLayout) !== NoHookEffect) { + markComponentLayoutEffectUnmountStopped(); + } + } } } effect = effect.next; @@ -520,18 +547,34 @@ function commitHookEffectListUnmount( } } -function commitHookEffectListMount(tag: HookFlags, finishedWork: Fiber) { +function commitHookEffectListMount(flags: HookFlags, finishedWork: Fiber) { const updateQueue: FunctionComponentUpdateQueue | null = (finishedWork.updateQueue: any); const lastEffect = updateQueue !== null ? updateQueue.lastEffect : null; if (lastEffect !== null) { const firstEffect = lastEffect.next; let effect = firstEffect; do { - if ((effect.tag & tag) === tag) { + if ((effect.tag & flags) === flags) { + if (enableSchedulingProfiler) { + if ((flags & HookPassive) !== NoHookEffect) { + markComponentPassiveEffectMountStarted(finishedWork); + } else if ((flags & HookLayout) !== NoHookEffect) { + markComponentLayoutEffectMountStarted(finishedWork); + } + } + // Mount const create = effect.create; effect.destroy = create(); + if (enableSchedulingProfiler) { + if ((flags & HookPassive) !== NoHookEffect) { + markComponentPassiveEffectMountStopped(); + } else if ((flags & HookLayout) !== NoHookEffect) { + markComponentLayoutEffectMountStopped(); + } + } + if (__DEV__) { const destroy = effect.destroy; if (destroy !== undefined && typeof destroy !== 'function') { @@ -1180,10 +1223,13 @@ function commitUnmount( do { const {destroy, tag} = effect; if (destroy !== undefined) { - if ( - (tag & HookInsertion) !== NoHookEffect || - (tag & HookLayout) !== NoHookEffect - ) { + if ((tag & HookInsertion) !== NoHookEffect) { + safelyCallDestroy(current, nearestMountedAncestor, destroy); + } else if ((tag & HookLayout) !== NoHookEffect) { + if (enableSchedulingProfiler) { + markComponentLayoutEffectUnmountStarted(current); + } + if ( enableProfilerTimer && enableProfilerCommitHooks && @@ -1195,6 +1241,10 @@ function commitUnmount( } else { safelyCallDestroy(current, nearestMountedAncestor, destroy); } + + if (enableSchedulingProfiler) { + markComponentLayoutEffectUnmountStopped(); + } } } effect = effect.next; diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index 8057c415e7459..c4da995064d29 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -30,6 +30,7 @@ import { enableProfilerTimer, enableProfilerCommitHooks, enableProfilerNestedUpdatePhase, + enableSchedulingProfiler, enableSuspenseServerRenderer, enableSuspenseCallback, enableScopeAPI, @@ -142,6 +143,16 @@ import { import {didWarnAboutReassigningProps} from './ReactFiberBeginWork.old'; import {doesFiberContain} from './ReactFiberTreeReflection'; import {invokeGuardedCallback, clearCaughtError} from 'shared/ReactErrorUtils'; +import { + markComponentPassiveEffectMountStarted, + markComponentPassiveEffectMountStopped, + markComponentPassiveEffectUnmountStarted, + markComponentPassiveEffectUnmountStopped, + markComponentLayoutEffectMountStarted, + markComponentLayoutEffectMountStopped, + markComponentLayoutEffectUnmountStarted, + markComponentLayoutEffectUnmountStopped, +} from './SchedulingProfiler'; let didWarnAboutUndefinedSnapshotBeforeUpdate: Set | null = null; if (__DEV__) { @@ -512,7 +523,23 @@ function commitHookEffectListUnmount( const destroy = effect.destroy; effect.destroy = undefined; if (destroy !== undefined) { + if (enableSchedulingProfiler) { + if ((flags & HookPassive) !== NoHookEffect) { + markComponentPassiveEffectUnmountStarted(finishedWork); + } else if ((flags & HookLayout) !== NoHookEffect) { + markComponentLayoutEffectUnmountStarted(finishedWork); + } + } + safelyCallDestroy(finishedWork, nearestMountedAncestor, destroy); + + if (enableSchedulingProfiler) { + if ((flags & HookPassive) !== NoHookEffect) { + markComponentPassiveEffectUnmountStopped(); + } else if ((flags & HookLayout) !== NoHookEffect) { + markComponentLayoutEffectUnmountStopped(); + } + } } } effect = effect.next; @@ -520,18 +547,34 @@ function commitHookEffectListUnmount( } } -function commitHookEffectListMount(tag: HookFlags, finishedWork: Fiber) { +function commitHookEffectListMount(flags: HookFlags, finishedWork: Fiber) { const updateQueue: FunctionComponentUpdateQueue | null = (finishedWork.updateQueue: any); const lastEffect = updateQueue !== null ? updateQueue.lastEffect : null; if (lastEffect !== null) { const firstEffect = lastEffect.next; let effect = firstEffect; do { - if ((effect.tag & tag) === tag) { + if ((effect.tag & flags) === flags) { + if (enableSchedulingProfiler) { + if ((flags & HookPassive) !== NoHookEffect) { + markComponentPassiveEffectMountStarted(finishedWork); + } else if ((flags & HookLayout) !== NoHookEffect) { + markComponentLayoutEffectMountStarted(finishedWork); + } + } + // Mount const create = effect.create; effect.destroy = create(); + if (enableSchedulingProfiler) { + if ((flags & HookPassive) !== NoHookEffect) { + markComponentPassiveEffectMountStopped(); + } else if ((flags & HookLayout) !== NoHookEffect) { + markComponentLayoutEffectMountStopped(); + } + } + if (__DEV__) { const destroy = effect.destroy; if (destroy !== undefined && typeof destroy !== 'function') { @@ -1180,10 +1223,13 @@ function commitUnmount( do { const {destroy, tag} = effect; if (destroy !== undefined) { - if ( - (tag & HookInsertion) !== NoHookEffect || - (tag & HookLayout) !== NoHookEffect - ) { + if ((tag & HookInsertion) !== NoHookEffect) { + safelyCallDestroy(current, nearestMountedAncestor, destroy); + } else if ((tag & HookLayout) !== NoHookEffect) { + if (enableSchedulingProfiler) { + markComponentLayoutEffectUnmountStarted(current); + } + if ( enableProfilerTimer && enableProfilerCommitHooks && @@ -1195,6 +1241,10 @@ function commitUnmount( } else { safelyCallDestroy(current, nearestMountedAncestor, destroy); } + + if (enableSchedulingProfiler) { + markComponentLayoutEffectUnmountStopped(); + } } } effect = effect.next; diff --git a/packages/react-reconciler/src/SchedulingProfiler.js b/packages/react-reconciler/src/SchedulingProfiler.js index 4df55137fda97..acfb2c3f976d0 100644 --- a/packages/react-reconciler/src/SchedulingProfiler.js +++ b/packages/react-reconciler/src/SchedulingProfiler.js @@ -161,6 +161,78 @@ export function markComponentRenderStopped(): void { } } +export function markComponentPassiveEffectMountStarted(fiber: Fiber): void { + if (enableSchedulingProfiler) { + if (supportsUserTimingV3) { + const componentName = getComponentNameFromFiber(fiber) || 'Unknown'; + // TODO (scheduling profiler) Add component stack id + markAndClear(`--component-passive-effect-mount-start-${componentName}`); + } + } +} + +export function markComponentPassiveEffectMountStopped(): void { + if (enableSchedulingProfiler) { + if (supportsUserTimingV3) { + markAndClear('--component-passive-effect-mount-stop'); + } + } +} + +export function markComponentPassiveEffectUnmountStarted(fiber: Fiber): void { + if (enableSchedulingProfiler) { + if (supportsUserTimingV3) { + const componentName = getComponentNameFromFiber(fiber) || 'Unknown'; + // TODO (scheduling profiler) Add component stack id + markAndClear(`--component-passive-effect-unmount-start-${componentName}`); + } + } +} + +export function markComponentPassiveEffectUnmountStopped(): void { + if (enableSchedulingProfiler) { + if (supportsUserTimingV3) { + markAndClear('--component-passive-effect-unmount-stop'); + } + } +} + +export function markComponentLayoutEffectMountStarted(fiber: Fiber): void { + if (enableSchedulingProfiler) { + if (supportsUserTimingV3) { + const componentName = getComponentNameFromFiber(fiber) || 'Unknown'; + // TODO (scheduling profiler) Add component stack id + markAndClear(`--component-layout-effect-mount-start-${componentName}`); + } + } +} + +export function markComponentLayoutEffectMountStopped(): void { + if (enableSchedulingProfiler) { + if (supportsUserTimingV3) { + markAndClear('--component-layout-effect-mount-stop'); + } + } +} + +export function markComponentLayoutEffectUnmountStarted(fiber: Fiber): void { + if (enableSchedulingProfiler) { + if (supportsUserTimingV3) { + const componentName = getComponentNameFromFiber(fiber) || 'Unknown'; + // TODO (scheduling profiler) Add component stack id + markAndClear(`--component-layout-effect-unmount-start-${componentName}`); + } + } +} + +export function markComponentLayoutEffectUnmountStopped(): void { + if (enableSchedulingProfiler) { + if (supportsUserTimingV3) { + markAndClear('--component-layout-effect-unmount-stop'); + } + } +} + export function markComponentErrored( fiber: Fiber, thrownValue: mixed, diff --git a/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js b/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js index 5b58342ee0b2f..9b6ec83a40d39 100644 --- a/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js +++ b/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js @@ -608,30 +608,32 @@ describe('SchedulingProfiler', () => { if (gate(flags => flags.enableSchedulingProfiler)) { expect(getMarks()).toMatchInlineSnapshot(` - Array [ - "--render-start-16", - "--component-render-start-Example", - "--component-render-stop", - "--render-stop", - "--commit-start-16", - "--react-version-17.0.3", - "--profiler-version-1", - "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen", - "--layout-effects-start-16", - "--schedule-state-update-1-Example", - "--layout-effects-stop", - "--render-start-1", - "--component-render-start-Example", - "--component-render-stop", - "--render-stop", - "--commit-start-1", - "--react-version-17.0.3", - "--profiler-version-1", - "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen", - "--commit-stop", - "--commit-stop", - ] - `); + Array [ + "--render-start-16", + "--component-render-start-Example", + "--component-render-stop", + "--render-stop", + "--commit-start-16", + "--react-version-17.0.3", + "--profiler-version-1", + "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen", + "--layout-effects-start-16", + "--component-layout-effect-mount-start-Example", + "--schedule-state-update-1-Example", + "--component-layout-effect-mount-stop", + "--layout-effects-stop", + "--render-start-1", + "--component-render-start-Example", + "--component-render-stop", + "--render-stop", + "--commit-start-1", + "--react-version-17.0.3", + "--profiler-version-1", + "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen", + "--commit-stop", + "--commit-stop", + ] + `); } }); @@ -652,33 +654,35 @@ describe('SchedulingProfiler', () => { if (gate(flags => flags.enableSchedulingProfiler)) { expect(getMarks()).toMatchInlineSnapshot(` - Array [ - "--schedule-render-16", - "--render-start-16", - "--component-render-start-Example", - "--component-render-stop", - "--render-stop", - "--commit-start-16", - "--react-version-17.0.3", - "--profiler-version-1", - "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen", - "--layout-effects-start-16", - "--layout-effects-stop", - "--commit-stop", - "--passive-effects-start-16", - "--schedule-state-update-16-Example", - "--passive-effects-stop", - "--render-start-16", - "--component-render-start-Example", - "--component-render-stop", - "--render-stop", - "--commit-start-16", - "--react-version-17.0.3", - "--profiler-version-1", - "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen", - "--commit-stop", - ] - `); + Array [ + "--schedule-render-16", + "--render-start-16", + "--component-render-start-Example", + "--component-render-stop", + "--render-stop", + "--commit-start-16", + "--react-version-17.0.3", + "--profiler-version-1", + "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen", + "--layout-effects-start-16", + "--layout-effects-stop", + "--commit-stop", + "--passive-effects-start-16", + "--component-passive-effect-mount-start-Example", + "--schedule-state-update-16-Example", + "--component-passive-effect-mount-stop", + "--passive-effects-stop", + "--render-start-16", + "--component-render-start-Example", + "--component-render-stop", + "--render-stop", + "--commit-start-16", + "--react-version-17.0.3", + "--profiler-version-1", + "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen", + "--commit-stop", + ] + `); } }); @@ -854,4 +858,141 @@ describe('SchedulingProfiler', () => { `); } }); + + it('should mark passive and layout effects', async () => { + function ComponentWithEffects() { + React.useLayoutEffect(() => { + Scheduler.unstable_yieldValue('layout 1 mount'); + return () => { + Scheduler.unstable_yieldValue('layout 1 unmount'); + }; + }, []); + + React.useEffect(() => { + Scheduler.unstable_yieldValue('passive 1 mount'); + return () => { + Scheduler.unstable_yieldValue('passive 1 unmount'); + }; + }, []); + + React.useLayoutEffect(() => { + Scheduler.unstable_yieldValue('layout 2 mount'); + return () => { + Scheduler.unstable_yieldValue('layout 2 unmount'); + }; + }, []); + + React.useEffect(() => { + Scheduler.unstable_yieldValue('passive 2 mount'); + return () => { + Scheduler.unstable_yieldValue('passive 2 unmount'); + }; + }, []); + + React.useEffect(() => { + Scheduler.unstable_yieldValue('passive 3 mount'); + return () => { + Scheduler.unstable_yieldValue('passive 3 unmount'); + }; + }, []); + + return null; + } + + const renderer = ReactTestRenderer.create(, { + unstable_isConcurrent: true, + }); + + expect(Scheduler).toFlushUntilNextPaint([ + 'layout 1 mount', + 'layout 2 mount', + ]); + + if (gate(flags => flags.enableSchedulingProfiler)) { + expect(getMarks()).toMatchInlineSnapshot(` + Array [ + "--schedule-render-16", + "--render-start-16", + "--component-render-start-ComponentWithEffects", + "--component-render-stop", + "--render-stop", + "--commit-start-16", + "--react-version-17.0.3", + "--profiler-version-1", + "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen", + "--layout-effects-start-16", + "--component-layout-effect-mount-start-ComponentWithEffects", + "--component-layout-effect-mount-stop", + "--component-layout-effect-mount-start-ComponentWithEffects", + "--component-layout-effect-mount-stop", + "--layout-effects-stop", + "--commit-stop", + ] + `); + } + + clearPendingMarks(); + + expect(Scheduler).toFlushAndYield([ + 'passive 1 mount', + 'passive 2 mount', + 'passive 3 mount', + ]); + + if (gate(flags => flags.enableSchedulingProfiler)) { + expect(getMarks()).toMatchInlineSnapshot(` + Array [ + "--passive-effects-start-16", + "--component-passive-effect-mount-start-ComponentWithEffects", + "--component-passive-effect-mount-stop", + "--component-passive-effect-mount-start-ComponentWithEffects", + "--component-passive-effect-mount-stop", + "--component-passive-effect-mount-start-ComponentWithEffects", + "--component-passive-effect-mount-stop", + "--passive-effects-stop", + ] + `); + } + + clearPendingMarks(); + + renderer.unmount(); + + expect(Scheduler).toFlushAndYield([ + 'layout 1 unmount', + 'layout 2 unmount', + 'passive 1 unmount', + 'passive 2 unmount', + 'passive 3 unmount', + ]); + + if (gate(flags => flags.enableSchedulingProfiler)) { + expect(getMarks()).toMatchInlineSnapshot(` + Array [ + "--schedule-render-16", + "--render-start-16", + "--render-stop", + "--commit-start-16", + "--react-version-17.0.3", + "--profiler-version-1", + "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen", + "--component-layout-effect-unmount-start-ComponentWithEffects", + "--component-layout-effect-unmount-stop", + "--component-layout-effect-unmount-start-ComponentWithEffects", + "--component-layout-effect-unmount-stop", + "--layout-effects-start-16", + "--layout-effects-stop", + "--commit-stop", + "--passive-effects-start-16", + "--component-passive-effect-unmount-start-ComponentWithEffects", + "--component-passive-effect-unmount-stop", + "--component-passive-effect-unmount-start-ComponentWithEffects", + "--component-passive-effect-unmount-stop", + "--component-passive-effect-unmount-start-ComponentWithEffects", + "--component-passive-effect-unmount-stop", + "--passive-effects-stop", + ] + `); + } + }); });