Add colour palette for python traces #240
Conversation
mrob95
changed the title
|
I have mainly been testing this with the @benfred Do you think this could be adapted to work with py-spy as well? It looks to me like py-spy removes a lot of the info which would allow us to tell the difference between user code and library code, but it would be great if we could make it work. |
src/flamegraph/color/palettes.rs
Outdated
| use crate::flamegraph::color::BasicPalette; | ||
|
|
||
| pub fn resolve(name: &str) -> BasicPalette { | ||
| if name.starts_with("native@") { // austin-specific format for native calls |
There was a problem hiding this comment.
I worry about encoding generator-specific detection into here. Is there no other way to detect native calls that isn't austin-specific?
There was a problem hiding this comment.
I agree it's a bit nasty to be doing profiler specific checks here. Unfortunately since getting profiles which combine Python stack traces with native calls is pretty involved, we are at the mercy of how the profiler decides to format things.
As far as I know the only profilers which support native calls are austin and py-spy, so if we can support both of those then maybe it's fine.
All austin native calls are prefixed with native@, e.g.
native@7f8b5cbb69a6:ufunc_generic_fastcall:12022
py-spy looks like this:
ufunc_generic_fastcall (numpy/core/_multiarray_umath.cpython-39-x86_64-linux-gnu.so)
I've tagged the author of py-spy in this PR, it would be good to get his input on the best way of parsing this. We could easily do a check for .so or .dll, but I suspect that if the native extension is built with debug symbols then we would get a reference to a source file, which could have any extension.
Alternatively, I'm happy to remove this branch and ignore native calls for now. For me at least it is very rare to need to go down into native calls when profiling Python.
There was a problem hiding this comment.
I'm really torn here. I'd love to hear from the py-spy maintainer too, but I think given the state of the landscape I'm then okay with putting in place checks specifically for austin/py-spy's markers for native calls.
Codecov ReportBase: 90.21% // Head: 90.33% // Increases project coverage by
Additional details and impacted files
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
mrob95
force-pushed
the
mr-python-colours
branch
from
90a1c7f to
f32e41f
Compare
mrob95
force-pushed
the
mr-python-colours
branch
from
aa484f6 to
7d29adb
Compare
|
Was it intentional that you also made changes to the |
Whoops, nope. Reverted that. To be honest I had forgotten about this PR! |
|
Heh, yeah, sorry, I've been falling really behind on OSS stuff! |
mrob95
force-pushed
the
mr-python-colours
branch
from
293252e to
3e4048f
Compare
mrob95
force-pushed
the
mr-python-colours
branch
from
3e4048f to
ed6ab63
Compare
|
BTW, you may be glad to hear that inferno helped us track down and report an O(n^2) in |
mrob95
changed the title
|
Published in 0.11.11 🎉 |
As a side-effect of avoiding duplication, this adds the "python" variant, which was added to inferno: jonhoo/inferno#240 This commit does change the order shown in `--help`.
As a side-effect of avoiding duplication, this adds the "python" variant, which was added to inferno: jonhoo/inferno#240 This commit does change the order shown in `--help`.
Adds a new colour palette for python. Aiming to distinguish four cases:
I've tested it out in a few scenarios and it definitely makes the output more understandable at a glance:
