RPP: Skip buildProfileCalls in SamplesHandler if it's an actual trace

Also clean up some trace vs cpuprofile tech debt

Bug:40278532
Change-Id: I1020fd61d1ff17f1cb24bbb92e94d066a25c0b75
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6005888
Auto-Submit: Paul Irish <[email protected]>
Reviewed-by: Andres Olivares <[email protected]>
Commit-Queue: Paul Irish <[email protected]>

Author: paulirish

front_end/models/trace/Processor.ts

@@ -49,21 +49,6 @@ declare global {
   }
 }
 
-export interface ParseOptions {
-  /**
-   * If the trace was just recorded on the current page, rather than an imported file.
-   * TODO(paulirish): Maybe remove. This is currently unused by the Processor and Handlers
-   * @default false
-   */
-  isFreshRecording?: boolean;
-  /**
-   * If the trace is a CPU Profile rather than a Chrome tracing trace.
-   * @default false
-   */
-  isCPUProfile?: boolean;
-  metadata?: Types.File.MetaData;
-}
-
 export class TraceProcessor extends EventTarget {
   // We force the Meta handler to be enabled, so the TraceHandlers type here is
   // the model handlers the user passes in and the Meta handler.
@@ -166,13 +151,13 @@ export class TraceProcessor extends EventTarget {
     this.#status = Status.IDLE;
   }
 
-  async parse(traceEvents: readonly Types.Events.Event[], options: ParseOptions): Promise<void> {
+  async parse(traceEvents: readonly Types.Events.Event[], options: Types.Configuration.ParseOptions): Promise<void> {
     if (this.#status !== Status.IDLE) {
       throw new Error(`Trace processor can't start parsing when not idle. Current state: ${this.#status}`);
     }
     try {
       this.#status = Status.PARSING;
-      await this.#computeParsedTrace(traceEvents);
+      await this.#computeParsedTrace(traceEvents, options);
       if (this.#data && !options.isCPUProfile) {  // We do not calculate insights for CPU Profiles.
         this.#computeInsights(this.#data, traceEvents, options);
       }
@@ -186,7 +171,8 @@ export class TraceProcessor extends EventTarget {
   /**
    * Run all the handlers and set the result to `#data`.
    */
-  async #computeParsedTrace(traceEvents: readonly Types.Events.Event[]): Promise<void> {
+  async #computeParsedTrace(traceEvents: readonly Types.Events.Event[], options: Types.Configuration.ParseOptions):
+      Promise<void> {
     /**
      * We want to yield regularly to maintain responsiveness. If we yield too often, we're wasting idle time.
      * We could do this by checking `performance.now()` regularly, but it's an expensive call in such a hot loop.
@@ -226,7 +212,7 @@ export class TraceProcessor extends EventTarget {
         // Yield to the UI because finalize() calls can be expensive
         // TODO(jacktfranklin): consider using `scheduler.yield()` or `scheduler.postTask(() => {}, {priority: 'user-blocking'})`
         await new Promise(resolve => setTimeout(resolve, 0));
-        await handler.finalize();
+        await handler.finalize(options);
       }
       const percent = calculateProgress(i / sortedHandlers.length, ProgressPhase.FINALIZE);
       this.dispatchEvent(new TraceParseProgressEvent({percent}));
@@ -442,7 +428,7 @@ export class TraceProcessor extends EventTarget {
   #computeInsightSet(
       insights: Insights.Types.TraceInsightSets, parsedTrace: Handlers.Types.ParsedTrace,
       insightRunners: Partial<typeof Insights.Models>, context: Insights.Types.InsightSetContext,
-      options: ParseOptions): void {
+      options: Types.Configuration.ParseOptions): void {
     const model = {} as Insights.Types.InsightSet['model'];
 
     for (const [name, insight] of Object.entries(insightRunners)) {
@@ -491,7 +477,7 @@ export class TraceProcessor extends EventTarget {
    */
   #computeInsights(
       parsedTrace: Handlers.Types.ParsedTrace, traceEvents: readonly Types.Events.Event[],
-      options: ParseOptions): void {
+      options: Types.Configuration.ParseOptions): void {
     this.#insights = new Map();
 
     const enabledInsightRunners = TraceProcessor.getEnabledInsightRunners(parsedTrace);

front_end/models/trace/extras/TimelineJSProfile.ts

@@ -26,14 +26,14 @@ export class TimelineJSProfileProcessor {
   }
 
   static createFakeTraceFromCpuProfile(profile: Protocol.Profiler.Profile, tid: Types.Events.ThreadID):
-      Types.Events.Event[] {
+      Types.File.TraceFile {
     const events: Types.Events.Event[] = [];
 
     const threadName = `Thread ${tid}`;
     appendEvent('TracingStartedInPage', {data: {sessionId: '1'}}, 0, 0, Types.Events.Phase.METADATA);
     appendEvent(Types.Events.Name.THREAD_NAME, {name: threadName}, 0, 0, Types.Events.Phase.METADATA, '__metadata');
     if (!profile) {
-      return events;
+      return {traceEvents: events, metadata: {}};
     }
 
     // Append a root to show the start time of the profile (which is earlier than first sample), so the Performance
@@ -45,7 +45,12 @@ export class TimelineJSProfileProcessor {
 
     // TODO: create a `Profile` event instead, as `cpuProfile` is legacy
     appendEvent('CpuProfile', {data: {cpuProfile: profile}}, profile.endTime, 0, Types.Events.Phase.COMPLETE);
-    return events;
+    return {
+      traceEvents: events,
+      metadata: {
+        dataOrigin: Types.File.DataOrigin.CPU_PROFILE,
+      }
+    };
 
     function appendEvent(
         name: string, args: any, ts: number, dur?: number, ph?: Types.Events.Phase, cat?: string): Types.Events.Event {

front_end/models/trace/handlers/SamplesHandler.test.ts

@@ -25,6 +25,25 @@ async function handleEventsFromTraceFile(context: Mocha.Context|Mocha.Suite|null
   return Trace.Handlers.ModelHandlers.Samples.data();
 }
 
+async function handleEventsFromCpuProfile(context: Mocha.Context|Mocha.Suite|null, name: string):
+    Promise<Trace.Handlers.ModelHandlers.Samples.SamplesHandlerData> {
+  const profile = await TraceLoader.rawCPUProfile(context, name);
+
+  const contents = Trace.Extras.TimelineJSProfile.TimelineJSProfileProcessor.createFakeTraceFromCpuProfile(
+      profile, Trace.Types.Events.ThreadID(1));
+
+  Trace.Handlers.ModelHandlers.Samples.reset();
+  for (const event of contents.traceEvents) {
+    Trace.Handlers.ModelHandlers.Meta.handleEvent(event);
+    Trace.Handlers.ModelHandlers.Samples.handleEvent(event);
+  }
+
+  await Trace.Handlers.ModelHandlers.Meta.finalize();
+  await Trace.Handlers.ModelHandlers.Samples.finalize({isCPUProfile: true});
+
+  return Trace.Handlers.ModelHandlers.Samples.data();
+}
+
 describeWithEnvironment('SamplesHandler', function() {
   it('finds all the profiles in a real world recording', async () => {
     const data = await handleEventsFromTraceFile(this, 'multiple-navigations-with-iframes.json.gz');
@@ -141,7 +160,7 @@ describeWithEnvironment('SamplesHandler', function() {
       for (const event of [mockProfileEvent, ...mockChunks]) {
         Trace.Handlers.ModelHandlers.Samples.handleEvent(event);
       }
-      await Trace.Handlers.ModelHandlers.Samples.finalize();
+      await Trace.Handlers.ModelHandlers.Samples.finalize({isCPUProfile: true});
       const data = Trace.Handlers.ModelHandlers.Samples.data();
       const calls = data.profilesInProcess.get(pid)?.get(tid)?.profileCalls.map(call => {
         const selfTime = data.entryToNode.get(call)?.selfTime;
@@ -177,23 +196,36 @@ describeWithEnvironment('SamplesHandler', function() {
 
       assert.deepEqual(callsTestData, expectedResult);
     });
-    it('can build profile calls from a CPU profile coming from a real world trace', async () => {
-      const data = await handleEventsFromTraceFile(this, 'multiple-navigations-with-iframes.json.gz');
+
+    it('can build profile calls from a CPU profile coming from a real world cpuprofile', async () => {
+      const data = await handleEventsFromCpuProfile(this, 'basic.cpuprofile.gz');
 
       const threadId = Trace.Types.Events.ThreadID(1);
-      const firstProcessId = Trace.Types.Events.ProcessID(2236123);
+      const firstProcessId = Trace.Types.Events.ProcessID(1);
       const profilesFirstProcess = data.profilesInProcess.get(firstProcessId);
-      const calls = profilesFirstProcess?.get(threadId)?.profileCalls.slice(0, 5).map(call => {
+      const profileData = profilesFirstProcess?.get(threadId) as Trace.Handlers.ModelHandlers.Samples.ProfileData;
+      // These particular calls are selected as some have children and others have selfTime
+      const calls = [
+        ...profileData?.profileCalls.slice(0, 4),
+        ...profileData?.profileCalls.slice(10, 15),
+      ].map(call => {
         const selfTime = data.entryToNode.get(call)?.selfTime;
         return {...call, selfTime};
       });
-      const tree = profilesFirstProcess?.get(threadId)?.profileTree;
+      const tree = profileData.profileTree;
       const expectedResult = [
-        {id: 2, dur: 392, ts: 643496962681, selfTime: 392, children: []},
-        {id: 3, dur: 682, ts: 643496963073, selfTime: 0, children: [4]},
-        {id: 4, dur: 682, ts: 643496963073, selfTime: 160, children: [5]},
-        {id: 5, dur: 522, ts: 643496963233, selfTime: 178, children: [6, 7]},
-        {id: 6, dur: 175, ts: 643496963411, selfTime: 175, children: []},
+        // The initial call stack
+        {id: 2, dur: 2369962, ts: 73029010084, selfTime: 0, children: [3]},
+        {id: 3, dur: 2369962, ts: 73029010084, selfTime: 0, children: [4]},
+        {id: 4, dur: 2369962, ts: 73029010084, selfTime: 0, children: [5]},
+        {id: 5, dur: 2369962, ts: 73029010084, selfTime: 0, children: [6]},
+        // various calls to hrtime
+        {id: 10, dur: 375, ts: 73029011751, selfTime: 375, children: []},
+        {id: 10, dur: 1083, ts: 73029012251, selfTime: 1083, children: []},
+        {id: 10, dur: 833, ts: 73029013459, selfTime: 833, children: []},
+        {id: 10, dur: 917, ts: 73029014417, selfTime: 792, children: []},
+        {id: 11, dur: 125, ts: 73029014667, selfTime: 125, children: []},
+
       ];
       assert.exists(tree?.roots);
       if (!tree?.roots) {

front_end/models/trace/handlers/SamplesHandler.ts

@@ -24,7 +24,7 @@ const entryToNode = new Map<Types.Events.Event, Helpers.TreeHelpers.TraceEntryNo
 // events matched by thread id.
 const preprocessedData = new Map<Types.Events.ProcessID, Map<Types.Events.ProfileID, PreprocessedData>>();
 
-function buildProfileCalls(): void {
+function parseCPUProfileData(parseOptions: Types.Configuration.ParseOptions): void {
   for (const [processId, profiles] of preprocessedData) {
     for (const [profileId, preProcessedData] of profiles) {
       const threadId = preProcessedData.threadId;
@@ -44,58 +44,65 @@ function buildProfileCalls(): void {
         profileTree,
         profileId,
       };
-
       const dataByThread = Platform.MapUtilities.getWithDefault(profilesInProcess, processId, () => new Map());
-      profileModel.forEachFrame(openFrameCallback, closeFrameCallback);
       dataByThread.set(threadId, finalizedData);
 
-      function openFrameCallback(
-          depth: number, node: CPUProfile.ProfileTreeModel.ProfileNode, sampleIndex: number,
-          timeStampMilliseconds: number): void {
-        if (threadId === undefined) {
-          return;
-        }
-        const ts = Helpers.Timing.milliToMicro(Types.Timing.Milli(timeStampMilliseconds));
-        const nodeId = node.id as Helpers.TreeHelpers.TraceEntryNodeId;
-
-        const profileCall = Helpers.Trace.makeProfileCall(node, profileId, sampleIndex, ts, processId, threadId);
-        finalizedData.profileCalls.push(profileCall);
-        indexStack.push(finalizedData.profileCalls.length - 1);
-        const traceEntryNode = Helpers.TreeHelpers.makeEmptyTraceEntryNode(profileCall, nodeId);
-        entryToNode.set(profileCall, traceEntryNode);
-        traceEntryNode.depth = depth;
-        if (indexStack.length === 1) {
-          // First call in the stack is a root call.
-          finalizedData.profileTree?.roots.add(traceEntryNode);
-        }
+      // Only need to build pure JS ProfileCalls if we're parsing a CPU Profile, otherwise SamplesIntegrator does the work.
+      if (parseOptions.isCPUProfile) {
+        buildProfileCallsForCPUProfile();
       }
-      function closeFrameCallback(
-          _depth: number, _node: CPUProfile.ProfileTreeModel.ProfileNode, _sampleIndex: number,
-          _timeStampMillis: number, durMs: number, selfTimeMs: number): void {
-        const profileCallIndex = indexStack.pop();
-        const profileCall = profileCallIndex !== undefined && finalizedData.profileCalls[profileCallIndex];
-        if (!profileCall) {
-          return;
-        }
-        const {callFrame, ts, pid, tid} = profileCall;
-        const traceEntryNode = entryToNode.get(profileCall);
-        if (callFrame === undefined || ts === undefined || pid === undefined || profileId === undefined ||
-            tid === undefined || traceEntryNode === undefined) {
-          return;
+
+      function buildProfileCallsForCPUProfile(): void {
+        profileModel.forEachFrame(openFrameCallback, closeFrameCallback);
+
+        function openFrameCallback(
+            depth: number, node: CPUProfile.ProfileTreeModel.ProfileNode, sampleIndex: number,
+            timeStampMilliseconds: number): void {
+          if (threadId === undefined) {
+            return;
+          }
+          const ts = Helpers.Timing.milliToMicro(Types.Timing.Milli(timeStampMilliseconds));
+          const nodeId = node.id as Helpers.TreeHelpers.TraceEntryNodeId;
+
+          const profileCall = Helpers.Trace.makeProfileCall(node, profileId, sampleIndex, ts, processId, threadId);
+          finalizedData.profileCalls.push(profileCall);
+          indexStack.push(finalizedData.profileCalls.length - 1);
+          const traceEntryNode = Helpers.TreeHelpers.makeEmptyTraceEntryNode(profileCall, nodeId);
+          entryToNode.set(profileCall, traceEntryNode);
+          traceEntryNode.depth = depth;
+          if (indexStack.length === 1) {
+            // First call in the stack is a root call.
+            finalizedData.profileTree?.roots.add(traceEntryNode);
+          }
         }
-        const dur = Helpers.Timing.milliToMicro(Types.Timing.Milli(durMs));
-        const selfTime = Helpers.Timing.milliToMicro(Types.Timing.Milli(selfTimeMs));
-        profileCall.dur = dur;
-        traceEntryNode.selfTime = selfTime;
+        function closeFrameCallback(
+            _depth: number, _node: CPUProfile.ProfileTreeModel.ProfileNode, _sampleIndex: number,
+            _timeStampMillis: number, durMs: number, selfTimeMs: number): void {
+          const profileCallIndex = indexStack.pop();
+          const profileCall = profileCallIndex !== undefined && finalizedData.profileCalls[profileCallIndex];
+          if (!profileCall) {
+            return;
+          }
+          const {callFrame, ts, pid, tid} = profileCall;
+          const traceEntryNode = entryToNode.get(profileCall);
+          if (callFrame === undefined || ts === undefined || pid === undefined || profileId === undefined ||
+              tid === undefined || traceEntryNode === undefined) {
+            return;
+          }
+          const dur = Helpers.Timing.milliToMicro(Types.Timing.Milli(durMs));
+          const selfTime = Helpers.Timing.milliToMicro(Types.Timing.Milli(selfTimeMs));
+          profileCall.dur = dur;
+          traceEntryNode.selfTime = selfTime;
 
-        const parentIndex = indexStack.at(-1);
-        const parent = parentIndex !== undefined && finalizedData.profileCalls.at(parentIndex);
-        const parentNode = parent && entryToNode.get(parent);
-        if (!parentNode) {
-          return;
+          const parentIndex = indexStack.at(-1);
+          const parent = parentIndex !== undefined && finalizedData.profileCalls.at(parentIndex);
+          const parentNode = parent && entryToNode.get(parent);
+          if (!parentNode) {
+            return;
+          }
+          traceEntryNode.parent = parentNode;
+          parentNode.children.push(traceEntryNode);
         }
-        traceEntryNode.parent = parentNode;
-        parentNode.children.push(traceEntryNode);
       }
     }
   }
@@ -183,8 +190,8 @@ export function handleEvent(event: Types.Events.Event): void {
   }
 }
 
-export async function finalize(): Promise<void> {
-  buildProfileCalls();
+export async function finalize(parseOptions: Types.Configuration.ParseOptions = {}): Promise<void> {
+  parseCPUProfileData(parseOptions);
 }
 
 export function data(): SamplesHandlerData {

front_end/models/trace/handlers/Threads.test.ts

@@ -38,13 +38,13 @@ describeWithEnvironment('Handler Threads helper', function() {
     // Bit of extra setup required: we need to mimic what the panel does where
     // it takes the CDP Profile and wraps it in fake trace events, before then
     // passing that through to the new engine.
-    const rawEvents = await TraceLoader.rawCPUProfile(this, 'node-fibonacci-website.cpuprofile.gz');
-    const events = Trace.Extras.TimelineJSProfile.TimelineJSProfileProcessor.createFakeTraceFromCpuProfile(
-        rawEvents,
+    const profile = await TraceLoader.rawCPUProfile(this, 'node-fibonacci-website.cpuprofile.gz');
+    const contents = Trace.Extras.TimelineJSProfile.TimelineJSProfileProcessor.createFakeTraceFromCpuProfile(
+        profile,
         Trace.Types.Events.ThreadID(1),
     );
-    const {parsedTrace} =
-        await TraceLoader.executeTraceEngineOnFileContents(events as unknown as Trace.Types.Events.Event[]);
+
+    const {parsedTrace} = await TraceLoader.executeTraceEngineOnFileContents(contents);
 
     // Check that we did indeed parse this properly as a CPU Profile.
     assert.strictEqual(parsedTrace.Renderer.processes.size, 0);

front_end/models/trace/handlers/Threads.ts

@@ -98,16 +98,15 @@ export function threadsInTrace(parsedTrace: ParsedTrace): readonly ThreadData[]
     return cached;
   }
 
-  // If we have Renderer threads, we prefer to use those. In the event that a
-  // trace is a CPU Profile trace, we will never have Renderer threads, so we
-  // know if there are no Renderer threads that we can fallback to using the
-  // data from the SamplesHandler.
+  // If we have Renderer threads, we prefer to use those.
   const threadsFromRenderer = threadsInRenderer(parsedTrace.Renderer, parsedTrace.AuctionWorklets);
   if (threadsFromRenderer.length) {
     threadsInTraceCache.set(parsedTrace, threadsFromRenderer);
     return threadsFromRenderer;
   }
 
+  // If it's a CPU Profile trace, there will be no Renderer threads.
+  // We can fallback to using the data from the SamplesHandler.
   const foundThreads: ThreadData[] = [];
   if (parsedTrace.Samples.profilesInProcess.size) {
     for (const [pid, process] of parsedTrace.Samples.profilesInProcess) {

front_end/models/trace/handlers/types.ts

@@ -8,11 +8,12 @@ import type * as ModelHandlers from './ModelHandlers.js';
 export interface Handler {
   reset(): void;
   handleEvent(data: {}): void;
-  finalize(): Promise<void>;
+  finalize(options?: Types.Configuration.ParseOptions): Promise<void>;
   data(): unknown;
   deps?(): HandlerName[];
   handleUserConfig?(config: Types.Configuration.Configuration): void;
 }
+
 export type HandlerName = keyof typeof ModelHandlers;
 
 // This type maps Handler names to the return type of their data

front_end/models/trace/types/Configuration.ts

@@ -2,6 +2,8 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
+import type * as File from './File.js';
+
 export interface Configuration {
   /**
    * Include V8 RCS functions in the JS stacks
@@ -48,3 +50,18 @@ export const defaults = (): Configuration => ({
 export function configToCacheKey(config: Configuration): string {
   return JSON.stringify(config);
 }
+
+export interface ParseOptions {
+  /**
+   * If the trace was just recorded on the current page, rather than an imported file.
+   * TODO(paulirish): Maybe remove. This is currently unused by the Processor and Handlers
+   * @default false
+   */
+  isFreshRecording?: boolean;
+  /**
+   * If the trace is a CPU Profile rather than a Chrome tracing trace.
+   * @default false
+   */
+  isCPUProfile?: boolean;
+  metadata?: File.MetaData;
+}