profiler-cli: name the process in the thread banner, and follow the queried thread - #6269
profiler-cli: name the process in the thread banner, and follow the queried thread#6269fqueze wants to merge 2 commits into
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6269 +/- ##
==========================================
+ Coverage 83.79% 83.86% +0.07%
==========================================
Files 350 350
Lines 37583 37603 +20
Branches 10459 10571 +112
==========================================
+ Hits 31492 31537 +45
+ Misses 5664 5639 -25
Partials 427 427 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
canova
left a comment
canova
left a comment
There was a problem hiding this comment.
Before this PR, our banner at the top reported the session state, which is sticky and disregarded the --thread etc. I think that's an important distinction. And I would prefer to keep that distinction still.
I'd keep selectedThreadHandle meaning selected, add a separate result-scoped field, and show both when they differ:
pq thread markers --thread t-2
[Thread: t-2 (GeckoMain, GPU Process) | Selected: t-0 | View: Full profile | Full: 14ms]
This will prevent the AI from getting confused in case it forgets to pass --thread after.
And another thing that I just noticed is that we don't use the friendly process names for the processes in the pq profile info output. If we are adding this to the header I think we should make them consistent because now they report slightly different namings. We should fix that one either in here or in a follow-up.
| "load" pre-selects the most active content/tab GeckoMain, as profiler.firefox.com | ||
| does -- rarely what you want on a CI profile, where the parent process GeckoMain | ||
| (usually t-0) is. A near-empty result set usually means the wrong thread. |
There was a problem hiding this comment.
I don't think adding a Firefox CI specific information is a good idea here. I think the right place for this is the profiler-analysis skill that we have inside Firefox. Let's move it there.
| These describe the thread the result is about, which for a --thread-scoped | ||
| command or a marker in another thread is not the selected one. |
There was a problem hiding this comment.
Huh what does this mean? We don't have --thread-scoped.
On the other hand we try to keep this schemas.txt file as lightweight as possible without much text and with mostly schema definitions. I would prefer to remove this.
There was a problem hiding this comment.
Ah, now I get that it's meant to say --thread t-x scoped.
…ried thread The [Thread: ...] banner named the thread but not its process, so "t-94 (GeckoMain)" read as the parent process when it was a child, and an empty result looked like missing data rather than the wrong thread. The banner now reads "t-94 (GeckoMain, WebExtensions)". Process labels use the front end's naming, so it says "GPU Process" where profile info prints the raw "gpu". A command scoped with --thread reports the thread it queried, but --thread does not select anything, so the banner keeps the two apart rather than letting a one-off query look like a change of selection: [Thread: t-2 (GeckoMain, GPU Process) | Selected: t-0 | View: Full profile ...] selectedThreadHandle/selectedThreads keep meaning the sticky session selection; the new resultThreadHandle/resultThreads carry the queried thread, and are only filled in when it differs from the selection.
"profile info" printed the raw process type for processes the back end did not name, so it showed "gpu" and "rdd" where the thread banner showed "GPU Process" and "Remote Data Decoder". The processType-to-label mapping moves out of getFriendlyThreadName into a shared getProcessTypeLabel, which getProcessName now uses as its fallback. A process that carries its own processName is unaffected.
5ca3e2a to
be2d953
Compare
2 participants
Main | Deploy preview
The [Thread: ...] banner named the thread but not its process, so "t-94 (GeckoMain)" read as the parent process when it was a child, and an empty result looked like missing data rather than the wrong thread. It also kept showing the selected thread when a command queried a different one via --thread.
The banner now reads "t-94 (GeckoMain, WebExtensions)" and follows --thread. Process labels use the front end's naming, so it says "GPU Process" where profile info prints the raw "gpu".