Skip to content

Commit 0e8a5af

Browse files
author
Brian Vaughn
authored
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.
1 parent 4ba2057 commit 0e8a5af

File tree

10 files changed

+649
-127
lines changed

10 files changed

+649
-127
lines changed

‎packages/react-devtools-scheduling-profiler/src/EventTooltip.js

Lines changed: 19 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -140,9 +140,26 @@ const TooltipReactComponentMeasure = ({
140140
}: {|
141141
componentMeasure: ReactComponentMeasure,
142142
|}) => {
143-
const {componentName, duration, timestamp, warning} = componentMeasure;
143+
const {componentName, duration, timestamp, type, warning} = componentMeasure;
144144

145-
const label = `${componentName} rendered`;
145+
let label = componentName;
146+
switch (type) {
147+
case 'render':
148+
label += ' rendered';
149+
break;
150+
case 'layout-effect-mount':
151+
label += ' mounted layout effect';
152+
break;
153+
case 'layout-effect-unmount':
154+
label += ' unmounted layout effect';
155+
break;
156+
case 'passive-effect-mount':
157+
label += ' mounted passive effect';
158+
break;
159+
case 'passive-effect-unmount':
160+
label += ' unmounted passive effect';
161+
break;
162+
}
146163

147164
return (
148165
<>

‎packages/react-devtools-scheduling-profiler/src/content-views/ComponentMeasuresView.js

Lines changed: 51 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,13 @@ export class ComponentMeasuresView extends View {
7676
showHoverHighlight: boolean,
7777
): boolean {
7878
const {frame} = this;
79-
const {componentName, duration, timestamp, warning} = componentMeasure;
79+
const {
80+
componentName,
81+
duration,
82+
timestamp,
83+
type,
84+
warning,
85+
} = componentMeasure;
8086

8187
const xStart = timestampToPosition(timestamp, scaleFactor, frame);
8288
const xStop = timestampToPosition(timestamp + duration, scaleFactor, frame);
@@ -96,16 +102,53 @@ export class ComponentMeasuresView extends View {
96102
return false; // Too small to render at this zoom level
97103
}
98104

105+
let textFillStyle = ((null: any): string);
106+
let typeLabel = ((null: any): string);
107+
99108
const drawableRect = intersectionOfRects(componentMeasureRect, rect);
100109
context.beginPath();
101110
if (warning !== null) {
102111
context.fillStyle = showHoverHighlight
103112
? COLORS.WARNING_BACKGROUND_HOVER
104113
: COLORS.WARNING_BACKGROUND;
105114
} else {
106-
context.fillStyle = showHoverHighlight
107-
? COLORS.REACT_COMPONENT_MEASURE_HOVER
108-
: COLORS.REACT_COMPONENT_MEASURE;
115+
switch (type) {
116+
case 'render':
117+
context.fillStyle = showHoverHighlight
118+
? COLORS.REACT_RENDER_HOVER
119+
: COLORS.REACT_RENDER;
120+
textFillStyle = COLORS.REACT_RENDER_TEXT;
121+
typeLabel = 'rendered';
122+
break;
123+
case 'layout-effect-mount':
124+
context.fillStyle = showHoverHighlight
125+
? COLORS.REACT_LAYOUT_EFFECTS_HOVER
126+
: COLORS.REACT_LAYOUT_EFFECTS;
127+
textFillStyle = COLORS.REACT_LAYOUT_EFFECTS_TEXT;
128+
typeLabel = 'mounted layout effect';
129+
break;
130+
case 'layout-effect-unmount':
131+
context.fillStyle = showHoverHighlight
132+
? COLORS.REACT_LAYOUT_EFFECTS_HOVER
133+
: COLORS.REACT_LAYOUT_EFFECTS;
134+
textFillStyle = COLORS.REACT_LAYOUT_EFFECTS_TEXT;
135+
typeLabel = 'unmounted layout effect';
136+
break;
137+
case 'passive-effect-mount':
138+
context.fillStyle = showHoverHighlight
139+
? COLORS.REACT_PASSIVE_EFFECTS_HOVER
140+
: COLORS.REACT_PASSIVE_EFFECTS;
141+
textFillStyle = COLORS.REACT_PASSIVE_EFFECTS_TEXT;
142+
typeLabel = 'mounted passive effect';
143+
break;
144+
case 'passive-effect-unmount':
145+
context.fillStyle = showHoverHighlight
146+
? COLORS.REACT_PASSIVE_EFFECTS_HOVER
147+
: COLORS.REACT_PASSIVE_EFFECTS;
148+
textFillStyle = COLORS.REACT_PASSIVE_EFFECTS_TEXT;
149+
typeLabel = 'unmounted passive effect';
150+
break;
151+
}
109152
}
110153
context.fillRect(
111154
drawableRect.origin.x,
@@ -114,9 +157,11 @@ export class ComponentMeasuresView extends View {
114157
drawableRect.size.height,
115158
);
116159

117-
const label = `${componentName} rendered - ${formatDuration(duration)}`;
160+
const label = `${componentName} ${typeLabel} - ${formatDuration(duration)}`;
118161

119-
drawText(label, context, componentMeasureRect, drawableRect);
162+
drawText(label, context, componentMeasureRect, drawableRect, {
163+
fillStyle: textFillStyle,
164+
});
120165

121166
return true;
122167
}

‎packages/react-devtools-scheduling-profiler/src/content-views/constants.js

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -59,8 +59,6 @@ export let COLORS = {
5959
PRIORITY_LABEL: '',
6060
USER_TIMING: '',
6161
USER_TIMING_HOVER: '',
62-
REACT_COMPONENT_MEASURE: '',
63-
REACT_COMPONENT_MEASURE_HOVER: '',
6462
REACT_IDLE: '',
6563
REACT_IDLE_HOVER: '',
6664
REACT_RENDER: '',
@@ -150,12 +148,6 @@ export function updateColorsToMatchTheme(element: Element): boolean {
150148
USER_TIMING_HOVER: computedStyle.getPropertyValue(
151149
'--color-scheduling-profiler-user-timing-hover',
152150
),
153-
REACT_COMPONENT_MEASURE: computedStyle.getPropertyValue(
154-
'--color-scheduling-profiler-react-render',
155-
),
156-
REACT_COMPONENT_MEASURE_HOVER: computedStyle.getPropertyValue(
157-
'--color-scheduling-profiler-react-render-hover',
158-
),
159151
REACT_IDLE: computedStyle.getPropertyValue(
160152
'--color-scheduling-profiler-react-idle',
161153
),

‎packages/react-devtools-scheduling-profiler/src/import-worker/__tests__/preprocessData-test.internal.js

Lines changed: 39 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -840,7 +840,7 @@ describe('preprocessData', () => {
840840
Object {
841841
"batchUID": 0,
842842
"depth": 0,
843-
"duration": 0.0019999999999999983,
843+
"duration": 0.004,
844844
"lanes": Array [
845845
4,
846846
],
@@ -852,11 +852,11 @@ describe('preprocessData', () => {
852852
Object {
853853
"batchUID": 1,
854854
"depth": 0,
855-
"duration": 0.010000000000000002,
855+
"duration": 0.009999999999999998,
856856
"lanes": Array [
857857
4,
858858
],
859-
"timestamp": 0.019,
859+
"timestamp": 0.021,
860860
"type": "render-idle",
861861
},
862862
Object {
@@ -866,37 +866,37 @@ describe('preprocessData', () => {
866866
"lanes": Array [
867867
4,
868868
],
869-
"timestamp": 0.019,
869+
"timestamp": 0.021,
870870
"type": "render",
871871
},
872872
Object {
873873
"batchUID": 1,
874874
"depth": 0,
875-
"duration": 0.006000000000000002,
875+
"duration": 0.005999999999999998,
876876
"lanes": Array [
877877
4,
878878
],
879-
"timestamp": 0.023,
879+
"timestamp": 0.025,
880880
"type": "commit",
881881
},
882882
Object {
883883
"batchUID": 1,
884884
"depth": 1,
885-
"duration": 0.0010000000000000009,
885+
"duration": 0.0009999999999999974,
886886
"lanes": Array [
887887
4,
888888
],
889-
"timestamp": 0.027,
889+
"timestamp": 0.029,
890890
"type": "layout-effects",
891891
},
892892
Object {
893893
"batchUID": 1,
894894
"depth": 0,
895-
"duration": 0.0010000000000000009,
895+
"duration": 0.0030000000000000027,
896896
"lanes": Array [
897897
4,
898898
],
899-
"timestamp": 0.03,
899+
"timestamp": 0.032,
900900
"type": "passive-effects",
901901
},
902902
],
@@ -906,16 +906,32 @@ describe('preprocessData', () => {
906906
"componentName": "App",
907907
"duration": 0.001,
908908
"timestamp": 0.006,
909+
"type": "render",
910+
"warning": null,
911+
},
912+
Object {
913+
"componentName": "App",
914+
"duration": 0.0019999999999999983,
915+
"timestamp": 0.017,
916+
"type": "passive-effect-mount",
909917
"warning": null,
910918
},
911919
Object {
912920
"componentName": "App",
913921
"duration": 0.0010000000000000009,
914-
"timestamp": 0.02,
922+
"timestamp": 0.022,
923+
"type": "render",
924+
"warning": null,
925+
},
926+
Object {
927+
"componentName": "App",
928+
"duration": 0.0010000000000000009,
929+
"timestamp": 0.033,
930+
"type": "passive-effect-mount",
915931
"warning": null,
916932
},
917933
],
918-
"duration": 0.031,
934+
"duration": 0.035,
919935
"flamechart": Array [],
920936
"internalModuleSourceToRanges": Map {},
921937
"laneToLabelMap": Map {
@@ -1000,7 +1016,7 @@ describe('preprocessData', () => {
10001016
Object {
10011017
"batchUID": 0,
10021018
"depth": 0,
1003-
"duration": 0.0019999999999999983,
1019+
"duration": 0.004,
10041020
"lanes": Array [
10051021
4,
10061022
],
@@ -1010,11 +1026,11 @@ describe('preprocessData', () => {
10101026
Object {
10111027
"batchUID": 1,
10121028
"depth": 0,
1013-
"duration": 0.010000000000000002,
1029+
"duration": 0.009999999999999998,
10141030
"lanes": Array [
10151031
4,
10161032
],
1017-
"timestamp": 0.019,
1033+
"timestamp": 0.021,
10181034
"type": "render-idle",
10191035
},
10201036
Object {
@@ -1024,37 +1040,37 @@ describe('preprocessData', () => {
10241040
"lanes": Array [
10251041
4,
10261042
],
1027-
"timestamp": 0.019,
1043+
"timestamp": 0.021,
10281044
"type": "render",
10291045
},
10301046
Object {
10311047
"batchUID": 1,
10321048
"depth": 0,
1033-
"duration": 0.006000000000000002,
1049+
"duration": 0.005999999999999998,
10341050
"lanes": Array [
10351051
4,
10361052
],
1037-
"timestamp": 0.023,
1053+
"timestamp": 0.025,
10381054
"type": "commit",
10391055
},
10401056
Object {
10411057
"batchUID": 1,
10421058
"depth": 1,
1043-
"duration": 0.0010000000000000009,
1059+
"duration": 0.0009999999999999974,
10441060
"lanes": Array [
10451061
4,
10461062
],
1047-
"timestamp": 0.027,
1063+
"timestamp": 0.029,
10481064
"type": "layout-effects",
10491065
},
10501066
Object {
10511067
"batchUID": 1,
10521068
"depth": 0,
1053-
"duration": 0.0010000000000000009,
1069+
"duration": 0.0030000000000000027,
10541070
"lanes": Array [
10551071
4,
10561072
],
1057-
"timestamp": 0.03,
1073+
"timestamp": 0.032,
10581074
"type": "passive-effects",
10591075
},
10601076
],
@@ -1108,7 +1124,7 @@ describe('preprocessData', () => {
11081124
"lanes": Array [
11091125
4,
11101126
],
1111-
"timestamp": 0.017,
1127+
"timestamp": 0.018,
11121128
"type": "schedule-state-update",
11131129
"warning": null,
11141130
},

0 commit comments

Comments
 (0)