Skip to content

Commit 30d74b6

Browse files
committed
Group stack-based and instant markers at the top of the marker chart.
1 parent d3c48d6 commit 30d74b6

File tree

2 files changed

+73
-39
lines changed

2 files changed

+73
-39
lines changed

src/profile-logic/marker-timing.ts

Lines changed: 72 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,31 @@ import type {
88
MarkerIndex,
99
MarkerTiming,
1010
MarkerTimingAndBuckets,
11+
MarkerSchemaByName,
1112
} from 'firefox-profiler/types';
1213

14+
import { getSchemaFromMarker } from './marker-schema';
15+
1316
// Arbitrarily set an upper limit for adding marker depths, avoiding very long
1417
// overlapping marker timings.
1518
const MAX_STACKING_DEPTH = 300;
1619

20+
function _createEmptyTiming(
21+
name: string,
22+
bucket: string,
23+
instantOnly: boolean
24+
): MarkerTiming {
25+
return {
26+
start: [],
27+
end: [],
28+
index: [],
29+
name,
30+
bucket,
31+
instantOnly,
32+
length: 0,
33+
};
34+
}
35+
1736
/**
1837
* This function computes the timing information for laying out the markers in the
1938
* MarkerChart component. Each marker is put into a single row based on its name. In
@@ -86,13 +105,15 @@ export function getMarkerTiming(
86105
markerIndexes: MarkerIndex[],
87106
// Categories can be null for things like Network Markers, where we don't care to
88107
// break things up by category.
89-
categories: CategoryList | null
108+
categories?: CategoryList,
109+
markerSchemaByName?: MarkerSchemaByName
90110
): MarkerTiming[] {
91-
// Each marker type will have it's own timing information, later collapse these into
111+
// Each marker type will have its own timing information, later collapse these into
92112
// a single array.
93113
const intervalMarkerTimingsMap: Map<string, MarkerTiming[]> = new Map();
94114
// Instant markers are on separate lines.
95115
const instantMarkerTimingsMap: Map<string, MarkerTiming> = new Map();
116+
const stackBasedMarkerTimings: MarkerTiming[] = [];
96117

97118
// Go through all of the markers.
98119
for (const markerIndex of markerIndexes) {
@@ -109,52 +130,60 @@ export function getMarkerTiming(
109130
markerTiming.length++;
110131
};
111132

112-
const bucketName = categories ? categories[marker.category].name : 'None';
133+
const schema = markerSchemaByName
134+
? getSchemaFromMarker(markerSchemaByName, marker.data)
135+
: null;
136+
let isStackBased = schema !== null && schema.isStackBased === true;
137+
if (marker.end === null) {
138+
// XXXmstange let's see how we like this
139+
isStackBased = true;
140+
}
141+
142+
// eslint-disable-next-line no-nested-ternary
143+
const bucketName = isStackBased
144+
? 'Stack'
145+
: categories
146+
? categories[marker.category].name
147+
: 'None';
113148

114149
// We want to group all network requests in the same line. Indeed they all
115150
// have different names and they'd end up with one single request in each
116151
// line without this special handling.
117-
const markerLineName =
118-
marker.data && marker.data.type === 'Network'
152+
// eslint-disable-next-line no-nested-ternary
153+
const markerLineName = isStackBased
154+
? 'Stack'
155+
: marker.data && marker.data.type === 'Network'
119156
? 'Network Requests'
120157
: marker.name;
121158

122-
const emptyTiming = ({
123-
instantOnly,
124-
}: {
125-
instantOnly: boolean;
126-
}): MarkerTiming => ({
127-
start: [],
128-
end: [],
129-
index: [],
130-
name: markerLineName,
131-
bucket: bucketName,
132-
instantOnly,
133-
length: 0,
134-
});
135-
136-
if (marker.end === null) {
159+
if (marker.end === null && !isStackBased) {
137160
// This is an instant marker.
138161
let instantMarkerTiming = instantMarkerTimingsMap.get(markerLineName);
139162
if (!instantMarkerTiming) {
140-
instantMarkerTiming = emptyTiming({ instantOnly: true });
163+
instantMarkerTiming = _createEmptyTiming(
164+
markerLineName,
165+
bucketName,
166+
true
167+
);
141168
instantMarkerTimingsMap.set(markerLineName, instantMarkerTiming);
142169
}
143170
addCurrentMarkerToMarkerTiming(instantMarkerTiming);
144171
continue;
145172
}
146173

147174
// This is an interval marker.
148-
let markerTimingsForName = intervalMarkerTimingsMap.get(markerLineName);
149-
if (markerTimingsForName === undefined) {
150-
markerTimingsForName = [];
151-
intervalMarkerTimingsMap.set(markerLineName, markerTimingsForName);
175+
let timingRows = isStackBased
176+
? stackBasedMarkerTimings
177+
: intervalMarkerTimingsMap.get(markerLineName);
178+
if (timingRows === undefined) {
179+
timingRows = [];
180+
intervalMarkerTimingsMap.set(markerLineName, timingRows);
152181
}
153182

154183
// Find the first row where the new marker fits.
155184
// Since the markers are sorted, look at the last added marker in this row. If
156185
// the new marker fits, go ahead and insert it.
157-
const foundMarkerTiming = markerTimingsForName.find(
186+
const foundMarkerTiming = timingRows.find(
158187
(markerTiming) =>
159188
markerTiming.end[markerTiming.length - 1] <= marker.start
160189
);
@@ -163,30 +192,34 @@ export function getMarkerTiming(
163192
addCurrentMarkerToMarkerTiming(foundMarkerTiming);
164193
continue;
165194
}
166-
167-
if (markerTimingsForName.length >= MAX_STACKING_DEPTH) {
195+
if (timingRows.length >= MAX_STACKING_DEPTH) {
168196
// There are too many markers stacked around the same time already, let's
169197
// ignore this marker.
170198
continue;
171199
}
172200

173201
// Otherwise, let's add a new row!
174-
const newTiming = emptyTiming({ instantOnly: false });
202+
const newTiming = _createEmptyTiming(markerLineName, bucketName, false);
175203
addCurrentMarkerToMarkerTiming(newTiming);
176-
markerTimingsForName.push(newTiming);
204+
timingRows.push(newTiming);
177205
continue;
178206
}
179-
180207
// Flatten out the maps into a single array.
181208
// One item in this array is one line in the drawn canvas.
182-
const allMarkerTimings = [...instantMarkerTimingsMap.values()].concat(
183-
...intervalMarkerTimingsMap.values()
184-
);
209+
const allMarkerTimings = [...instantMarkerTimingsMap.values()]
210+
.concat(...intervalMarkerTimingsMap.values())
211+
.concat(...stackBasedMarkerTimings);
185212

186213
// Sort all the marker timings in place, first by the bucket, then by their names.
187214
allMarkerTimings.sort((a, b) => {
188215
if (a.bucket !== b.bucket) {
189216
// Sort by buckets first.
217+
if (a.bucket === 'Stack') {
218+
return -1;
219+
}
220+
if (b.bucket === 'Stack') {
221+
return 1;
222+
}
190223
// Show the 'Test' category first. Test markers are almost guaranteed to
191224
// be the most relevant when they exist.
192225
if (a.bucket === 'Test') {
@@ -286,16 +319,18 @@ export function getMarkerTiming(
286319
* ]
287320
*/
288321
export function getMarkerTimingAndBuckets(
289-
getMarker: (param: MarkerIndex) => Marker,
322+
getMarker: (marker: MarkerIndex) => Marker,
290323
markerIndexes: MarkerIndex[],
291324
// Categories can be null for things like Network Markers, where we don't care to
292325
// break things up by category.
293-
categories: CategoryList | null
326+
categories?: CategoryList,
327+
markerSchemaByName?: MarkerSchemaByName
294328
): MarkerTimingAndBuckets {
295329
const allMarkerTimings = getMarkerTiming(
296330
getMarker,
297331
markerIndexes,
298-
categories
332+
categories,
333+
markerSchemaByName
299334
);
300335

301336
// Interleave the bucket names in between the marker timings.

src/selectors/per-thread/markers.ts

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -454,6 +454,7 @@ export function getMarkerSelectorsPerThread(
454454
getMarkerGetter,
455455
getMarkerChartMarkerIndexes,
456456
ProfileSelectors.getCategories,
457+
ProfileSelectors.getMarkerSchemaByName,
457458
MarkerTimingLogic.getMarkerTimingAndBuckets
458459
);
459460

@@ -506,7 +507,6 @@ export function getMarkerSelectorsPerThread(
506507
const getNetworkTrackTiming: Selector<MarkerTiming[]> = createSelector(
507508
getMarkerGetter,
508509
getNetworkMarkerIndexes,
509-
() => null,
510510
MarkerTimingLogic.getMarkerTiming
511511
);
512512

@@ -517,7 +517,6 @@ export function getMarkerSelectorsPerThread(
517517
const getUserTimingMarkerTiming: Selector<MarkerTiming[]> = createSelector(
518518
getMarkerGetter,
519519
getUserTimingMarkerIndexes,
520-
() => null,
521520
MarkerTimingLogic.getMarkerTiming
522521
);
523522

0 commit comments

Comments
 (0)