Skip to content
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

Add flat mode to console renderer #240

Merged
merged 18 commits into from May 28, 2023
Merged

Conversation

matthiasdiener
Copy link
Contributor

@matthiasdiener matthiasdiener commented May 3, 2023

This was inspired by a similar flat mode in vmprof. Sometimes it is interesting to focus on functions that get called from multiple locations in the call graph in order to determine which functions consume the most time overall.

Run with pyinstrument ... -p flat.

Here is how it looks like:

$ pyinstrument --load-prev 2023-05-03T14-25-05  # standard mode

  _     ._   __/__   _ _  _  _ _/_   Recorded: 14:25:05  Samples:  165
 /_//_/// /_\ / //_// / //_'/ //     Duration: 1.285     CPU time: 0.068
/   _/                      v4.4.0

Program: examples/c_sort.py

1.284 <module>  ../examples/c_sort.py:1
├─ 1.193 slow_key  ../examples/c_sort.py:27
│  └─ 1.193 sleep  None
│        [2 frames hidden]  ..
└─ 0.090 <module>  numpy/__init__.py:1
      [158 frames hidden]  numpy, pickle, .., _compat_pickle, __...

To view this report with different options, run:
    pyinstrument --load-prev 2023-05-03T14-25-05 [options]

$ pyinstrument --load-prev 2023-05-03T14-25-05 -p flat  # flat mode

  _     ._   __/__   _ _  _  _ _/_   Recorded: 14:25:05  Samples:  165
 /_//_/// /_\ / //_// / //_'/ //     Duration: 1.285     CPU time: 0.068
/   _/                      v4.4.0

Program: examples/c_sort.py

1.193s sleep  ../<built-in>:0
0.023s <module>  numpy/random/_pickle.py:1
0.018s <module>  numpy/core/__init__.py:1
0.018s <module>  numpy/core/overrides.py:1
0.018s <module>  numpy/__init__.py:1
0.013s <module>  numpy/lib/__init__.py:1
0.000s <module>  ../examples/c_sort.py:1
0.000s slow_key  ../examples/c_sort.py:27
0.000s <module>  numpy/core/multiarray.py:1
0.000s <module>  numpy/random/__init__.py:1

To view this report with different options, run:
    pyinstrument --load-prev 2023-05-03T14-25-05 [options]

@joerick
Copy link
Owner

joerick commented May 4, 2023

Nice, thanks! I'll take a look soon. Just a quick note, 'exclusive time' is known as 'self time' in this project. I think it's already available as total_self_time on Frame. But, there may have been a bug in that property until recently, can't remember if it's merged or not!

@matthiasdiener
Copy link
Contributor Author

Nice, thanks! I'll take a look soon. Just a quick note, 'exclusive time' is known as 'self time' in this project. I think it's already available as total_self_time on Frame. But, there may have been a bug in that property until recently, can't remember if it's merged or not!

Thanks! I tried this with total_self_time, but it always returned zero. Looking at the other PRs, I think this might be a fix: https://github.com/joerick/pyinstrument/pull/236/files#diff-34c9ac7ed6042dfd8c5340562bb37088e871be076ce870b1325da6e3f8f4d5cb

@matthiasdiener
Copy link
Contributor Author

Nice, thanks! I'll take a look soon. Just a quick note, 'exclusive time' is known as 'self time' in this project. I think it's already available as total_self_time on Frame. But, there may have been a bug in that property until recently, can't remember if it's merged or not!

Thanks! I tried this with total_self_time, but it always returned zero. Looking at the other PRs, I think this might be a fix: https://github.com/joerick/pyinstrument/pull/236/files#diff-34c9ac7ed6042dfd8c5340562bb37088e871be076ce870b1325da6e3f8f4d5cb

I added that fix to this PR and modified the code to use total_self_time. Could you please approve the CI workflow?

Copy link
Owner

@joerick joerick left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good @matthiasdiener, thank you! A few comments inline. The last commit you added isn't correct though, it's not possible to say anything about 'ncalls' in pyinstrument because it is a statistical profiler - they could be many calls that pyinstrument didn't record because it's only recording every {interval}s.

pyinstrument/renderers/console.py Outdated Show resolved Hide resolved
pyinstrument/renderers/console.py Outdated Show resolved Hide resolved
@matthiasdiener
Copy link
Contributor Author

matthiasdiener commented May 16, 2023

The last commit you added isn't correct though, it's not possible to say anything about 'ncalls' in pyinstrument because it is a statistical profiler - they could be many calls that pyinstrument didn't record because it's only recording every {interval}s.

You're right, of course. I reverted that commit in bcca59c.

@joerick
Copy link
Owner

joerick commented May 25, 2023

leave this with me @matthiasdiener, I have some more thoughts on code structure that are probably easier to do than describe!

@joerick
Copy link
Owner

joerick commented May 26, 2023

Would you mind checking this still works as you intended @matthiasdiener ? I think I'm happy with it.

Copy link
Contributor Author

@matthiasdiener matthiasdiener left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @joerick! This still works as intended.

pyinstrument/renderers/console.py Outdated Show resolved Hide resolved
pyinstrument/renderers/console.py Outdated Show resolved Hide resolved
joerick and others added 2 commits May 26, 2023 18:38
@matthiasdiener
Copy link
Contributor Author

Thanks @joerick! This is still working as intended.

@joerick joerick merged commit c0f91cd into joerick:main May 28, 2023
21 checks passed
@matthiasdiener matthiasdiener deleted the flatconsole branch May 30, 2023 21:15
@joerick
Copy link
Owner

joerick commented Jun 5, 2023

thank you @matthiasdiener for this! gonna get a release out really soon

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants