firefox-devtools / firefox-devtools/profiler
Do we rely on the order of markers anywhere?
- Dominant language
- TypeScript
- Stars
- 1.5k
- Forks
- 491
- Avg merge
- 3d 46m
- Merged PRs (30d)
- 27
Description
Do we rely on the order of markers anywhere? The answer to this question might just be "no", and that would actually make me very happy. But I'm a little hesitant to just assume that the ordering is irrelevant.
Background:
The marker table has a `startTime` and an `endTime` column. There are no comments on the order of timestamps in these columns: https://github.com/firefox-devtools/profiler/blob/af62509321b9610b7acacb7e4094eccd99945a35/src/types/profile.js#L257-L258
A long time ago, the marker table only had a `time` column, and, starting with version 1, that column was guaranteed to be sorted:
https://github.com/firefox-devtools/profiler/blob/af62509321b9610b7acacb7e4094eccd99945a35/src/profile-logic/processed-profile-versioning.js#L256-L261
But in version 30 / 31, we changed the marker table to have `startTime` and `endTime` columns:
https://github.com/firefox-devtools/profiler/blob/af62509321b9610b7acacb7e4094eccd99945a35/src/profile-logic/processed-profile-versioning.js#L1354-L1355
There are no comments about any ordering requirements on these new columns. And maybe there are no such requirements!
I'm just filing this issue so that we can audit the code and be sure, and maybe to fix some comments.
I found one comment that says "markers are ordered by start time" in the tooltip test, but it might be outdated:
https://github.com/firefox-devtools/profiler/blob/af62509321b9610b7acacb7e4094eccd99945a35/src/test/components/TooltipMarker.test.js#L528-L529
I checked the implementation of the "drop samples outside markers" transform, and it doesn't rely on an order; this call to `canonicalizeRangeSet` does the ordering:
https://github.com/firefox-devtools/profiler/blob/af62509321b9610b7acacb7e4094eccd99945a35/src/profile-logic/transforms.js#L1693
┆Issue is synchronized with this [Jira Task](https://mozilla-hub.atlassian.net/browse/FP-801)
Contributor guide
Assessment
This issue has not been assessed yet.