Closed Bug 1691092 Opened 2 years ago Closed 2 years ago

Use a separate category for profiler internals and hide profiler code in marker stacks

Categories

(Core :: Gecko Profiler, task, P3)

task

Tracking

()

RESOLVED FIXED
87 Branch
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.
Attachment #9201478 - Attachment is obsolete: true

Good ideas!

Severity: -- → N/A
Priority: -- → P3

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!

Depends on: 1691589

(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.
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Target Milestone: --- → 87 Branch
You need to log in before you can comment on or make changes to this bug.