1640969 - Add startTime, and endTime to the marker struct

Status: RESOLVED FIXED

(Core :: Gecko Profiler, enhancement, P2)

Description

[Greg Tatum [:gregtatum] (REO Fx132)]

Markers currently have a time field in the output JSON. Many markers in the payloads also include a startTime and endTime. These various fields end up being ambiguous in how they are used.

For the output marker table we should remove the time field, and add startTime and endTime. Each of these properties can then be null or number.

This will also need a profiler upgrader for the front-end which will need to land first.

Comment 1

[Greg Tatum [:gregtatum] (REO Fx132)]

Attachment: Bug 1640969 - Add Instant, Interval, IntervalStart, IntervalEnd markers to the profiler; r?gerald

This patch is the first step to our so-called "Markers 2.0" work. It's attempting to consolidate
some ad-hoc practices in the profiler into more solid practices. This is the first step to getting
the Gecko profiler to produce a format that has the timing information applied to the serialized
marker tuple, rather than a mix in the tuple and the payload.

Comment 2

[Greg Tatum [:gregtatum] (REO Fx132)]

Attachment: Bug 1640969 - Remove startTime and endTime from most marker payloads; r?gerald

Depends on D78990

Comment 3

[Greg Tatum [:gregtatum] (REO Fx132)]

Attachment: Bug 1640969 - Update profiler gtests for startTime and endTime in markers; r?gerald

Depends on D84346

Comment 4

[Greg Tatum [:gregtatum] (REO Fx132)]

The try run looks pretty good now: https://treeherder.mozilla.org/#/jobs?repo=try&revision=a4ddff5dc623afa40e67d1551a9d3fd4db1849cb

Comment 5

[Greg Tatum [:gregtatum] (REO Fx132)]

The front-end merged, and once deployed this can be merged as well.

https://github.com/firefox-devtools/profiler/pull/2660

I'll be away from the office for a bit, so Nazim can help coordinate that, as he can deploy and was the reviewer for the front-end piece.

Comment 6

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

Attachment: Bug 1640969 - StoreMarker() doesn't need the TimeStamp argument anymore; r?canaltinova

Depends on D84595

Comment 7

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

Attachment: Bug 1640969 - Fix StoreMarkers calls from Android code; r?canaltinova

Depends on D85374

Comment 8

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

Attachment: Bug 1640969 - Ensure Tracing markers always have correct times and kind; r?canaltinova

Depends on D85375

Comment 9

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

Attachment: Bug 1640969 - Fix gtests for tracing markers, and add PHASE tests; r=canaltinova

Depends on D85376

Comment 10

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/b481d63c4ce1
Add Instant, Interval, IntervalStart, IntervalEnd markers to the profiler; r=gerald
https://hg.mozilla.org/integration/autoland/rev/702818e2e923
Remove startTime and endTime from most marker payloads; r=gerald
https://hg.mozilla.org/integration/autoland/rev/552a50d291d8
Update profiler gtests for startTime and endTime in markers; r=gerald
https://hg.mozilla.org/integration/autoland/rev/39429089fcf6
StoreMarker() doesn't need the TimeStamp argument anymore; r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/0ced6d9dea69
Fix StoreMarkers calls from Android code; r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/68d8385968bb
Ensure Tracing markers always have correct times and kind; r=canaltinova
https://hg.mozilla.org/integration/autoland/rev/baa47c1d4947
Fix gtests for tracing markers, and add PHASE tests; r=canaltinova

Comment 11

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/b481d63c4ce1
https://hg.mozilla.org/mozilla-central/rev/702818e2e923
https://hg.mozilla.org/mozilla-central/rev/552a50d291d8
https://hg.mozilla.org/mozilla-central/rev/39429089fcf6
https://hg.mozilla.org/mozilla-central/rev/0ced6d9dea69
https://hg.mozilla.org/mozilla-central/rev/68d8385968bb
https://hg.mozilla.org/mozilla-central/rev/baa47c1d4947