Add a generic palette to highlight kernel / jitted code #203
Conversation
The "java" and "js" colour palettes have a very useful feature of being able to highlight which frames are jitted or kernel functions based on the annotations added to the function makes by the stackcollapse programs. Unfortunately they also make a number of assumptions based on the function names which are very specific to those languages, and cause nonsensical results with other runtimes. This PR adds a new generic "annotated" colour palette that only uses the function name annotations, and can thus be used regardless of the runtime being profiled.
Codecov Report
@@ Coverage Diff @@
## master #203 +/- ##
==========================================
- Coverage 90.08% 89.40% -0.69%
==========================================
Files 17 17
Lines 2239 2312 +73
==========================================
+ Hits 2017 2067 +50
- Misses 222 245 +23
Continue to review full report at Codecov.
|
|
Just out of interest, could you post a flamegraph with this color mode enabled? Would be interesting to see what it looks like! |
|
|
For posterity: That's really neat! I almost wonder if we should make this the default behavior for the |
|
@jonhoo I wouldn't be opposed to that idea. |
|
The output definitely looks useful. I would suggest documenting the heuristics, so people know what to expect. |
|
@asherkin I'm pretty happy making this the default. Could you update the PR so that Oh, and as in the other PR, could you please update the changelog at the end? |
|
@jonhoo Apologies for the late response, I caught the virus that's been going around and have been out of action for a few weeks. Would love to make this the default! I think the only concern I'd have with that myself is the massively reduced colour range of "red" used here compared to the "hot" palette. I only used these colours as they're what the java one was using, how would you feel about me crafting specifically a new default using these annotations? I'm thinking "hot" for the default (so the output will be the same as currently for anything not-annotated), something muted like greys for the kernel frames, and something bright (probably stick with the green/blue) for the jit/inline frames? |
|
Ouch, I'm sorry to hear that. Glad to have you back with us! Absolutely, that sounds like an excellent idea 🎉 |
|
I've implemented the proposed new colour scheme and I think it's looking good. The default has changed to be the new annotated palette - let me know if you would prefer that to still be called "hot" from a CLI perspective and I'll make that change, right now I've kept it as its own new thing (internally it'll never be totally straightforward due to BasicPalette vs MultiPalette anyway). I'll update the changelog once that's cleared up. I could do with some pointers regarding where you'd like documentation related to the annotation behaviour - the adding of annotations and the details around that are handled (and documented) on the stackcollapse side, so I'm not totally sure what's wanted over here on the flamegraph end. EDIT: Whoops, need to sort the tests.
|
asherkin
force-pushed
the
generic-annotated-palette
branch
from
1dae0aa
to
6d0fa59
Compare
asherkin
force-pushed
the
generic-annotated-palette
branch
from
6d0fa59
to
16e2494
Compare
|
Nice! How about we call the old palette As for documentation, I think the thing to do is mention in the stack collapsing docs how we detect JIT/kernel things, and then in the flamegraph docs how we expect names to be annotated in order to highlight them as JIT/kernel. How does that sound? I think we'll likely end up with a full API makeover (including docs) at some point in the future when we land a proper API as well 😅 |
The "java" and "js" colour palettes have a very useful feature of being able to highlight which frames are jitted or kernel functions based on the annotations added by the stackcollapse programs. Unfortunately they also make a number of assumptions based on the function names which are very specific to those languages, and cause nonsensical results with other runtimes.
This PR adds a new generic "annotated" colour palette that only uses the function name annotations, and can thus be used regardless of the runtime being profiled.