Show a category breakdown in profiler-cli - #6256
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #6256 +/- ##
==========================================
+ Coverage 83.73% 83.75% +0.01%
==========================================
Files 350 352 +2
Lines 37523 37587 +64
Branches 10543 10552 +9
==========================================
+ Hits 31420 31481 +61
- Misses 5676 5678 +2
- Partials 427 428 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
canova
left a comment
There was a problem hiding this comment.
Thanks! I haven't reviewed the code yet, but posting some high level review after looking briefly to keep the conversation going:
Let's not add a --categories argument and directly output them all the time.
I think it came up a few times in the past before, so it's good to be on the same page. I see things like this when it comes to whether we want an argument or not:
- Are we changing a state that has a default: yes let's add as an arg (like the call tree strategy)
- Are we applying a filter: yes (like
--limitor--searchetc.) - Are we adding an information that's otherwise not visible:
- do we think it's useful in general: no, let's output it all the time
- do we think it's not so useful in general: no, let's add as a sub command
The reason behind is that, arguments are less visible and it doesn't provide a lot of value. If we think it's important, we should just include it all the time. If we think it's not that important in general but can be useful for some cases only, we can add as a sub command.
Let me know what you think!
The call tree sidebar sorted its category breakdown, dropped the empty entries and worked out each row's share of the total inline in its render method. Moving that to sortCategoryBreakdown lets other views present a breakdown the same way without reimplementing the ordering or the percentage denominator. No visible change: the sidebar renders exactly what it did before.
The CLI had no way to tell whether a hot region of a profile was Layout, GC, JavaScript or Graphics, which the call tree sidebar shows in the web UI. `thread samples` now breaks the samples currently in view down by category and subcategory, and `function info` does the same for one function's running and self time. Closes firefox-devtools#6204
I agree with the reasoning, I just think we need to put that knowledge from your head into some project docs :) |
Main | Deploy preview
The CLI had no way to tell whether a hot region of a profile was Layout, GC, JavaScript or Graphics, which the call tree sidebar shows in the web UI.
thread samplesnow breaks the samples currently in view down by category and subcategory, andfunction infodoes the same for one function's running and self time.Closes #6204
Profile
Usage examples: