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
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions lib/mix/lib/mix/tasks/profile.tprof.ex
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ defmodule Mix.Tasks.Profile.Tprof do
* `--calls` - filters out any results with a call count lower than this
* `--time` - filters out any results that took lower than specified (in µs), the `type` needs to be `time`
* `--memory` - filters out any results that used less memory than specified (in words), the `type` needs to be `memory`
* `--sort` - sorts the results by `calls` or by the value of `type` (default: the value of `type`)
* `--sort` - sorts the results by `calls`, `per_call` or by the value of `type` (default: the value of `type`)
* `--eval`, `-e` - evaluates the given code
* `--require`, `-r` - requires pattern before running the command
* `--parallel`, `-p` - makes all requires parallel
Expand Down Expand Up @@ -214,6 +214,7 @@ defmodule Mix.Tasks.Profile.Tprof do
defp parse_opt({:sort, "time"}), do: {:sort, :time}
defp parse_opt({:sort, "calls"}), do: {:sort, :calls}
defp parse_opt({:sort, "memory"}), do: {:sort, :memory}
defp parse_opt({:sort, "per_call"}), do: {:sort, :per_call}
defp parse_opt({:sort, other}), do: Mix.raise("Invalid sort option: #{other}")
defp parse_opt(other), do: other

Expand All @@ -235,7 +236,7 @@ defmodule Mix.Tasks.Profile.Tprof do
`type` needs to be `:time`
* `:memory` - filters out any results that used less memory than specified (in words),
`type` needs to be `:memory`
* `:sort` - sort the results by `:calls` or by the value of `type`
* `:sort` - sort the results by `:calls`, `:per_call` or by the value of `type`
(default: the value of `type`)
* `:warmup` - if the code should be warmed up before profiling (default: `true`)
* `:set_on_spawn` - if newly spawned processes should be measured (default: `true`)
Expand Down Expand Up @@ -267,6 +268,9 @@ defmodule Mix.Tasks.Profile.Tprof do
:calls ->
:calls

:per_call ->
:measurement_per_call
Comment on lines +271 to +272
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.


^type ->
:measurement

Expand Down
12 changes: 10 additions & 2 deletions lib/mix/test/mix/tasks/profile.tprof_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -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

end)
end

test "sorts based on memory usage", context do
in_tmp(context.test, fn ->
assert capture_io(fn ->
Tprof.run(["--type", "memory", "--sort", "calls", "-e", @expr])
end) =~ ~r/Enum\.each\/2.*:erlang\.integer_to_binary\/1/s
end) =~ ~r/\nEnum\.each\/2.*\n:erlang\.integer_to_binary\/1/s
end)
end

test "sorts based on memory per call", context do
in_tmp(context.test, fn ->
assert capture_io(fn ->
Tprof.run(["--type", "memory", "--sort", "per_call", "-e", @expr])
end) =~ ~r/\n:erlang\.integer_to_binary\/1.*\nEnum\.each\/2/s
end)
end

Expand Down