1929070 - Interval start/end markers cannot be correctly matched in the profiler frontend without additional information

Status: NEW

(Core :: Gecko Profiler, defect, P3)

Description

[Adam Brouwers-Harries [:aabh] [he/him] ⌚GMT]

The MarkerOptions MarkerTiming is very useful for displaying a timespan associated with a given activity. Sometimes, however, we may need to record the start and end of the activity separately, in two different markers. When we do so, we use a push/pop style algorithim to match start and end markers within the profiler frontend.

For many applications, this works well, but causes issues when code creates markers where the interval start/end are not equivalently push/pop. An excellent example of this is the profiler markers added while working on Bug 1919353. In this situation, we have some Glean code that starts and stops three timers. As the markers recorded are namespaced to the metric, we cannot distinguish between the timers, and so the frontend incorrectly matches timer start/stops, as seen in this profile: https://share.firefox.dev/4evLo4V

The glean code adds markers in the following order (where A_s means an interval starting marker for timer A, and A_e means an intervalending marker for timer A):

A_s  B_s  C_s   A_e  B_e  C_e

In the frontend, we would expect to see interval markers that look like:

A--------A
    B--------B
        C---------C

However, the frontend is unable to distinguish between the markers for A, B, and C, and instead derives the following from the profile:

M_s  M_s  M_s   M_e  M_e  M_e

The frontend then runs the stack-based algorithm to pair the markers, incorrectly matching A_e with C_s, and A_s with C_e, meaning that we end up with:

M-----------------M
    M----------M
        MM

and with the start/end labeled:

A---------------C
    B--------B
        CA

This is all due to the fact that when recording an interval we have no way to distinguish one interval from another. In the Glean case above, we "know" within the code which interval corresponds to which timer, but we are unable to pass that information down into the profiler while recording a marker. This bug proposes that we add a new (optional) id field to the MarkerTiming struct, which would allow code running within Firefox to pass relevant information to the profiler. At a later date, the profiler frontend can then be modified to adapt the "pairing" algorithm to use this data.

Comment 1

[Adam Brouwers-Harries [:aabh] [he/him] ⌚GMT]

Attachment: [WIP] Bug 1929070 - Add an optional index to MarkerTiming so that the frontend can correctly match start/end intervals r?julienw,canaltinova,mstange,florian

Comment 2

[BugBot [:suhaib / :marco/ :calixte]]

The severity field is not set for this bug.
:canova, could you have a look please?

For more information, please visit BugBot documentation.

Comment 3

[Adam Brouwers-Harries [:aabh] [he/him] ⌚GMT]

Following discussion with the profiler team, we've decided that the original solution proposed in this bug isn't the best technical solution, even though we still need to find some way to match start/end markers.

• This approach will introduce overhead to all markers, in the form of another mandatory number, as the markertiming section of a marker is completely static, and optional values are not possible. This would likely increase profile sizes.
• On the frontend, we will end up creating a (mostly empty) array of indices when we transform markers from AoS to SoA form. This would incurr a large memory overhead. We may be able to get around this by using a sparse array, however serialising that to JSON in the processed format would re-introduce the "zeros", meaning that we lose the benefits of sparsity.