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 7a49d46 commit 0415673
Show file tree
Hide file tree
Showing 11 changed files with 132 additions and 63 deletions.
12 changes: 10 additions & 2 deletions packages/react-devtools-core/src/backend.js
Original file line number Diff line number Diff line change
Expand Up @@ -60,18 +60,26 @@ function debug(methodName: string, ...args: Array<mixed>) {
}
}

type ReactNativeProfilingSettings = {
recordChangeDescriptions: boolean,
};
export function initialize(
maybeSettingsOrSettingsPromise?:
| DevToolsHookSettings
| Promise<DevToolsHookSettings>,
shouldStartProfilingRightNow: boolean = false,
profilingSettings?: ProfilingSettings,
profilingSettings?: ReactNativeProfilingSettings,
) {
const mergedProfilingSettings: ProfilingSettings | void =
typeof profilingSettings === 'object' && profilingSettings != null
? {...profilingSettings, recordTimeline: false}
: undefined;

installHook(
window,
maybeSettingsOrSettingsPromise,
shouldStartProfilingRightNow,
profilingSettings,
mergedProfilingSettings,
);
}

Expand Down
13 changes: 12 additions & 1 deletion packages/react-devtools-extensions/src/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import {
} from 'react-devtools-shared/src/storage';
import {
SESSION_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY,
SESSION_STORAGE_RECORD_TIMELINE_KEY,
SESSION_STORAGE_RELOAD_AND_PROFILE_KEY,
} from 'react-devtools-shared/src/constants';
import type {ProfilingSettings} from 'react-devtools-shared/src/backend/types';
Expand Down Expand Up @@ -46,18 +47,28 @@ export function getProfilingSettings(): ProfilingSettings {
recordChangeDescriptions:
sessionStorageGetItem(SESSION_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY) ===
'true',
recordTimeline:
sessionStorageGetItem(SESSION_STORAGE_RECORD_TIMELINE_KEY) === 'true',
};
}

export function onReloadAndProfile(recordChangeDescriptions: boolean): void {
export function onReloadAndProfile(
recordChangeDescriptions: boolean,
recordTimeline: boolean,
): void {
sessionStorageSetItem(SESSION_STORAGE_RELOAD_AND_PROFILE_KEY, 'true');
sessionStorageSetItem(
SESSION_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY,
recordChangeDescriptions ? 'true' : 'false',
);
sessionStorageSetItem(
SESSION_STORAGE_RECORD_TIMELINE_KEY,
recordTimeline ? 'true' : 'false',
);
}

export function onReloadAndProfileFlagsReset(): void {
sessionStorageRemoveItem(SESSION_STORAGE_RELOAD_AND_PROFILE_KEY);
sessionStorageRemoveItem(SESSION_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY);
sessionStorageRemoveItem(SESSION_STORAGE_RECORD_TIMELINE_KEY);
}
50 changes: 28 additions & 22 deletions packages/react-devtools-shared/src/backend/agent.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,9 @@ export default class Agent extends EventEmitter<{
_persistedSelection: PersistedSelection | null = null;
_persistedSelectionMatch: PathMatch | null = null;
_traceUpdatesEnabled: boolean = false;
_onReloadAndProfile: ((recordChangeDescriptions: boolean) => void) | void;
_onReloadAndProfile:
| ((recordChangeDescriptions: boolean, recordTimeline: boolean) => void)
| void;

constructor(
bridge: BackendBridge,
Expand Down Expand Up @@ -658,17 +660,19 @@ export default class Agent extends EventEmitter<{
this._bridge.send('isReloadAndProfileSupportedByBackend', true);
};

reloadAndProfile: (recordChangeDescriptions: boolean) => void =
recordChangeDescriptions => {
if (typeof this._onReloadAndProfile === 'function') {
this._onReloadAndProfile(recordChangeDescriptions);
}
reloadAndProfile: ({
recordChangeDescriptions: boolean,
recordTimeline: boolean,
}) => void = ({recordChangeDescriptions, recordTimeline}) => {
if (typeof this._onReloadAndProfile === 'function') {
this._onReloadAndProfile(recordChangeDescriptions, recordTimeline);
}

// This code path should only be hit if the shell has explicitly told the Store that it supports profiling.
// In that case, the shell must also listen for this specific message to know when it needs to reload the app.
// The agent can't do this in a way that is renderer agnostic.
this._bridge.send('reloadAppForProfiling');
};
// This code path should only be hit if the shell has explicitly told the Store that it supports profiling.
// In that case, the shell must also listen for this specific message to know when it needs to reload the app.
// The agent can't do this in a way that is renderer agnostic.
this._bridge.send('reloadAppForProfiling');
};

renamePath: RenamePathParams => void = ({
hookID,
Expand Down Expand Up @@ -737,17 +741,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
18 changes: 14 additions & 4 deletions packages/react-devtools-shared/src/backend/fiber/renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -5035,6 +5035,7 @@ export function attach(
let isProfiling: boolean = false;
let profilingStartTime: number = 0;
let recordChangeDescriptions: boolean = false;
let recordTimeline: boolean = false;
let rootToCommitProfilingMetadataMap: CommitProfilingMetadataMap | null =
null;

Expand Down Expand Up @@ -5176,12 +5177,16 @@ export function attach(
}
}

function startProfiling(shouldRecordChangeDescriptions: boolean) {
function startProfiling(
shouldRecordChangeDescriptions: boolean,
shouldRecordTimeline: boolean,
) {
if (isProfiling) {
return;
}

recordChangeDescriptions = shouldRecordChangeDescriptions;
recordTimeline = shouldRecordTimeline;

// Capture initial values as of the time profiling starts.
// It's important we snapshot both the durations and the id-to-root map,
Expand Down Expand Up @@ -5212,7 +5217,7 @@ export function attach(
rootToCommitProfilingMetadataMap = new Map();

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

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

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

recordTimeline = false;
}

// 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
8 changes: 6 additions & 2 deletions packages/react-devtools-shared/src/bridge.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import type {
ProfilingDataBackend,
RendererID,
DevToolsHookSettings,
ProfilingSettings,
} from 'react-devtools-shared/src/backend/types';
import type {StyleAndLayout as StyleAndLayoutPayload} from 'react-devtools-shared/src/backend/NativeStyleEditor/types';

Expand Down Expand Up @@ -206,6 +207,9 @@ export type BackendEvents = {
hookSettings: [$ReadOnly<DevToolsHookSettings>],
};

type StartProfilingParams = ProfilingSettings;
type ReloadAndProfilingParams = ProfilingSettings;

type FrontendEvents = {
clearErrorsAndWarnings: [{rendererID: RendererID}],
clearErrorsForElementID: [ElementAndRendererID],
Expand All @@ -226,13 +230,13 @@ type FrontendEvents = {
overrideSuspense: [OverrideSuspense],
overrideValueAtPath: [OverrideValueAtPath],
profilingData: [ProfilingDataBackend],
reloadAndProfile: [boolean],
reloadAndProfile: [ReloadAndProfilingParams],
renamePath: [RenamePath],
savedPreferences: [SavedPreferencesParams],
setTraceUpdatesEnabled: [boolean],
shutdown: [],
startInspectingHost: [],
startProfiling: [boolean],
startProfiling: [StartProfilingParams],
stopInspectingHost: [boolean],
stopProfiling: [],
storeAsGlobal: [StoreAsGlobalParams],
Expand Down
2 changes: 2 additions & 0 deletions packages/react-devtools-shared/src/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ export const LOCAL_STORAGE_PARSE_HOOK_NAMES_KEY =
'React::DevTools::parseHookNames';
export const SESSION_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY =
'React::DevTools::recordChangeDescriptions';
export const SESSION_STORAGE_RECORD_TIMELINE_KEY =
'React::DevTools::recordTimeline';
export const SESSION_STORAGE_RELOAD_AND_PROFILE_KEY =
'React::DevTools::reloadAndProfile';
export const LOCAL_STORAGE_BROWSER_THEME = 'React::DevTools::theme';
Expand Down
5 changes: 4 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,10 @@ export default class ProfilerStore extends EventEmitter<{
}

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

this._isProfilingBasedOnUserInput = true;
this.emit('isProfiling');
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,8 +54,11 @@ export default function ReloadAndProfileButton({
// For now, let's just skip doing it entirely to avoid paying snapshot costs for data we don't need.
// startProfiling();

bridge.send('reloadAndProfile', recordChangeDescriptions);
}, [bridge, recordChangeDescriptions]);
bridge.send('reloadAndProfile', {
recordChangeDescriptions,
recordTimeline: store.supportsTimeline,
});
}, [bridge, recordChangeDescriptions, store]);

if (!supportsReloadAndProfile) {
return null;
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 0415673

Please sign in to comment.