fix[react-devtools]: record timeline data only when supported (#31154)

Stacked on #31132. See last
commit.

There are 2 issues:
1. We've been recording timeline events, even if Timeline Profiler was
not supported by the Host. We've been doing this for React Native, for
example, which would significantly regress perf of recording a profiling
session, but we were not even using this data.
2. Currently, we are generating component stack for every state update
event. This is extremely expensive, and we should not be doing this.

We can't currently fix the second one, because we would still need to
generate all these stacks, and this would still take quite a lot of
time. As of right now, we can't generate a component stack lazily
without relying on the fact that reference to the Fiber is not stale.
With `enableOwnerStacks` we could populate component stacks in some
collection, which would be cached at the Backend, and then returned only
once Frontend asks for it. This approach also eliminates the need for
keeping a reference to a Fiber.

Author: hoxyq

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;

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 {

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 => {

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<PathFrame> | 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 = {

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<DevToolsHookSettings>],
 };
 
+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],

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';

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');

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;

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(

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);
 }