1637684 - Extend ChromeUtils.addProfilerMarker to have optional payload for additional meta data

Status: NEW

(Core :: Gecko Profiler, enhancement, P3)

Description

[:Harald Kirschner :digitarald]

Bug 1634467 and bug 1634466 are capturing work to add markers to DevTools to make diagnosis of user-reported profiles easier by adding more situational context (how many messages renderered, what kind of file was opened, etc).

It would help if markers could have optional payload so this information doesn't have to be encoded in the marker label.

Comment 1

[:Harald Kirschner :digitarald]

Florian, is this something you already thought about?

Comment 2

[Florian Quèze [:florian]]

(In reply to :Harald Kirschner :digitarald from comment #1)

Florian, is this something you already thought about?

I haven't really thought about it, no. Would you want to pass an arbitrary JS object and have its properties be saved as part of the marker? I'm not sure how feasible that would be, and how we would display it in the profiler front-end, but if it's possible with relatively low effort that would be very nice.

Comment 3

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

By "addProfileMarker", do you mean Services.profiler.AddMarker()?

I guess (not an IDL expert) we could easily add an optional second string, and store that in a TextMarkerPayload.

Not sure about accepting an arbitrary JS object, it sounds like a lot of work, but assuming we can somehow do whatever dump() does, it should be possible(?) 🤷‍♂️

Comment 4

[Florian Quèze [:florian]]

(In reply to Gerald Squelart [:gerald] (he/him) from comment #3)

By "addProfileMarker", do you mean Services.profiler.AddMarker()?

I'm pretty sure Harald has ChromeUtils.addProfilerMarker in mind. It already supports an optional string parameter to make it a text marker.

Not sure about accepting an arbitrary JS object, it sounds like a lot of work, but assuming we can somehow do whatever dump() does, it should be possible(?) 🤷‍♂️

Maybe that would be like a text marker with multiple strings as key/value pairs?

Comment 5

[Greg Tatum [:gregtatum]]

I'm making this block Bug 1665457 as it could be very helpful for the test infrastructure to have consistent markers.

Comment 6

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

Some thoughts, as we're getting closer to shipping the new display schema:

One difficulty is to transfer arbitrary data from JS to C++, serialize it in the profiler buffer, and later stream it out as JSON.
But we can see that end-to-end, we go from JS to JSON 🤔💡

So, how about: Whatever JS functions we end up with, they would eventually call into C++ with two JSON strings that contain the full marker data and the associated schema. JSON strings are easy to serialize and stream as-is.
On the public JS side, we could provide helper functions to ensure that we have proper "marker types" (i.e., a named set of expected marker data and their display schema), and that markers of that types actually carry the expected data; it would be good to clearly show errors along the way to help our developer-users.

And this could be extended to our other languages Rust and Java: We could have public functions that encourage proper usage, and internally they package things as JSON strings through our C++ backend so they're ready to be written in the final profile.

Comment 7

[Greg Tatum [:gregtatum]]

Sounds like a great plan!