Skip to content

Commit

Permalink
Scheduling Profiler: Add marks for component effects (mount and unmou…
Browse files Browse the repository at this point in the history 
…nt) (#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.
  • Loading branch information
Brian Vaughn committed Oct 21, 2021
1 parent 4ba2057 commit 0e8a5af
Show file tree
Hide file tree
Showing 10 changed files with 649 additions and 127 deletions.
21 changes: 19 additions & 2 deletions packages/react-devtools-scheduling-profiler/src/EventTooltip.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
<>
Expand Down
57 changes: 51 additions & 6 deletions packages/react-devtools-scheduling-profiler/src/content-views/ComponentMeasuresView.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -96,16 +102,53 @@ 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) {
context.fillStyle = showHoverHighlight
? 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,
Expand All @@ -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;
}
Expand Down
8 changes: 0 additions & 8 deletions packages/react-devtools-scheduling-profiler/src/content-views/constants.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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: '',
Expand Down Expand Up @@ -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',
),
Expand Down
62 changes: 39 additions & 23 deletions ...-devtools-scheduling-profiler/src/import-worker/__tests__/preprocessData-test.internal.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -840,7 +840,7 @@ describe('preprocessData', () => {
Object {
"batchUID": 0,
"depth": 0,
"duration": 0.0019999999999999983,
"duration": 0.004,
"lanes": Array [
4,
],
Expand All @@ -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 {
Expand All @@ -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",
},
],
Expand All @@ -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 {
Expand Down Expand Up @@ -1000,7 +1016,7 @@ describe('preprocessData', () => {
Object {
"batchUID": 0,
"depth": 0,
"duration": 0.0019999999999999983,
"duration": 0.004,
"lanes": Array [
4,
],
Expand All @@ -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 {
Expand All @@ -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",
},
],
Expand Down Expand Up @@ -1108,7 +1124,7 @@ describe('preprocessData', () => {
"lanes": Array [
4,
],
"timestamp": 0.017,
"timestamp": 0.018,
"type": "schedule-state-update",
"warning": null,
},
Expand Down
Loading

0 comments on commit 0e8a5af

Please sign in to comment.