1818080 - Add Firefox profiler markers to Remote Protocol commands and events

Status: RESOLVED FIXED

(Remote Protocol :: Agent, task, P1)

Description

[Henrik Skupin [:whimboo][⌚️UTC+2]]

For my work on bug 1770733 I've used the Firefox profiler to measure the timing when returning 1000 DOM elements (https://share.firefox.dev/3xL169k) and for that it was very helpful to use profiler markers. We probably want to add those by default to help with investigating performance issues with the protocol.

An entry in our JS code could look like:

ChromeUtils.addProfilerMarker(`WebDriver-BiDi [${method}] start`, Cu.now());

We should decide which details the message should contain. If we want to include the parameters and response payload as well we probably want to truncate these.

For now I could imagine the following:

• When the packet (command) is received by the WebSocket - marking it as start
• When the response (result, error) has been finished - marking it as end

Sasha and Julian, do you have other possible use cases? If not those two might be the most important ones.

Comment 1

[Julian Descottes [:jdescottes]]

We could also try to add markers MessageHandler's handleCommand, or in our JSWindowActors, to know how much time we spend in each layer.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Here an example for measuring the length of WebDriver BiDi commands: https://share.firefox.dev/3JzNqVj

The question is which prefix we would like to have. If it may include Remote Protocol as well, so that we could even easily search for Marionette and CDP at the same time. A different own category might be another option for Remote Protocol, but that seems to be some more work and might need to get synced with the Profiler team.

What do you think? Once we agreed on a format it will be easy to add other markers as well as Julian pointed out.

Comment 3

[Alexandra Borovova [:Sasha]]

Adding Remote Protocol prefix sounds good.

Comment 4

[Julian Descottes [:jdescottes]]

Until the UI can accommodate it, I would avoid having too many prefixes. With "WebDriver BiDi" in the shared profile, the command names are already almost completely cropped out.

We could have something compact, such as Remote (BiDi) or similar, but let's make sure the marker table is still usable.

I imagine you added markers to the command directly for now (eg in onPacket in WebDriverBiDiConnection.sys.mjs)?
It could also be interesting to instrument the MessageHandler itself so that we can get information about what happens in the content processes as well?

Edit: I meant the "Marker Chart" tab, I always get the names mixed up. Basically https://share.firefox.dev/40oTSnZ I find that view very helpful to get a quick sense of the activity related to markers.

Comment 5

[Henrik Skupin [:whimboo][⌚️UTC+2]]

I did quite a few updates and here is how it looks now when Marionette and WebDriver BiDi work in tandem: https://share.firefox.dev/3M5FsVn

What's new:

• Our own Remote-Protocol category
• The marker name is very generic now and only contains %protocol%: Command or %protocol%: Event (CDP, BiDi only)
• The marker description does no longer contain the stringified payload given that this will cause a lot of data to be stored for eg. network events. Instead it only contains the command/event name and the command id

As such I would suggest that we get this landed as first iteration and if needed we can follow-up with updates for the message handler. Right now I do not see a huge benefit in having those extra markers given that we would have to add them to the content process and then it's no longer visible in the same view.

Comment 6

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Attachment: Bug 1818080 - [remote] Add profile markers for CDP, Marionette and WebDriver BiDi commands and events.

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f2d872b80977
[remote] Add profile markers for CDP, Marionette and WebDriver BiDi commands and events. r=webdriver-reviewers,canaltinova,jdescottes

Comment 8

[Noemi Erli[:noemi_erli]]

https://hg.mozilla.org/mozilla-central/rev/f2d872b80977