RPP: improve performance of FramesHandler by taking subset of events

The FramesHandler took a copy of all trace events, sorted them, and then
passed them into TimelineFrameModel.

This was a performance hit because sorting a large trace (my test trace
of Figma has over 1m events, and it's not even particularly huge) is
expensive.

Looking through the code, I noticed that we actually only care about a
subset of the events. On my Figma test trace, about 100k of the events
are relevant to the TimelineFrameModel, which is 10% of the total.

Therefore, this CL updates the code to pick out the relevant events in
`handleEvent` and sort just that subset.

On my test trace, this has a decent performance boost:

Before:
- parse:Frames:finalize: 250ms

After:
- parse:Frames:finalize: 65ms

[email protected]

Bug: 436491188
Change-Id: I882fb1ff052fe983653228ec94bbf3041d4c46e1
Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6850647
Commit-Queue: Connor Clark <[email protected]>
Auto-Submit: Jack Franklin <[email protected]>
Reviewed-by: Connor Clark <[email protected]>

Author: jackfranklin

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

@@ -2,7 +2,6 @@
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
-import {describeWithMockConnection} from '../../../testing/MockConnection.js';
 import {TraceLoader} from '../../../testing/TraceLoader.js';
 import * as Trace from '../trace.js';
 
@@ -31,7 +30,7 @@ async function processTrace(events: readonly Trace.Types.Events.Event[]): Promis
   }
 }
 
-describeWithMockConnection('FramesHandler', () => {
+describe('FramesHandler', () => {
   it('can parse out a trace and return the frames', async function() {
     const rawEvents = await TraceLoader.rawEvents(this, 'web-dev-with-commit.json.gz');
     await processTrace(rawEvents);
@@ -66,10 +65,6 @@ describeWithMockConnection('FramesHandler', () => {
   });
 
   it('can create LayerPaintEvents from Paint and snapshot events', async function() {
-    // Advanced instrumentation trace file is large: allow the bots more time
-    // to process it.
-    this.timeout(20_000);
-
     const rawEvents = await TraceLoader.rawEvents(this, 'web-dev-with-advanced-instrumentation.json.gz');
     await processTrace(rawEvents);
     const parsedFrames = Trace.Handlers.ModelHandlers.Frames.data().frames;

front_end/models/trace/handlers/FramesHandler.ts

@@ -11,7 +11,7 @@ import {data as layerTreeHandlerData, type LayerTreeData} from './LayerTreeHandl
 import {data as metaHandlerData, type MetaHandlerData} from './MetaHandler.js';
 import {data as rendererHandlerData, type RendererHandlerData} from './RendererHandler.js';
 import * as Threads from './Threads.js';
-import type {FinalizeOptions, HandlerName} from './types.js';
+import type {HandlerName} from './types.js';
 
 /**
  * IMPORTANT: this handler is slightly different to the rest. This is because
@@ -25,22 +25,61 @@ import type {FinalizeOptions, HandlerName} from './types.js';
  */
 
 let model: TimelineFrameModel|null = null;
+const relevantFrameEvents: Types.Events.Event[] = [];
+
+type FrameEvent = Types.Events.BeginFrame|Types.Events.DroppedFrame|Types.Events.RequestMainThreadFrame|
+                  Types.Events.BeginMainThreadFrame|Types.Events.Commit|Types.Events.CompositeLayers|
+                  Types.Events.ActivateLayerTree|Types.Events.NeedsBeginFrameChanged|Types.Events.DrawFrame;
+
+function isFrameEvent(event: Types.Events.Event): event is FrameEvent {
+  return (
+      Types.Events.isSetLayerId(event) || Types.Events.isBeginFrame(event) || Types.Events.isDroppedFrame(event) ||
+      Types.Events.isRequestMainThreadFrame(event) || Types.Events.isBeginMainThreadFrame(event) ||
+      Types.Events.isNeedsBeginFrameChanged(event) ||
+      // Note that "Commit" is the replacement for "CompositeLayers" so in a trace
+      // we wouldn't expect to see a combination of these. All "new" trace
+      // recordings use "Commit", but we can easily support "CompositeLayers" too
+      // to not break older traces being imported.
+      Types.Events.isCommit(event) || Types.Events.isCompositeLayers(event) ||
+      Types.Events.isActivateLayerTree(event) || Types.Events.isDrawFrame(event));
+}
+
+function entryIsTopLevel(entry: Types.Events.Event): boolean {
+  const devtoolsTimelineCategory = 'disabled-by-default-devtools.timeline';
+  return entry.name === Types.Events.Name.RUN_TASK && entry.cat.includes(devtoolsTimelineCategory);
+}
+
+const MAIN_FRAME_MARKERS = new Set<Types.Events.Name>([
+  Types.Events.Name.SCHEDULE_STYLE_RECALCULATION,
+  Types.Events.Name.INVALIDATE_LAYOUT,
+  Types.Events.Name.BEGIN_MAIN_THREAD_FRAME,
+  Types.Events.Name.SCROLL_LAYER,
+]);
 
 export function reset(): void {
   model = null;
+  relevantFrameEvents.length = 0;
 }
-export function handleEvent(): void {
+export function handleEvent(event: Types.Events.Event): void {
+  // This might seem like a wide set of events to filter for, but these are all
+  // the types of events that we care about in the TimelineFrameModel class at
+  // the bottom of this file. Previously we would take a copy of an array of
+  // all trace events, but on a few test traces, this set of filtered events
+  // accounts for about 10% of the total events, so it's a big performance win
+  // to deal with a much smaller subset of the data.
+  if (isFrameEvent(event) || Types.Events.isLayerTreeHostImplSnapshot(event) || entryIsTopLevel(event) ||
+      MAIN_FRAME_MARKERS.has(event.name as Types.Events.Name) || Types.Events.isPaint(event)) {
+    relevantFrameEvents.push(event);
+  }
 }
 
-export async function finalize({allTraceEvents}: FinalizeOptions): Promise<void> {
-  // Snapshot events can be emitted out of order, so we need to sort before
-  // building the frames model, but we need to take a copy because we don't
-  // can't mutate the raw input.
-  // TODO(crbug.com/436491188): this is expensive...we copy and sort the entire trace each time we import it.
-  const allEvents = allTraceEvents.toSorted(Helpers.Trace.eventTimeComparator);
+export async function finalize(): Promise<void> {
+  // We have to sort the events by timestamp, because the model code expects to
+  // process events in order.
+  Helpers.Trace.sortTraceEventsInPlace(relevantFrameEvents);
 
   const modelForTrace = new TimelineFrameModel(
-      allEvents,
+      relevantFrameEvents,
       rendererHandlerData(),
       auctionWorkletsData(),
       metaHandlerData(),
@@ -65,28 +104,6 @@ export function deps(): HandlerName[] {
   return ['Meta', 'Renderer', 'AuctionWorklets', 'LayerTree'];
 }
 
-type FrameEvent = Types.Events.BeginFrame|Types.Events.DroppedFrame|Types.Events.RequestMainThreadFrame|
-                  Types.Events.BeginMainThreadFrame|Types.Events.Commit|Types.Events.CompositeLayers|
-                  Types.Events.ActivateLayerTree|Types.Events.NeedsBeginFrameChanged|Types.Events.DrawFrame;
-
-function isFrameEvent(event: Types.Events.Event): event is FrameEvent {
-  return (
-      Types.Events.isSetLayerId(event) || Types.Events.isBeginFrame(event) || Types.Events.isDroppedFrame(event) ||
-      Types.Events.isRequestMainThreadFrame(event) || Types.Events.isBeginMainThreadFrame(event) ||
-      Types.Events.isNeedsBeginFrameChanged(event) ||
-      // Note that "Commit" is the replacement for "CompositeLayers" so in a trace
-      // we wouldn't expect to see a combination of these. All "new" trace
-      // recordings use "Commit", but we can easily support "CompositeLayers" too
-      // to not break older traces being imported.
-      Types.Events.isCommit(event) || Types.Events.isCompositeLayers(event) ||
-      Types.Events.isActivateLayerTree(event) || Types.Events.isDrawFrame(event));
-}
-
-function entryIsTopLevel(entry: Types.Events.Event): boolean {
-  const devtoolsTimelineCategory = 'disabled-by-default-devtools.timeline';
-  return entry.name === Types.Events.Name.RUN_TASK && entry.cat.includes(devtoolsTimelineCategory);
-}
-
 export class TimelineFrameModel {
   #frames: TimelineFrame[] = [];
   #frameById: Record<number, TimelineFrame> = {};
@@ -362,13 +379,6 @@ export class TimelineFrameModel {
   }
 }
 
-const MAIN_FRAME_MARKERS = new Set<Types.Events.Name>([
-  Types.Events.Name.SCHEDULE_STYLE_RECALCULATION,
-  Types.Events.Name.INVALIDATE_LAYOUT,
-  Types.Events.Name.BEGIN_MAIN_THREAD_FRAME,
-  Types.Events.Name.SCROLL_LAYER,
-]);
-
 /**
  * Legacy class that represents TimelineFrames that was ported from the old SDK.
  * This class is purposefully not exported as it breaks the abstraction that