Use a separate category for profiler internals and hide profiler code in marker stacks
Categories
(Core :: Gecko Profiler, task, P3)
Tracking
()
Tracking | Status | |
---|---|---|
firefox87 | --- | fixed |
People
(Reporter: florian, Assigned: florian)
References
Details
Attachments
(2 files, 1 obsolete file)
I'm opening this bug to request feedback on patches attempting to address 2 issues:
- I saw more than once profiles that were made useless by the overhead of the native allocations feature that the bug reporter had enabled in the hope of making the profile more useful. I think it would be nice to make this overhead immediately visible to avoid wasting time investigating timing issues in memory profiles. To make this visible, my idea was to add a PROFILER category, add a few labels in profiler code that is often visible in stack samples, and display that category in red on the front-end to make it impossible to miss. (This will require a front-end patch to add the red color in https://github.com/firefox-devtools/profiler/blob/6b0146209d963aaa1154a9777b71a9125f5b822b/src/utils/colors.js#L92)
- the first few frames of stacks shown for markers that collect a stack are always profiler internals. These stack frames add no value and should be either hidden by the front-end (but how would it detect them?), or maybe better, not collected at all. I think it should be safe to make
MergeStacks
return immediately once it encounters the PROFILER category. I'm pretty sure this idea was initially suggested to me by Gerald (or Greg?) either in a bugzilla comment or phabricator review, but I can't find where.
Assignee | ||
Comment 1•3 years ago
|
||
Comment hidden (obsolete) |
Updated•3 years ago
|
Assignee | ||
Comment 3•3 years ago
|
||
Assignee | ||
Comment 4•3 years ago
|
||
Example profile: https://share.firefox.dev/3ruAljZ
Good ideas!
Comment 6•3 years ago
|
||
display that category in red on the front-end to make it impossible to miss. (This will require a front-end patch to add the red color in https://github.com/firefox-devtools/profiler/blob/6b0146209d963aaa1154a9777b71a9125f5b822b/src/utils/colors.js#L92)
I had more thoughts about that when discussing this at the stand-up this morning.
Now I'm not 100% convinced about this "red" color for the profiler code. Let me explain why.
Currently the "red" color is used only once: for the jank markers. This means that as of now when you see a "red" bar as a user means that you should focus your attention on this part of the profile.
But the profiler code is especially the opposite: that's code that should be ignore by most users.
Therefore I think that by using the same "red" color for profiler code than for jank markers, we make things more confusing.
I agree about the premise for this work though: if we see too much profiler code in a profile, the profile is probably useless, or at least to be taken with a grain of salt.
I don't have a great suggestion, but here is a possibly bad one:
- indeed have a label for the profiler that would change the category, but that doesn't change the color. It would like a "flag". Not sure that this is possible :/ Otherwise make this category grey like other uninteresting stuff => it's not useful for most users, but we'll use it to display a warning (see the second point).
- in the UI, we can compute how much of this label is in one thread and display a warning. We do a similar computation for the Idle category already, so that we can hide tracks.
Happy to hear more suggestions!
Assignee | ||
Comment 7•3 years ago
|
||
(In reply to Florian Quèze [:florian] from comment #4)
Example profile: https://share.firefox.dev/3ruAljZ
Same example profile in a deploy preview showing the red color for the profiler category: https://deploy-preview-3171--perf-html.netlify.app/public/9enczmewncn64safwkt4swbgvwhnjd4n5tvktcg
And another example profile, startup on Windows: https://deploy-preview-3171--perf-html.netlify.app/public/trbxsq1js0ka6p6dm4c10mm5qramnnf6s0se2tg
Pushed by fqueze@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/72365fc165fd add PROFILER category to make profiler overhead very visible, r=gerald. https://hg.mozilla.org/integration/autoland/rev/6ac60fef7bdd When capturing a marker stack, stop when the PROFILER category is encountered, r=gerald.
Comment 9•3 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/72365fc165fd
https://hg.mozilla.org/mozilla-central/rev/6ac60fef7bdd
Description
•