-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add flamegraph profiling using flame and flamer #1120
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
d662bfa
to
f585f7e
Compare
This is pretty cool! Is it mandatory to add the flame-it attribute to each function you wish to track? Can we just follow all functions? |
// flame doesn't include notes in the html graph :( | ||
flame_note!( | ||
flame::StrCow::from("CodeObj name"), | ||
self.code.obj_name.clone().into() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Now we can follow python objects as well!
@@ -334,6 +342,7 @@ impl Frame { | |||
|
|||
/// Execute a single instruction. | |||
#[allow(clippy::cognitive_complexity)] | |||
#[cfg_attr(feature = "flame-it", flame("Frame"))] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we need to explicitly enable all trace points?
Off topic: why does the importlib loading take so long? Is it a whole lot of code? |
re: importlib: The main problem is that it is a fair bit of code, and RustPython isn't all that optimized. See #1047. re: automatically tagging all fns: see llogiq/flamer#45. I tried to do that initially, and there were a lot of issues with it. Even so, manually tagging all fns in |
I had to use a modified local clone of flame to get this flamegraph due to a bug (llogiq/flame#47), but something interesting that you can see is that in |
Here it is with a simple optimization to skip |
CPython actually have large parts of the |
This is just an initial change to get the ball rolling for flame profiling, we can add more scopes later.