1692921 - Markers 2.0 timings output null timestamps as 0.0 instead of JSON null, but front-end is not expecting zeroes

Status: RESOLVED MOVED

(Core :: Gecko Profiler, defect, P2)

Description

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

When markers were implemented with MarkerPayload-derived classes, some payloads had optional timing information, which could be output as JSON null for unspecified timestamps.
The front-end relies on these nulls when gauging the total visible range.

But since Markers 2.0 introduced better timing information (or maybe even before?), we now output unspecified timestamps as 0 (zero). This effectively forces all profiles to start at 0, which can be especially bad when most timestamps are later, e.g.: https://share.firefox.dev/2OCCOuF (the data is right at the end, around 27,000 seconds, but the range starts at 0 -- Thanks @Bas for reporting this issue.)

In the code, MarkerTiming::GetStartTime() and GetEndTime() return 0.0 for null timestamps, instead we should return Maybe<double>, so that DeserializeAfterKindAndStream could output nulls where appropriate.

It may also be possible for the front-end to fix "bad" profiles (from when this started until this fix lands), by detecting and ignoring unwanted zeroes.

Comment 1

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Interestingly, a comment in MarkerTiming::timeStampToDouble says "The Phase lets us know not to use this [zero] value".

Greg, I think I stole this C++ code from your original prototype(?)
If you remember, did you imagine that the front-end would use the phase and discard these zeroes?
Looking at this front-end patch from issue #2626, the title "Teach getTimeRangeForThread about phased markers" would suggest that, but the implementation there doesn't seem to account for it. 🤔

So the cause and solution may not be as I initially thought, I'll update this bug title&priorities, and discuss this with the front-end team...

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Note: This only affects the front-end when there are no samples in the profile (otherwise only samples are used to gauge the time range).

Comment 3

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

It seems most of the front-end was already using marker phases as expected, except for the time range computation.
-> Moved to https://github.com/firefox-devtools/profiler/issues/3172