Skip to content

Commit 04b386f

Browse files
committed
Group stack-based and instant markers at the top of the marker chart.
1 parent fe24e53 commit 04b386f

File tree

2 files changed

+72
-30
lines changed

2 files changed

+72
-30
lines changed

src/profile-logic/marker-timing.js

+71-30
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
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,48 +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.name, 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 = ({ instantOnly }): MarkerTiming => ({
123-
start: [],
124-
end: [],
125-
index: [],
126-
name: markerLineName,
127-
bucket: bucketName,
128-
instantOnly,
129-
length: 0,
130-
});
131-
132-
if (marker.end === null) {
159+
if (marker.end === null && !isStackBased) {
133160
// This is an instant marker.
134161
let instantMarkerTiming = instantMarkerTimingsMap.get(markerLineName);
135162
if (!instantMarkerTiming) {
136-
instantMarkerTiming = emptyTiming({ instantOnly: true });
163+
instantMarkerTiming = _createEmptyTiming(
164+
markerLineName,
165+
bucketName,
166+
true
167+
);
137168
instantMarkerTimingsMap.set(markerLineName, instantMarkerTiming);
138169
}
139170
addCurrentMarkerToMarkerTiming(instantMarkerTiming);
140171
continue;
141172
}
142173

143174
// This is an interval marker.
144-
let markerTimingsForName = intervalMarkerTimingsMap.get(markerLineName);
145-
if (markerTimingsForName === undefined) {
146-
markerTimingsForName = [];
147-
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);
148181
}
149182

150183
// Find the first row where the new marker fits.
151184
// Since the markers are sorted, look at the last added marker in this row. If
152185
// the new marker fits, go ahead and insert it.
153-
const foundMarkerTiming = markerTimingsForName.find(
186+
const foundMarkerTiming = timingRows.find(
154187
(markerTiming) =>
155188
markerTiming.end[markerTiming.length - 1] <= marker.start
156189
);
@@ -160,29 +193,35 @@ export function getMarkerTiming(
160193
continue;
161194
}
162195

163-
if (markerTimingsForName.length >= MAX_STACKING_DEPTH) {
196+
if (timingRows.length >= MAX_STACKING_DEPTH) {
164197
// There are too many markers stacked around the same time already, let's
165198
// ignore this marker.
166199
continue;
167200
}
168201

169202
// Otherwise, let's add a new row!
170-
const newTiming = emptyTiming({ instantOnly: false });
203+
const newTiming = _createEmptyTiming(markerLineName, bucketName, false);
171204
addCurrentMarkerToMarkerTiming(newTiming);
172-
markerTimingsForName.push(newTiming);
205+
timingRows.push(newTiming);
173206
continue;
174207
}
175208

176209
// Flatten out the maps into a single array.
177210
// One item in this array is one line in the drawn canvas.
178-
const allMarkerTimings = [...instantMarkerTimingsMap.values()].concat(
179-
...intervalMarkerTimingsMap.values()
180-
);
211+
const allMarkerTimings = [...instantMarkerTimingsMap.values()]
212+
.concat(...intervalMarkerTimingsMap.values())
213+
.concat(...stackBasedMarkerTimings);
181214

182215
// Sort all the marker timings in place, first by the bucket, then by their names.
183216
allMarkerTimings.sort((a, b) => {
184217
if (a.bucket !== b.bucket) {
185218
// Sort by buckets first.
219+
if (a.bucket === 'Stack') {
220+
return -1;
221+
}
222+
if (b.bucket === 'Stack') {
223+
return 1;
224+
}
186225
// Show the 'Test' category first. Test markers are almost guaranteed to
187226
// be the most relevant when they exist.
188227
if (a.bucket === 'Test') {
@@ -286,12 +325,14 @@ export function getMarkerTimingAndBuckets(
286325
markerIndexes: MarkerIndex[],
287326
// Categories can be null for things like Network Markers, where we don't care to
288327
// break things up by category.
289-
categories: ?CategoryList
328+
categories: ?CategoryList,
329+
markerSchemaByName: ?MarkerSchemaByName
290330
): MarkerTimingAndBuckets {
291331
const allMarkerTimings = getMarkerTiming(
292332
getMarker,
293333
markerIndexes,
294-
categories
334+
categories,
335+
markerSchemaByName
295336
);
296337

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

src/selectors/per-thread/markers.js

+1
Original file line numberDiff line numberDiff line change
@@ -488,6 +488,7 @@ export function getMarkerSelectorsPerThread(
488488
getMarkerGetter,
489489
getMarkerChartMarkerIndexes,
490490
ProfileSelectors.getCategories,
491+
ProfileSelectors.getMarkerSchemaByName,
491492
MarkerTimingLogic.getMarkerTimingAndBuckets
492493
);
493494

0 commit comments

Comments
 (0)