Skip to content

Commit

Permalink
fix[react-devtools]: record timeline data only when supported
Browse files Browse the repository at this point in the history
  • Loading branch information
hoxyq committed Oct 8, 2024
1 parent b4b3148 commit a2751c2
Show file tree
Hide file tree
Showing 7 changed files with 80 additions and 46 deletions.
24 changes: 13 additions & 11 deletions packages/react-devtools-shared/src/backend/agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -737,17 +737,19 @@ export default class Agent extends EventEmitter<{
this.emit('shutdown');
};

startProfiling: (recordChangeDescriptions: boolean) => void =
recordChangeDescriptions => {
this._isProfiling = true;
for (const rendererID in this._rendererInterfaces) {
const renderer = ((this._rendererInterfaces[
(rendererID: any)
]: any): RendererInterface);
renderer.startProfiling(recordChangeDescriptions);
}
this._bridge.send('profilingStatus', this._isProfiling);
};
startProfiling: (
recordChangeDescriptions: boolean,
recordTimeline: boolean,
) => void = (recordChangeDescriptions, recordTimeline) => {
this._isProfiling = true;
for (const rendererID in this._rendererInterfaces) {
const renderer = ((this._rendererInterfaces[
(rendererID: any)
]: any): RendererInterface);
renderer.startProfiling(recordChangeDescriptions, recordTimeline);
}
this._bridge.send('profilingStatus', this._isProfiling);
};

stopProfiling: () => void = () => {
this._isProfiling = false;
Expand Down
14 changes: 10 additions & 4 deletions packages/react-devtools-shared/src/backend/fiber/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -5176,7 +5176,10 @@ export function attach(
}
}

function startProfiling(shouldRecordChangeDescriptions: boolean) {
function startProfiling(
shouldRecordChangeDescriptions: boolean,
recordTimeline: boolean,
) {
if (isProfiling) {
return;
}
Expand Down Expand Up @@ -5212,7 +5215,7 @@ export function attach(
rootToCommitProfilingMetadataMap = new Map();

if (toggleProfilingStatus !== null) {
toggleProfilingStatus(true);
toggleProfilingStatus(true, profilingSettings.recordTimeline);
}
}

Expand All @@ -5221,13 +5224,16 @@ export function attach(
recordChangeDescriptions = false;

if (toggleProfilingStatus !== null) {
toggleProfilingStatus(false);
toggleProfilingStatus(false, profilingSettings.recordTimeline);
}
}

// Automatically start profiling so that we don't miss timing info from initial "mount".
if (shouldStartProfilingRightNow) {
startProfiling(profilingSettings.recordChangeDescriptions);
startProfiling(
profilingSettings.recordChangeDescriptions,
profilingSettings.recordTimeline,
);
}

function getNearestFiber(devtoolsInstance: DevToolsInstance): null | Fiber {
Expand Down
73 changes: 45 additions & 28 deletions packages/react-devtools-shared/src/backend/profilingHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,10 @@ export function setPerformanceMock_ONLY_FOR_TESTING(
}

export type GetTimelineData = () => TimelineData | null;
export type ToggleProfilingStatus = (value: boolean) => void;
export type ToggleProfilingStatus = (
value: boolean,
recordTimeline?: boolean,
) => void;

type Response = {
getTimelineData: GetTimelineData,
Expand Down Expand Up @@ -839,7 +842,10 @@ export function createProfilingHooks({
}
}

function toggleProfilingStatus(value: boolean) {
function toggleProfilingStatus(
value: boolean,
recordTimeline: boolean = false,
) {
if (isProfiling !== value) {
isProfiling = value;

Expand Down Expand Up @@ -875,34 +881,45 @@ export function createProfilingHooks({
currentReactComponentMeasure = null;
currentReactMeasuresStack = [];
currentFiberStacks = new Map();
currentTimelineData = {
// Session wide metadata; only collected once.
internalModuleSourceToRanges,
laneToLabelMap: laneToLabelMap || new Map(),
reactVersion,

// Data logged by React during profiling session.
componentMeasures: [],
schedulingEvents: [],
suspenseEvents: [],
thrownErrors: [],

// Data inferred based on what React logs.
batchUIDToMeasuresMap: new Map(),
duration: 0,
laneToReactMeasureMap,
startTime: 0,

// Data only available in Chrome profiles.
flamechart: [],
nativeEvents: [],
networkMeasures: [],
otherUserTimingMarks: [],
snapshots: [],
snapshotHeight: 0,
};
if (recordTimeline) {
currentTimelineData = {
// Session wide metadata; only collected once.
internalModuleSourceToRanges,
laneToLabelMap: laneToLabelMap || new Map(),
reactVersion,

// Data logged by React during profiling session.
componentMeasures: [],
schedulingEvents: [],
suspenseEvents: [],
thrownErrors: [],

// Data inferred based on what React logs.
batchUIDToMeasuresMap: new Map(),
duration: 0,
laneToReactMeasureMap,
startTime: 0,

// Data only available in Chrome profiles.
flamechart: [],
nativeEvents: [],
networkMeasures: [],
otherUserTimingMarks: [],
snapshots: [],
snapshotHeight: 0,
};
}
nextRenderShouldStartNewBatch = true;
} else {
// This is __EXPENSIVE__.
// We could end up with hundreds of state updated, and for each one of them
// would try to create a component stack with possibly hundreds of Fibers.
// Creating a cache of component stacks won't help, generating a single stack is already expensive enough.
// We should find a way to lazily generate component stacks on demand, when user inspects a specific event.
// If we succeed with moving React DevTools Timeline Profiler to Performance panel, then Timeline Profiler would probably be removed.
// If not, then once enableOwnerStacks is adopted, revisit this again and cache component stacks per Fiber,
// but only return them when needed, sending hundreds of component stacks is beyond the Bridge's bandwidth.

// Postprocess Profile data
if (currentTimelineData !== null) {
currentTimelineData.schedulingEvents.forEach(event => {
Expand Down
6 changes: 5 additions & 1 deletion packages/react-devtools-shared/src/backend/types.js
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,10 @@ export type RendererInterface = {
renderer: ReactRenderer | null,
setTraceUpdatesEnabled: (enabled: boolean) => void,
setTrackedPath: (path: Array<PathFrame> | null) => void,
startProfiling: (recordChangeDescriptions: boolean) => void,
startProfiling: (
recordChangeDescriptions: boolean,
recordTimeline: boolean,
) => void,
stopProfiling: () => void,
storeAsGlobal: (
id: number,
Expand Down Expand Up @@ -487,6 +490,7 @@ export type DevToolsBackend = {

export type ProfilingSettings = {
recordChangeDescriptions: boolean,
recordTimeline: boolean,
};

export type DevToolsHook = {
Expand Down
2 changes: 1 addition & 1 deletion packages/react-devtools-shared/src/bridge.js
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ type FrontendEvents = {
setTraceUpdatesEnabled: [boolean],
shutdown: [],
startInspectingHost: [],
startProfiling: [boolean],
startProfiling: [boolean, boolean],
stopInspectingHost: [boolean],
stopProfiling: [],
storeAsGlobal: [StoreAsGlobalParams],
Expand Down
6 changes: 5 additions & 1 deletion packages/react-devtools-shared/src/devtools/ProfilerStore.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,11 @@ export default class ProfilerStore extends EventEmitter<{
}

startProfiling(): void {
this._bridge.send('startProfiling', this._store.recordChangeDescriptions);
this._bridge.send(
'startProfiling',
this._store.recordChangeDescriptions,
this._store.supportsTimeline,
);

this._isProfilingBasedOnUserInput = true;
this.emit('isProfiling');
Expand Down
1 change: 1 addition & 0 deletions packages/react-devtools-shared/src/hook.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ const targetConsole: Object = console;

const defaultProfilingSettings: ProfilingSettings = {
recordChangeDescriptions: false,
recordTimeline: false,
};

export function installHook(
Expand Down

0 comments on commit a2751c2

Please sign in to comment.