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.

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

Scheduling profiler: Added lane labels and durations to React measures #22029

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import type {
BatchUID,
Flamechart,
NativeEvent,
Phase,
ReactLane,
ReactComponentMeasure,
ReactMeasureType,
Expand Down Expand Up @@ -315,6 +316,7 @@ function processTimelineEvent(
let warning = null;
if (state.measureStack.find(({type}) => type === 'commit')) {
// TODO (scheduling profiler) Only warn if the subsequent update is longer than some threshold.
// This might be easier to do if we separated warnings into a second pass.
warning = WARNING_STRINGS.NESTED_UPDATE;
}

Expand All @@ -331,6 +333,7 @@ function processTimelineEvent(
let warning = null;
if (state.measureStack.find(({type}) => type === 'commit')) {
// TODO (scheduling profiler) Only warn if the subsequent update is longer than some threshold.
// This might be easier to do if we separated warnings into a second pass.
warning = WARNING_STRINGS.NESTED_UPDATE;
}

Expand All @@ -345,25 +348,18 @@ function processTimelineEvent(

// React Events - suspense
else if (name.startsWith('--suspense-suspend-')) {
const [id, componentName, ...rest] = name.substr(19).split('-');
const [id, componentName, phase, laneBitmaskString] = name
.substr(19)
.split('-');
const lanes = getLanesFromTransportDecimalBitmask(laneBitmaskString);

// Older versions of the scheduling profiler data didn't contain phase or lane values.
let phase = null;
// TODO It's possible we don't have lane-to-label mapping yet (since it's logged during commit phase)
// We may need to do this sort of error checking in a separate pass.
let warning = null;
if (rest.length === 3) {
switch (rest[0]) {
case 'mount':
case 'update':
phase = rest[0];
break;
}

if (phase === 'update') {
const laneLabels = rest[2];
// HACK This is a bit gross but the numeric lane value might change between render versions.
if (!laneLabels.includes('Transition')) {
warning = WARNING_STRINGS.SUSPENDD_DURING_UPATE;
}
if (phase === 'update') {
// HACK This is a bit gross but the numeric lane value might change between render versions.
if (lanes.some(lane => laneToLabelMap.get(lane) === 'Transition')) {
warning = WARNING_STRINGS.SUSPENDD_DURING_UPATE;
bvaughn marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand Down Expand Up @@ -392,7 +388,7 @@ function processTimelineEvent(
depth,
duration: null,
id,
phase,
phase: ((phase: any): Phase),
bvaughn marked this conversation as resolved.
Show resolved Hide resolved
resolution: 'unresolved',
resuspendTimestamps: null,
timestamp: startTime,
Expand Down
4 changes: 3 additions & 1 deletion packages/react-devtools-scheduling-profiler/src/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,14 @@ export type ReactScheduleForceUpdateEvent = {|
+type: 'schedule-force-update',
|};

export type Phase = 'mount' | 'update';

export type SuspenseEvent = {|
...BaseReactEvent,
depth: number,
duration: number | null,
+id: string,
+phase: 'mount' | 'update' | null,
+phase: Phase | null,
resolution: 'rejected' | 'resolved' | 'unresolved',
resuspendTimestamps: Array<number> | null,
+type: 'suspense',
Expand Down