Skip to content

Add sort :per_call option to tprof #13611

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

Merged
merged 1 commit into from
May 28, 2024
Merged

Conversation

sabiwara
Copy link
Contributor

A small follow-up for #13605, I didn't include it in the MVP to keep the PR smaller.

One of the niceties of tprof is the ability to sort per "per call measurement", perhaps we can expose the option?
We might consider some other options as well, but sorting by percentage seems redundant since it should be proportional?

$ mix profile.tprof -e "Enum.each(1..5, &String.Chars.Integer.to_string/1)" --sort per_call
Warmup...


Profile results of #PID<0.106.0>
#                                               CALLS      % TIME µS/CALL
Total                                              20 100.00    6    0.30
String.Chars.Integer.to_string/1                    5   0.00    0    0.00
Enum.reduce_range/5                                 3   0.00    0    0.00
:erlang.integer_to_binary/1                         5  16.67    1    0.20
anonymous fn/3 in Enum.each/2                       5  33.33    2    0.40
anonymous fn/0 in :elixir_compiler_1.__FILE__/1     1  16.67    1    1.00
Enum.each/2                                         1  33.33    2    2.00

Profile done over 6 matching functions
mix profile.tprof -e "Enum.each(1..5, &String.Chars.Integer.to_string/1)" --type memory --sort per_call
Warmup...


Profile results of #PID<0.106.0>
#                           CALLS      % WORDS PER CALL
Total                           6 100.00    19     3.17
:erlang.integer_to_binary/1     5  78.95    15     3.00
Enum.each/2                     1  21.05     4     4.00

Profile done over 2 matching functions

Comment on lines +271 to +272
:per_call ->
:measurement_per_call
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: we could add a when type != :calls here because it doesn't really make sense and would just return arbitrary order, but since tprof allows it I thought there is no need to be extra defensive.

@@ -77,15 +77,23 @@ defmodule Mix.Tasks.Profile.TprofTest do
in_tmp(context.test, fn ->
assert capture_io(fn ->
Tprof.run(["--sort", "calls", "-e", @expr])
end) =~ ~r/Enum\.each\/2.*String\.Chars\.Integer\.to_string\/1/s
end) =~ ~r/\nEnum\.each\/2.*\nString\.Chars\.Integer\.to_string\/1/s
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Note: prepending \n here because otherwise it might match due to anonymous fn/3 in Enum.each/2

Copy link
Member

@josevalim josevalim left a comment

Choose a reason for hiding this comment

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

Please backport to v1.17 after merging!

@sabiwara sabiwara merged commit a49a9d8 into elixir-lang:main May 28, 2024
9 checks passed
sabiwara added a commit that referenced this pull request May 28, 2024
@sabiwara sabiwara deleted the tprof-per-call branch May 28, 2024 22:03
@sabiwara
Copy link
Contributor Author

Backported

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

Successfully merging this pull request may close these issues.

2 participants