diff --git a/packages/react-devtools-shared/src/backend/agent.js b/packages/react-devtools-shared/src/backend/agent.js index 12704899ecbdc..e05abf51aba46 100644 --- a/packages/react-devtools-shared/src/backend/agent.js +++ b/packages/react-devtools-shared/src/backend/agent.js @@ -153,12 +153,17 @@ 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, isProfiling: boolean = false, - onReloadAndProfile?: (recordChangeDescriptions: boolean) => void, + onReloadAndProfile?: ( + recordChangeDescriptions: boolean, + recordTimeline: boolean, + ) => void, ) { super(); @@ -658,17 +663,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, @@ -740,17 +747,19 @@ export default class Agent extends EventEmitter<{ this.removeAllListeners(); }; - 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; diff --git a/packages/react-devtools-shared/src/backend/fiber/renderer.js b/packages/react-devtools-shared/src/backend/fiber/renderer.js index 57aca225b8d1f..d582e2a3ced96 100644 --- a/packages/react-devtools-shared/src/backend/fiber/renderer.js +++ b/packages/react-devtools-shared/src/backend/fiber/renderer.js @@ -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; @@ -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, @@ -5212,7 +5217,7 @@ export function attach( rootToCommitProfilingMetadataMap = new Map(); if (toggleProfilingStatus !== null) { - toggleProfilingStatus(true); + toggleProfilingStatus(true, recordTimeline); } } @@ -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 (shouldStartProfilingNow) { - startProfiling(profilingSettings.recordChangeDescriptions); + startProfiling( + profilingSettings.recordChangeDescriptions, + profilingSettings.recordTimeline, + ); } function getNearestFiber(devtoolsInstance: DevToolsInstance): null | Fiber { diff --git a/packages/react-devtools-shared/src/backend/profilingHooks.js b/packages/react-devtools-shared/src/backend/profilingHooks.js index 47a01035308e2..a8111713c9fb2 100644 --- a/packages/react-devtools-shared/src/backend/profilingHooks.js +++ b/packages/react-devtools-shared/src/backend/profilingHooks.js @@ -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, @@ -839,7 +842,10 @@ export function createProfilingHooks({ } } - function toggleProfilingStatus(value: boolean) { + function toggleProfilingStatus( + value: boolean, + recordTimeline: boolean = false, + ) { if (isProfiling !== value) { isProfiling = value; @@ -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 => { diff --git a/packages/react-devtools-shared/src/backend/types.js b/packages/react-devtools-shared/src/backend/types.js index 61546f2c289a6..9d9d9a8eb5e65 100644 --- a/packages/react-devtools-shared/src/backend/types.js +++ b/packages/react-devtools-shared/src/backend/types.js @@ -419,7 +419,10 @@ export type RendererInterface = { renderer: ReactRenderer | null, setTraceUpdatesEnabled: (enabled: boolean) => void, setTrackedPath: (path: Array | null) => void, - startProfiling: (recordChangeDescriptions: boolean) => void, + startProfiling: ( + recordChangeDescriptions: boolean, + recordTimeline: boolean, + ) => void, stopProfiling: () => void, storeAsGlobal: ( id: number, @@ -487,6 +490,7 @@ export type DevToolsBackend = { export type ProfilingSettings = { recordChangeDescriptions: boolean, + recordTimeline: boolean, }; export type DevToolsHook = { diff --git a/packages/react-devtools-shared/src/bridge.js b/packages/react-devtools-shared/src/bridge.js index dde6e7c3ffff8..cb494e1b3c1ba 100644 --- a/packages/react-devtools-shared/src/bridge.js +++ b/packages/react-devtools-shared/src/bridge.js @@ -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'; @@ -206,6 +207,9 @@ export type BackendEvents = { hookSettings: [$ReadOnly], }; +type StartProfilingParams = ProfilingSettings; +type ReloadAndProfilingParams = ProfilingSettings; + type FrontendEvents = { clearErrorsAndWarnings: [{rendererID: RendererID}], clearErrorsForElementID: [ElementAndRendererID], @@ -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], diff --git a/packages/react-devtools-shared/src/constants.js b/packages/react-devtools-shared/src/constants.js index 6893610b46d8f..b08738165906c 100644 --- a/packages/react-devtools-shared/src/constants.js +++ b/packages/react-devtools-shared/src/constants.js @@ -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'; diff --git a/packages/react-devtools-shared/src/devtools/ProfilerStore.js b/packages/react-devtools-shared/src/devtools/ProfilerStore.js index 5ed7e69f294b9..b9bbdb11df539 100644 --- a/packages/react-devtools-shared/src/devtools/ProfilerStore.js +++ b/packages/react-devtools-shared/src/devtools/ProfilerStore.js @@ -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'); diff --git a/packages/react-devtools-shared/src/devtools/views/Profiler/ReloadAndProfileButton.js b/packages/react-devtools-shared/src/devtools/views/Profiler/ReloadAndProfileButton.js index 90dba6f7d8359..95ac82a97323d 100644 --- a/packages/react-devtools-shared/src/devtools/views/Profiler/ReloadAndProfileButton.js +++ b/packages/react-devtools-shared/src/devtools/views/Profiler/ReloadAndProfileButton.js @@ -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; diff --git a/packages/react-devtools-shared/src/hook.js b/packages/react-devtools-shared/src/hook.js index 4aa6518cd1f4b..d754140f96541 100644 --- a/packages/react-devtools-shared/src/hook.js +++ b/packages/react-devtools-shared/src/hook.js @@ -52,6 +52,7 @@ const targetConsole: Object = console; const defaultProfilingSettings: ProfilingSettings = { recordChangeDescriptions: false, + recordTimeline: false, }; export function installHook( diff --git a/packages/react-devtools-shared/src/utils.js b/packages/react-devtools-shared/src/utils.js index d3f18920fce1f..bbf1599898d8a 100644 --- a/packages/react-devtools-shared/src/utils.js +++ b/packages/react-devtools-shared/src/utils.js @@ -38,6 +38,7 @@ import { LOCAL_STORAGE_OPEN_IN_EDITOR_URL, SESSION_STORAGE_RELOAD_AND_PROFILE_KEY, SESSION_STORAGE_RECORD_CHANGE_DESCRIPTIONS_KEY, + SESSION_STORAGE_RECORD_TIMELINE_KEY, } from './constants'; import { ComponentFilterElementType, @@ -1002,18 +1003,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); }