Skip to content

Commit bcea1c7

Browse files
paulirishDevtools-frontend LUCI CQ
authored andcommitted
Remove TimelineJSProfile by merging it into SamplesIntegrator
- Move createFakeTraceFromCpuProfile to SamplesIntegrator static. Hard to say it *belongs* there, but it's a decent home. - DRY up duplicate nativeGroup impl; SamplesIntegrator's existing one gets a const enum upgrade. Change-Id: I2d2d31c9a4dd6a8e96a89c627fb0b26f01491e20 Bug: 358583420 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6277146 Auto-Submit: Paul Irish <[email protected]> Reviewed-by: Jack Franklin <[email protected]> Commit-Queue: Jack Franklin <[email protected]>
1 parent 6470381 commit bcea1c7

File tree

12 files changed

+73
-100
lines changed

12 files changed

+73
-100
lines changed

config/gni/devtools_grd_files.gni

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1095,7 +1095,6 @@ grd_files_debug_sources = [
10951095
"front_end/models/trace/extras/ScriptDuplication.js",
10961096
"front_end/models/trace/extras/StackTraceForEvent.js",
10971097
"front_end/models/trace/extras/ThirdParties.js",
1098-
"front_end/models/trace/extras/TimelineJSProfile.js",
10991098
"front_end/models/trace/extras/TraceFilter.js",
11001099
"front_end/models/trace/extras/TraceTree.js",
11011100
"front_end/models/trace/handlers/AnimationFramesHandler.js",

front_end/models/trace/extras/BUILD.gn

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ devtools_module("extras") {
1616
"ScriptDuplication.ts",
1717
"StackTraceForEvent.ts",
1818
"ThirdParties.ts",
19-
"TimelineJSProfile.ts",
2019
"TraceFilter.ts",
2120
"TraceTree.ts",
2221
]

front_end/models/trace/extras/TimelineJSProfile.ts

Lines changed: 0 additions & 81 deletions
This file was deleted.

front_end/models/trace/extras/TraceTree.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,9 +4,9 @@
44

55
import type * as Protocol from '../../../generated/protocol.js';
66
import * as Helpers from '../helpers/helpers.js';
7+
import {SamplesIntegrator} from '../helpers/SamplesIntegrator.js';
78
import * as Types from '../types/types.js';
89

9-
import {TimelineJSProfileProcessor} from './TimelineJSProfile.js';
1010
import type {TraceFilter} from './TraceFilter.js';
1111

1212
export class Node {
@@ -591,8 +591,8 @@ export function eventStackFrame(event: Types.Events.Event): Protocol.Runtime.Cal
591591

592592
export function generateEventID(event: Types.Events.Event): string {
593593
if (Types.Events.isProfileCall(event)) {
594-
const name = TimelineJSProfileProcessor.isNativeRuntimeFrame(event.callFrame) ?
595-
TimelineJSProfileProcessor.nativeGroup(event.callFrame.functionName) :
594+
const name = SamplesIntegrator.isNativeRuntimeFrame(event.callFrame) ?
595+
SamplesIntegrator.nativeGroup(event.callFrame.functionName) :
596596
event.callFrame.functionName;
597597
const location = event.callFrame.scriptId || event.callFrame.url || '';
598598
return `f:${name}@${location}`;

front_end/models/trace/extras/extras.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,5 @@ export * as Metadata from './Metadata.js';
99
export * as ScriptDuplication from './ScriptDuplication.js';
1010
export * as StackTraceForEvent from './StackTraceForEvent.js';
1111
export * as ThirdParties from './ThirdParties.js';
12-
export * as TimelineJSProfile from './TimelineJSProfile.js';
1312
export * as TraceFilter from './TraceFilter.js';
1413
export * as TraceTree from './TraceTree.js';

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ async function handleEventsFromCpuProfile(context: Mocha.Context|Mocha.Suite|nul
2929
Promise<Trace.Handlers.ModelHandlers.Samples.SamplesHandlerData> {
3030
const profile = await TraceLoader.rawCPUProfile(context, name);
3131

32-
const contents = Trace.Extras.TimelineJSProfile.TimelineJSProfileProcessor.createFakeTraceFromCpuProfile(
32+
const contents = Trace.Helpers.SamplesIntegrator.SamplesIntegrator.createFakeTraceFromCpuProfile(
3333
profile, Trace.Types.Events.ThreadID(1));
3434

3535
Trace.Handlers.ModelHandlers.Samples.reset();

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ describeWithEnvironment('Handler Threads helper', function() {
3939
// it takes the CDP Profile and wraps it in fake trace events, before then
4040
// passing that through to the new engine.
4141
const profile = await TraceLoader.rawCPUProfile(this, 'node-fibonacci-website.cpuprofile.gz');
42-
const contents = Trace.Extras.TimelineJSProfile.TimelineJSProfileProcessor.createFakeTraceFromCpuProfile(
42+
const contents = Trace.Helpers.SamplesIntegrator.SamplesIntegrator.createFakeTraceFromCpuProfile(
4343
profile,
4444
Trace.Types.Events.ThreadID(1),
4545
);

front_end/models/trace/helpers/SamplesIntegrator.ts

Lines changed: 58 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -458,12 +458,12 @@ export class SamplesIntegrator {
458458
return runtimeCallStatsEnabled && Boolean(SamplesIntegrator.nativeGroup(name));
459459
}
460460

461-
static nativeGroup(nativeName: string): 'Parse'|'Compile'|null {
461+
static nativeGroup(nativeName: string): SamplesIntegrator.NativeGroups|null {
462462
if (nativeName.startsWith('Parse')) {
463-
return 'Parse';
463+
return SamplesIntegrator.NativeGroups.PARSE;
464464
}
465465
if (nativeName.startsWith('Compile') || nativeName.startsWith('Recompile')) {
466-
return 'Compile';
466+
return SamplesIntegrator.NativeGroups.COMPILE;
467467
}
468468
return null;
469469
}
@@ -496,4 +496,59 @@ export class SamplesIntegrator {
496496
}
497497
stack.length = j;
498498
}
499+
500+
static createFakeTraceFromCpuProfile(profile: Protocol.Profiler.Profile, tid: Types.Events.ThreadID):
501+
Types.File.TraceFile {
502+
const events: Types.Events.Event[] = [];
503+
504+
const threadName = `Thread ${tid}`;
505+
appendEvent('TracingStartedInPage', {data: {sessionId: '1'}}, 0, 0, Types.Events.Phase.METADATA);
506+
appendEvent(Types.Events.Name.THREAD_NAME, {name: threadName}, 0, 0, Types.Events.Phase.METADATA, '__metadata');
507+
if (!profile) {
508+
return {traceEvents: events, metadata: {}};
509+
}
510+
511+
// Append a root to show the start time of the profile (which is earlier than first sample), so the Performance
512+
// panel won't truncate this time period.
513+
// 'JSRoot' doesn't exist in the new engine and is not the name of an actual trace event, but changing it might break other trace processing tools that rely on this, so we stick with this name.
514+
// TODO(crbug.com/341234884): consider removing this or clarify why it's required.
515+
appendEvent(
516+
'JSRoot', {}, profile.startTime, profile.endTime - profile.startTime, Types.Events.Phase.COMPLETE, 'toplevel');
517+
518+
// TODO: create a `Profile` event instead, as `cpuProfile` is legacy
519+
appendEvent('CpuProfile', {data: {cpuProfile: profile}}, profile.endTime, 0, Types.Events.Phase.COMPLETE);
520+
return {
521+
traceEvents: events,
522+
metadata: {
523+
dataOrigin: Types.File.DataOrigin.CPU_PROFILE,
524+
}
525+
};
526+
527+
function appendEvent(
528+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
529+
name: string, args: any, ts: number, dur?: number, ph?: Types.Events.Phase, cat?: string): Types.Events.Event {
530+
const event: Types.Events.Event = {
531+
cat: cat || 'disabled-by-default-devtools.timeline',
532+
name,
533+
ph: ph || Types.Events.Phase.COMPLETE,
534+
pid: Types.Events.ProcessID(1),
535+
tid,
536+
ts: Types.Timing.Micro(ts),
537+
args,
538+
};
539+
540+
if (dur) {
541+
event.dur = Types.Timing.Micro(dur);
542+
}
543+
events.push(event);
544+
return event;
545+
}
546+
}
547+
}
548+
549+
export namespace SamplesIntegrator {
550+
export const enum NativeGroups {
551+
COMPILE = 'Compile',
552+
PARSE = 'Parse',
553+
}
499554
}

front_end/models/trace/helpers/Trace.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -504,7 +504,7 @@ export function frameIDForEvent(event: Types.Events.Event): string|null {
504504
const DevToolsTimelineEventCategory = 'disabled-by-default-devtools.timeline';
505505
export function isTopLevelEvent(event: Types.Events.Event): boolean {
506506
if (event.name === 'JSRoot' && event.cat === 'toplevel') {
507-
// This is used in TimelineJSProfile to insert a fake event prior to the
507+
// This is used in createFakeTraceFromCpuProfile to insert a fake event prior to the
508508
// CPU Profile in order to ensure the trace isn't truncated. So if we see
509509
// this, we want to treat it as a top level event.
510510
// TODO(crbug.com/341234884): do we need this?

front_end/panels/timeline/TimelineLoader.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ export class TimelineLoader implements Common.StringOutputStream.OutputStream {
8888
loader.#traceIsCPUProfile = true;
8989

9090
try {
91-
const contents = Trace.Extras.TimelineJSProfile.TimelineJSProfileProcessor.createFakeTraceFromCpuProfile(
91+
const contents = Trace.Helpers.SamplesIntegrator.SamplesIntegrator.createFakeTraceFromCpuProfile(
9292
profile, Trace.Types.Events.ThreadID(1));
9393

9494
window.setTimeout(async () => {
@@ -261,7 +261,7 @@ export class TimelineLoader implements Common.StringOutputStream.OutputStream {
261261
}
262262

263263
#parseCPUProfileFormatFromFile(parsedTrace: Protocol.Profiler.Profile): void {
264-
const traceFile = Trace.Extras.TimelineJSProfile.TimelineJSProfileProcessor.createFakeTraceFromCpuProfile(
264+
const traceFile = Trace.Helpers.SamplesIntegrator.SamplesIntegrator.createFakeTraceFromCpuProfile(
265265
parsedTrace, Trace.Types.Events.ThreadID(1));
266266

267267
this.#collectEvents(traceFile.traceEvents);

0 commit comments

Comments
 (0)