Skip to content

Commit 703da59

Browse files
paulirishDevtools-frontend LUCI CQ
authored andcommitted
RPP: Support columns, pretty'd sources in Source's panel profile data
The profile data decorations in the Sources panel now apply to the '-' lines in Sources when a minified file is pretty-printed, and correctly for minified + source map'd scripts. The Sources panel displays profile data ("x ms") in the line gutters when a performance trace is active. However, we did not handle common edge cases involving pretty-formatted files (the `{}` button), nor did we handle source mapped content well (we totally ignored column information, and since most generated code is minified across few lines, profile data would be displayed far too coarsely). Now, we utilize the pretty -> original line mapping to create accurate profile data for the formatted code; and further we consider the column when mapping the profile data to a UILocation. Bug: 406012441 Change-Id: I84278eec4e778c62f8568e2c96353fa98be92b32 Bypass-Check-License: test fixture cannot have the license without breaking the test. (because isMinified) Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/6588993 Commit-Queue: Paul Irish <[email protected]> Reviewed-by: Connor Clark <[email protected]>
1 parent cd88537 commit 703da59

File tree

22 files changed

+365
-119
lines changed

22 files changed

+365
-119
lines changed

front_end/models/cpu_profile/CPUProfileDataModel.ts

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,15 @@ export class CPUProfileDataModel extends ProfileTreeModel {
5656
* for CPU profiles coming from traces.
5757
*/
5858
traceIds?: Record<string, number>;
59+
/**
60+
* Each item in the `lines` array contains the script line executing
61+
* when the sample in that array position was taken.
62+
*/
5963
lines?: number[];
64+
/**
65+
* Same as `lines` above, but with the script column.
66+
*/
67+
columns?: number[];
6068
totalHitCount: number;
6169
profileHead: CPUProfileNode;
6270
/**
@@ -90,14 +98,8 @@ export class CPUProfileDataModel extends ProfileTreeModel {
9098
this.traceIds = profile.traceIds;
9199
this.samples = profile.samples;
92100

93-
// Lines are available only in profiles coming from tracing.
94-
// Elements in the lines array have a 1 to 1 correspondence with
95-
// samples, by array position. They can be 1 or 0 and indicate if
96-
// there is line data for a given sample, i.e. if a given sample
97-
// needs to be included to calculate the line level execution time
98-
// data, which we show in the sources panel after recording a
99-
// profile.
100101
this.lines = profile.lines;
102+
this.columns = profile.columns;
101103
this.totalHitCount = 0;
102104
this.profileHead = this.translateProfileTree(profile.nodes);
103105
this.initialize(this.profileHead);
@@ -557,6 +559,7 @@ export type ExtendedProfileNode = Protocol.Profiler.ProfileNode&{parent?: number
557559
export type ExtendedProfile = Protocol.Profiler.Profile&{
558560
nodes: Protocol.Profiler.ProfileNode[] | ExtendedProfileNode[],
559561
lines?: number[],
562+
columns?: number[],
560563
/**
561564
* A sample can be manually collected with v8::CpuProfiler::collectSample.
562565
* When this is done an id (trace id) can be passed to the API to

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,8 @@ describeWithEnvironment('SamplesHandler', function() {
255255
assert.strictEqual(profileById.size, 1);
256256
const cpuProfileData = profileById.values().next().value as Trace.Handlers.ModelHandlers.Samples.ProfileData;
257257
const cpuProfile = cpuProfileData.rawProfile;
258-
assert.deepEqual(Object.keys(cpuProfile), ['startTime', 'endTime', 'nodes', 'samples', 'timeDeltas', 'lines']);
258+
assert.deepEqual(
259+
Object.keys(cpuProfile), ['startTime', 'endTime', 'nodes', 'samples', 'timeDeltas', 'lines', 'columns']);
259260
assert.lengthOf(cpuProfile.nodes, 153);
260261
assert.strictEqual(cpuProfile.startTime, 287510826176);
261262
assert.strictEqual(cpuProfile.endTime, 287510847633);

front_end/models/trace/handlers/SamplesHandler.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,9 +217,11 @@ export function handleEvent(event: Types.Events.Event): void {
217217

218218
const timeDeltas = event.args.data?.timeDeltas || [];
219219
const lines = event.args.data?.lines || Array(samples.length).fill(0);
220+
const columns = event.args.data?.columns || Array(samples.length).fill(0);
220221
cdpProfile.samples?.push(...samples);
221222
cdpProfile.timeDeltas?.push(...timeDeltas);
222223
cdpProfile.lines?.push(...lines);
224+
cdpProfile.columns?.push(...columns);
223225

224226
if (traceIds) {
225227
cdpProfile.traceIds ??= {};
@@ -270,6 +272,7 @@ function getOrCreatePreProcessedData(
270272
samples: [],
271273
timeDeltas: [],
272274
lines: [],
275+
columns: [],
273276
},
274277
profileId,
275278
}));

front_end/models/trace/types/TraceEvents.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,8 +174,9 @@ export interface ProfileChunk extends Sample {
174174
data?: ArgsData & {
175175
cpuProfile?: PartialProfile,
176176
timeDeltas?: Micro[],
177-
lines?: Micro[],
177+
lines?: number[],
178178
source?: ProfileSource,
179+
columns?: number[],
179180
},
180181
};
181182
}

front_end/models/workspace/UISourceCode.test.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -378,6 +378,38 @@ describe('UISourceCode', () => {
378378

379379
assert.isTrue(sutObject.sut.editDisabled());
380380
});
381+
382+
describe('formatChanged', () => {
383+
it('should remap performance decorations when the source is formatted', () => {
384+
const {sut} = setupMockedUISourceCode();
385+
const decorations = new Map<number, Map<number, number>>();
386+
const columnData = new Map<number, number>();
387+
columnData.set(10, 100); // column 10, value 100
388+
decorations.set(1, columnData); // line 1
389+
390+
sut.setDecorationData(Workspace.UISourceCode.DecoratorType.PERFORMANCE, decorations);
391+
392+
const format = {
393+
originalToFormatted: (lineNumber: number, columnNumber?: number) => {
394+
if (lineNumber === 0 && columnNumber === 9) {
395+
return [4, 1]; // new line 5, new column 2 (0-based)
396+
}
397+
return [lineNumber, columnNumber || 0];
398+
},
399+
};
400+
401+
sut.formatChanged(format);
402+
403+
const newDecorations = sut.getDecorationData(Workspace.UISourceCode.DecoratorType.PERFORMANCE);
404+
assert.isDefined(newDecorations);
405+
406+
const newLineData = newDecorations.get(5); // check line 5 (1-based)
407+
assert.isDefined(newLineData);
408+
409+
const newData = newLineData.get(2); // check column 2 (1-based)
410+
assert.strictEqual(newData, 100);
411+
});
412+
});
381413
});
382414

383415
describe('UILocation', () => {

front_end/models/workspace/UISourceCode.ts

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,9 @@ export class UISourceCode extends Common.ObjectWrapper.ObjectWrapper<EventTypes>
3535
#contentType: Common.ResourceType.ResourceType;
3636
#requestContentPromise: Promise<TextUtils.ContentData.ContentDataOrError>|null = null;
3737
#decorations = new Map<string, any>();
38+
#formattedDecorations = new Map<string, any>();
3839
#hasCommits = false;
40+
#prettied = false;
3941
#messages: Set<Message>|null = null;
4042
#content: TextUtils.ContentData.ContentDataOrError|null = null;
4143
#forceLoadOnCheckContent = false;
@@ -497,6 +499,9 @@ export class UISourceCode extends Common.ObjectWrapper.ObjectWrapper<EventTypes>
497499
}
498500

499501
getDecorationData(type: string): any {
502+
if (this.#prettied && this.#formattedDecorations.get(type)) {
503+
return this.#formattedDecorations.get(type);
504+
}
500505
return this.#decorations.get(type);
501506
}
502507

@@ -508,6 +513,45 @@ export class UISourceCode extends Common.ObjectWrapper.ObjectWrapper<EventTypes>
508513
return this.#disableEdit;
509514
}
510515

516+
formatChanged(format: {originalToFormatted(lineNumber: number, columnNumber?: number): number[]}|null): void {
517+
if (this.#prettied === Boolean(format)) {
518+
return;
519+
}
520+
this.#prettied = Boolean(format);
521+
if (!format) {
522+
this.dispatchEventToListeners(Events.DecorationChanged, DecoratorType.PERFORMANCE);
523+
return;
524+
}
525+
const performanceDecorations = this.#decorations.get(DecoratorType.PERFORMANCE);
526+
if (!performanceDecorations) {
527+
return;
528+
}
529+
let formattedPerformanceDecorations: Map<number, Map<number, number>> =
530+
this.#formattedDecorations.get(DecoratorType.PERFORMANCE);
531+
if (!formattedPerformanceDecorations) {
532+
formattedPerformanceDecorations = new Map<number, Map<number, number>>();
533+
this.#formattedDecorations.set(DecoratorType.PERFORMANCE, formattedPerformanceDecorations);
534+
} else {
535+
formattedPerformanceDecorations.clear();
536+
}
537+
538+
for (const [lineNumber, columnData] of performanceDecorations) {
539+
for (const [columnNumber, data] of columnData) {
540+
const [formattedLineNumber, formattedColumnNumber] =
541+
format.originalToFormatted(lineNumber - 1, columnNumber - 1);
542+
const oneBasedFormattedLineNumber = formattedLineNumber + 1;
543+
const oneBasedFormattedColumnNumber = formattedColumnNumber + 1;
544+
let lineData = formattedPerformanceDecorations.get(oneBasedFormattedLineNumber);
545+
if (!lineData) {
546+
lineData = new Map();
547+
formattedPerformanceDecorations.set(oneBasedFormattedLineNumber, lineData);
548+
}
549+
lineData.set(oneBasedFormattedColumnNumber, (lineData.get(oneBasedFormattedColumnNumber) || 0) + data);
550+
}
551+
}
552+
this.dispatchEventToListeners(Events.DecorationChanged, 'performance');
553+
}
554+
511555
isIgnoreListed(): boolean {
512556
return IgnoreListManager.instance().isUserOrSourceMapIgnoreListedUISourceCode(this);
513557
}
@@ -696,3 +740,9 @@ export class UISourceCodeMetadata {
696740
this.contentSize = contentSize;
697741
}
698742
}
743+
744+
export const enum DecoratorType {
745+
PERFORMANCE = 'performance',
746+
MEMORY = 'memory',
747+
COVERAGE = 'coverage',
748+
}

front_end/panels/sources/CoveragePlugin.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,10 +7,10 @@ import * as i18n from '../../core/i18n/i18n.js';
77
import type * as Platform from '../../core/platform/platform.js';
88
import * as SDK from '../../core/sdk/sdk.js';
99
import * as TextUtils from '../../models/text_utils/text_utils.js';
10-
import type * as Workspace from '../../models/workspace/workspace.js';
10+
import * as Workspace from '../../models/workspace/workspace.js';
1111
import * as CodeMirror from '../../third_party/codemirror.next/codemirror.next.js';
1212
import type * as TextEditor from '../../ui/components/text_editor/text_editor.js';
13-
import * as SourceFrame from '../../ui/legacy/components/source_frame/source_frame.js';
13+
import type * as SourceFrame from '../../ui/legacy/components/source_frame/source_frame.js';
1414
import * as UI from '../../ui/legacy/legacy.js';
1515
import * as Coverage from '../coverage/coverage.js';
1616

@@ -123,7 +123,7 @@ export class CoveragePlugin extends Plugin {
123123
}
124124

125125
private getCoverageManager(): Coverage.CoverageDecorationManager.CoverageDecorationManager|undefined {
126-
return this.uiSourceCode.getDecorationData(SourceFrame.SourceFrame.DecoratorType.COVERAGE);
126+
return this.uiSourceCode.getDecorationData(Workspace.UISourceCode.DecoratorType.COVERAGE);
127127
}
128128

129129
override editorInitialized(editor: TextEditor.TextEditor.TextEditor): void {
@@ -132,9 +132,9 @@ export class CoveragePlugin extends Plugin {
132132
}
133133
}
134134

135-
override decorationChanged(type: SourceFrame.SourceFrame.DecoratorType, editor: TextEditor.TextEditor.TextEditor):
135+
override decorationChanged(type: Workspace.UISourceCode.DecoratorType, editor: TextEditor.TextEditor.TextEditor):
136136
void {
137-
if (type === SourceFrame.SourceFrame.DecoratorType.COVERAGE) {
137+
if (type === Workspace.UISourceCode.DecoratorType.COVERAGE) {
138138
this.startDecoUpdate(editor);
139139
}
140140
}

front_end/panels/sources/Plugin.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ export class Plugin {
4141
void {
4242
}
4343

44-
decorationChanged(_type: SourceFrame.SourceFrame.DecoratorType, _editor: TextEditor.TextEditor.TextEditor): void {
44+
decorationChanged(_type: Workspace.UISourceCode.DecoratorType, _editor: TextEditor.TextEditor.TextEditor): void {
4545
}
4646

4747
editorExtension(): CodeMirror.Extension {

front_end/panels/sources/ProfilePlugin.ts

Lines changed: 22 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,9 @@
55

66
import * as i18n from '../../core/i18n/i18n.js';
77
import * as Platform from '../../core/platform/platform.js';
8-
import type * as Workspace from '../../models/workspace/workspace.js';
8+
import * as Workspace from '../../models/workspace/workspace.js';
99
import * as CodeMirror from '../../third_party/codemirror.next/codemirror.next.js';
1010
import type * as TextEditor from '../../ui/components/text_editor/text_editor.js';
11-
import * as SourceFrame from '../../ui/legacy/components/source_frame/source_frame.js';
1211

1312
import {Plugin} from './Plugin.js';
1413

@@ -90,22 +89,31 @@ class PerformanceMarker extends CodeMirror.GutterMarker {
9089
}
9190

9291
function markersFromProfileData(
93-
map: Map<number, number>, state: CodeMirror.EditorState,
94-
type: SourceFrame.SourceFrame.DecoratorType): CodeMirror.RangeSet<CodeMirror.GutterMarker> {
95-
const markerType = type === SourceFrame.SourceFrame.DecoratorType.PERFORMANCE ? PerformanceMarker : MemoryMarker;
92+
map: Map<number, number>|Map<number, Map<number, number>>, state: CodeMirror.EditorState,
93+
type: Workspace.UISourceCode.DecoratorType): CodeMirror.RangeSet<CodeMirror.GutterMarker> {
94+
const markerType = type === Workspace.UISourceCode.DecoratorType.PERFORMANCE ? PerformanceMarker : MemoryMarker;
9695
const markers: Array<CodeMirror.Range<CodeMirror.GutterMarker>> = [];
96+
const aggregatedByLine = new Map<number, number>();
9797
for (const [line, value] of map) {
9898
if (line <= state.doc.lines) {
99-
const {from} = state.doc.line(line);
100-
markers.push(new markerType(value).range(from));
99+
if (value instanceof Map) {
100+
for (const [, data] of value) {
101+
aggregatedByLine.set(line, (aggregatedByLine.get(line) || 0) + data);
102+
}
103+
continue;
104+
}
105+
aggregatedByLine.set(line, value);
101106
}
102107
}
108+
for (const [line, value] of aggregatedByLine) {
109+
const {from} = state.doc.line(line);
110+
markers.push(new markerType(value).range(from));
111+
}
103112
return CodeMirror.RangeSet.of(markers, true);
104113
}
105114

106-
const makeLineLevelProfilePlugin = (type: SourceFrame.SourceFrame.DecoratorType): typeof Plugin =>
107-
class extends Plugin {
108-
updateEffect = CodeMirror.StateEffect.define<Map<number, number>>();
115+
const makeLineLevelProfilePlugin = (type: Workspace.UISourceCode.DecoratorType): typeof Plugin => class extends Plugin {
116+
updateEffect = CodeMirror.StateEffect.define<Map<number, number>|Map<number, Map<number, number>>>();
109117
field: CodeMirror.StateField<CodeMirror.RangeSet<CodeMirror.GutterMarker>>;
110118
gutter: CodeMirror.Extension;
111119
compartment: CodeMirror.Compartment = new CodeMirror.Compartment();
@@ -134,7 +142,7 @@ const makeLineLevelProfilePlugin = (type: SourceFrame.SourceFrame.DecoratorType)
134142
return uiSourceCode.contentType().hasScripts();
135143
}
136144

137-
private getLineMap(): Map<number, number>|undefined {
145+
private getLineMap(): Map<number, number>|Map<number, Map<number, number>>|undefined {
138146
return this.uiSourceCode.getDecorationData(type);
139147
}
140148

@@ -144,7 +152,7 @@ const makeLineLevelProfilePlugin = (type: SourceFrame.SourceFrame.DecoratorType)
144152
!map ? [] : [this.field.init(state => markersFromProfileData(map, state, type)), this.gutter, theme]);
145153
}
146154

147-
override decorationChanged(type: SourceFrame.SourceFrame.DecoratorType, editor: TextEditor.TextEditor.TextEditor):
155+
override decorationChanged(type: Workspace.UISourceCode.DecoratorType, editor: TextEditor.TextEditor.TextEditor):
148156
void {
149157
const installed = Boolean(editor.state.field(this.field, false));
150158
const map = this.getLineMap();
@@ -189,6 +197,6 @@ const theme = CodeMirror.EditorView.baseTheme({
189197
},
190198
});
191199

192-
export const MemoryProfilePlugin = makeLineLevelProfilePlugin(SourceFrame.SourceFrame.DecoratorType.MEMORY);
200+
export const MemoryProfilePlugin = makeLineLevelProfilePlugin(Workspace.UISourceCode.DecoratorType.MEMORY);
193201

194-
export const PerformanceProfilePlugin = makeLineLevelProfilePlugin(SourceFrame.SourceFrame.DecoratorType.PERFORMANCE);
202+
export const PerformanceProfilePlugin = makeLineLevelProfilePlugin(Workspace.UISourceCode.DecoratorType.PERFORMANCE);

front_end/panels/sources/UISourceCodeFrame.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -238,6 +238,7 @@ export class UISourceCodeFrame extends Common.ObjectWrapper
238238
}
239239

240240
override async setContent(content: string): Promise<void> {
241+
this.#uiSourceCode.formatChanged(this.formattedMap);
241242
this.disposePlugins();
242243
this.loadPlugins();
243244
await super.setContent(content);
@@ -423,7 +424,7 @@ export class UISourceCodeFrame extends Common.ObjectWrapper
423424

424425
private onDecorationChanged(event: Common.EventTarget.EventTargetEvent<string>): void {
425426
for (const plugin of this.plugins) {
426-
plugin.decorationChanged(event.data as SourceFrame.SourceFrame.DecoratorType, this.textEditor);
427+
plugin.decorationChanged(event.data as Workspace.UISourceCode.DecoratorType, this.textEditor);
427428
}
428429
}
429430

0 commit comments

Comments
 (0)