Skip to content

Commit 1f1d642

Browse files
paulirishDevtools-frontend LUCI CQ
authored andcommitted
DrJones/Perf: Ensure selected event is always within its own tree
Bug:377672675 Change-Id: Ie514815ed09c586eee1ac827774d6dc5161ce159 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6002096 Reviewed-by: Alex Rudenko <[email protected]> Auto-Submit: Paul Irish <[email protected]> Commit-Queue: Alex Rudenko <[email protected]>
1 parent 5708181 commit 1f1d642

File tree

3 files changed

+125
-12
lines changed

3 files changed

+125
-12
lines changed

front_end/models/trace/extras/TraceTree.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@ export class TopDownNode extends Node {
103103
}
104104
const startTime = root.startTime;
105105
const endTime = root.endTime;
106-
const instantEventCallback = root.doNotAggregate ? onInstantEvent : undefined;
106+
const instantEventCallback = (root.doNotAggregate || root.includeInstantEvents) ? onInstantEvent : undefined;
107107
const eventIdCallback = root.doNotAggregate ? undefined : generateEventID;
108108
const eventGroupIdCallback = root.getEventGroupIdCallback();
109109
let depth = 0;
@@ -238,18 +238,21 @@ export class TopDownNode extends Node {
238238

239239
export class TopDownRootNode extends TopDownNode {
240240
readonly filter: (e: Types.Events.Event) => boolean;
241+
/** This is all events passed in to create the tree, and it's very likely that it included events outside of the passed startTime/endTime as that filtering is done in `Helpers.Trace.forEachEvent` */
241242
readonly events: Types.Events.Event[];
242243
readonly startTime: Types.Timing.MilliSeconds;
243244
readonly endTime: Types.Timing.MilliSeconds;
244245
eventGroupIdCallback: ((arg0: Types.Events.Event) => string)|null|undefined;
246+
/** Default behavior is to aggregate similar trace events into one Node based on generateEventID(), eventGroupIdCallback(), etc. Set true to keep nodes 1:1 with events. */
245247
readonly doNotAggregate: boolean|undefined;
248+
readonly includeInstantEvents?: boolean;
246249
override totalTime: number;
247250
override selfTime: number;
248251

249252
constructor(
250253
events: Types.Events.Event[], filters: TraceFilter[], startTime: Types.Timing.MilliSeconds,
251254
endTime: Types.Timing.MilliSeconds, doNotAggregate?: boolean,
252-
eventGroupIdCallback?: ((arg0: Types.Events.Event) => string)|null) {
255+
eventGroupIdCallback?: ((arg0: Types.Events.Event) => string)|null, includeInstantEvents?: boolean) {
253256
super('', null, null);
254257
this.root = this;
255258
this.events = events;
@@ -258,6 +261,8 @@ export class TopDownRootNode extends TopDownNode {
258261
this.endTime = endTime;
259262
this.eventGroupIdCallback = eventGroupIdCallback;
260263
this.doNotAggregate = doNotAggregate;
264+
this.includeInstantEvents = includeInstantEvents;
265+
261266
this.totalTime = endTime - startTime;
262267
this.selfTime = this.totalTime;
263268
}

front_end/panels/timeline/utils/AICallTree.test.ts

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Use of this source code is governed by a BSD-style license that can be
33
// found in the LICENSE file.
44

5+
import * as Trace from '../../../models/trace/trace.js';
56
import {describeWithEnvironment} from '../../../testing/EnvironmentHelpers.js';
67
import {TraceLoader} from '../../../testing/TraceLoader.js';
78

@@ -77,4 +78,108 @@ self: 0.2
7778
const fibCallCount = lines.filter(line => line.includes('fibonacci')).length;
7879
assert.isTrue(fibCallCount > 10);
7980
});
81+
82+
it('AITreeFilter includes the right items in the tree', async function() {
83+
const {parsedTrace} = await TraceLoader.traceEngine(this, 'two-workers.json.gz');
84+
const mainEvents = parsedTrace.Renderer.allTraceEntries;
85+
86+
// A very small 'get storage' event. It's 6µs long
87+
const tinyEvent = mainEvents.find(event => event.ts === 107350149168);
88+
if (!tinyEvent) {
89+
throw new Error('Could not find expected event.');
90+
}
91+
const tinyStr = Utils.AICallTree.AICallTree.from(tinyEvent, mainEvents, parsedTrace).serialize();
92+
assert.strictEqual(tinyStr.split('\n').filter(l => l.startsWith('Node:')).join('\n'), `
93+
Node: 1 – Task
94+
Node: 2 – Parse HTML
95+
Node: 3 – Evaluate script
96+
Node: 4 – (anonymous)
97+
Node: 5 – get storage`.trim());
98+
assert.include(tinyStr, 'get storage');
99+
100+
// An evaluateScript that has 3 'Compile code' children
101+
const evaluateEvent = mainEvents.find(event => event.ts === 107350147808);
102+
if (!evaluateEvent) {
103+
throw new Error('Could not find expected event.');
104+
}
105+
const treeStr = Utils.AICallTree.AICallTree.from(evaluateEvent, mainEvents, parsedTrace).serialize();
106+
assert.strictEqual(treeStr.split('\n').filter(l => l.startsWith('Node:')).join('\n'), `
107+
Node: 1 – Task
108+
Node: 2 – Parse HTML
109+
Node: 3 – Evaluate script
110+
Node: 4 – Compile script
111+
Node: 5 – (anonymous)
112+
Node: 6 – H.la`.trim());
113+
assert.notInclude(treeStr, 'Compile code');
114+
115+
// An Compile code event within the evaluateEvent call tree
116+
const compileEvent = mainEvents.find(event => event.ts === 107350148218);
117+
if (!compileEvent) {
118+
throw new Error('Could not find expected event.');
119+
}
120+
const compileStr = Utils.AICallTree.AICallTree.from(compileEvent, mainEvents, parsedTrace).serialize();
121+
assert.strictEqual(compileStr.split('\n').filter(l => l.startsWith('Node:')).join('\n'), `
122+
Node: 1 – Task
123+
Node: 2 – Parse HTML
124+
Node: 3 – Evaluate script
125+
Node: 4 – (anonymous)
126+
Node: 5 – Compile code`.trim());
127+
assert.include(compileStr, 'Compile code');
128+
});
129+
});
130+
131+
describe('AITreeFilter', () => {
132+
const makeEvent = (
133+
name: string,
134+
ts: number,
135+
dur: number,
136+
): Trace.Types.Events.Event => ({
137+
name,
138+
cat: 'disabled-by-default-devtools.timeline',
139+
ph: Trace.Types.Events.Phase.COMPLETE,
140+
ts: Trace.Types.Timing.MicroSeconds(ts),
141+
dur: Trace.Types.Timing.MicroSeconds(dur),
142+
pid: Trace.Types.Events.ProcessID(1),
143+
tid: Trace.Types.Events.ThreadID(4),
144+
args: {},
145+
});
146+
147+
it('always includes the selected event', () => {
148+
const selectedEvent = makeEvent('selected', 0, 100);
149+
const filter = new Utils.AICallTree.AITreeFilter(selectedEvent);
150+
assert.strictEqual(filter.accept(selectedEvent), true);
151+
});
152+
153+
it('includes events that are long enough', () => {
154+
const selectedEvent = makeEvent('selected', 0, 100);
155+
const filter = new Utils.AICallTree.AITreeFilter(selectedEvent);
156+
157+
assert.strictEqual(filter.accept(makeEvent('short', 0, 1)), true);
158+
assert.strictEqual(filter.accept(makeEvent('short', 0, 0.6)), true);
159+
assert.strictEqual(filter.accept(makeEvent('long', 0, 101)), true);
160+
assert.strictEqual(filter.accept(makeEvent('long', 0, 200)), true);
161+
assert.strictEqual(filter.accept(makeEvent('long', 0, 1000)), true);
162+
});
163+
164+
it('excludes events that are too short', () => {
165+
const selectedEvent = makeEvent('selected', 0, 100);
166+
const filter = new Utils.AICallTree.AITreeFilter(selectedEvent);
167+
168+
assert.strictEqual(filter.accept(makeEvent('short', 0, 0)), false);
169+
assert.strictEqual(filter.accept(makeEvent('short', 0, 0.1)), false);
170+
assert.strictEqual(filter.accept(makeEvent('short', 0, 0.4)), false);
171+
});
172+
173+
it('excludes COMPILE_CODE nodes if non-selected', () => {
174+
const selectedEvent = makeEvent('selected', 0, 100);
175+
const compileCodeEvent = makeEvent(Trace.Types.Events.Name.COMPILE_CODE, 0, 100);
176+
const filter = new Utils.AICallTree.AITreeFilter(selectedEvent);
177+
assert.strictEqual(filter.accept(compileCodeEvent), false);
178+
});
179+
180+
it('includes COMPILE_CODE nodes if selected', () => {
181+
const selectedEvent = makeEvent(Trace.Types.Events.Name.COMPILE_CODE, 0, 100);
182+
const filter = new Utils.AICallTree.AITreeFilter(selectedEvent);
183+
assert.strictEqual(filter.accept(selectedEvent), true);
184+
});
80185
});

front_end/panels/timeline/utils/AICallTree.ts

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -31,22 +31,22 @@ export class AICallTree {
3131
static from(
3232
selectedEvent: Trace.Types.Events.Event, events: Trace.Types.Events.Event[],
3333
parsedTrace: Trace.Handlers.Types.ParsedTrace): AICallTree {
34-
const timings = Trace.Helpers.Timing.eventTimingsMilliSeconds(selectedEvent);
35-
const selectedEventBounds = Trace.Helpers.Timing.traceWindowFromMicroSeconds(
36-
Trace.Helpers.Timing.millisecondsToMicroseconds(timings.startTime),
37-
Trace.Helpers.Timing.millisecondsToMicroseconds(timings.endTime));
34+
const {startTime, endTime} = Trace.Helpers.Timing.eventTimingsMilliSeconds(selectedEvent);
3835

36+
const selectedEventBounds = Trace.Helpers.Timing.traceWindowFromMicroSeconds(
37+
Trace.Helpers.Timing.millisecondsToMicroseconds(startTime),
38+
Trace.Helpers.Timing.millisecondsToMicroseconds(endTime));
3939
const threadEvents = parsedTrace.Renderer.processes.get(selectedEvent.pid)?.threads.get(selectedEvent.tid)?.entries;
4040
if (!threadEvents) {
4141
throw new Error('Cannot locate thread');
4242
}
4343
const overlappingEvents = threadEvents.filter(e => Trace.Helpers.Timing.eventIsInBounds(e, selectedEventBounds));
4444

4545
const visibleEventsFilter = new Trace.Extras.TraceFilter.VisibleEventsFilter(visibleTypes());
46-
const customFilter = new AITreeFilter(timings.duration);
46+
const customFilter = new AITreeFilter(selectedEvent);
4747
// Build a tree bounded by the selected event's timestamps, and our other filters applied
4848
const rootNode = new Trace.Extras.TraceTree.TopDownRootNode(
49-
overlappingEvents, [visibleEventsFilter, customFilter], timings.startTime, timings.endTime, false, null);
49+
overlappingEvents, [visibleEventsFilter, customFilter], startTime, endTime, false, null, true);
5050

5151
// Walk the tree to find selectedNode
5252
let selectedNode: Trace.Extras.TraceTree.Node|null = null;
@@ -147,14 +147,17 @@ export class AICallTree {
147147

148148
export class AITreeFilter extends Trace.Extras.TraceFilter.TraceFilter {
149149
#minDuration: Trace.Types.Timing.MicroSeconds;
150-
constructor(eventDuration: Trace.Types.Timing.MilliSeconds) {
150+
#selectedEvent: Trace.Types.Events.Event;
151+
constructor(selectedEvent: Trace.Types.Events.Event) {
151152
super();
152153
// The larger the selected event is, the less small ones matter. We'll exclude items under ½% of the selected event's size
153-
// We'll always exclude items under 0.15ms of total time.
154-
const minDurationMs = Math.max(Trace.Types.Timing.MilliSeconds(0.15), eventDuration * 0.005);
155-
this.#minDuration = Trace.Helpers.Timing.millisecondsToMicroseconds(Trace.Types.Timing.MilliSeconds(minDurationMs));
154+
this.#minDuration = Trace.Types.Timing.MicroSeconds((selectedEvent.dur ?? 1) * 0.005);
155+
this.#selectedEvent = selectedEvent;
156156
}
157157
accept(event: Trace.Types.Events.Event): boolean {
158+
if (event === this.#selectedEvent) {
159+
return true;
160+
}
158161
if (event.name === Trace.Types.Events.Name.COMPILE_CODE) {
159162
return false;
160163
}

0 commit comments

Comments
 (0)