Skip to content

Commit

Permalink
Consider total when computing tprof layout width
Browse files Browse the repository at this point in the history
Prior to this patch, the length of the total
value being measured was not taken into account
into the layout. This commit addresses it and
also removes outdated comments and redundant
variables.

Closes erlang#8139.
  • Loading branch information
josevalim committed Feb 18, 2024
1 parent 1e5ea6c commit 7bab1f7
Show file tree
Hide file tree
Showing 2 changed files with 26 additions and 15 deletions.
23 changes: 8 additions & 15 deletions lib/tools/src/tprof.erl
Expand Up @@ -1001,21 +1001,16 @@ format_impl(Device, Inspected) when is_map(Inspected) ->
end, Inspected).

format_each(Device, call_count, _Total, Inspected) ->
%% viewport size
%% Viewport = case io:columns() of {ok, C} -> C; _ -> 80 end,
%% layout: module and fun/arity columns are resizable, the rest are not
%% convert all lines to strings
{Widths, Lines} = lists:foldl(
fun ({Mod, {F, A}, Calls, _Value, _VPC, Percent}, {Widths, Ln}) ->
Line = [lists:flatten(io_lib:format("~tw:~tw/~w", [Mod, F, A])),
integer_to_list(Calls), float_to_list(Percent, [{decimals, 2}])],
NewWidths = [erlang:max(Old, New) || {Old, New} <- lists:zip([string:length(L) || L <- Line], Widths)],
{NewWidths, [Line | Ln]}
end, {[0, 5, 5], []}, Inspected),
%% figure our max column widths according to viewport (cut off module/funArity)
FilteredWidths = Widths,

%% figure out formatting line
Fmt = lists:flatten(io_lib:format("~~.~wts ~~~ws [~~~ws]~~n", FilteredWidths)),
Fmt = lists:flatten(io_lib:format("~~.~wts ~~~ws [~~~ws]~~n", Widths)),
%% print using this format
format_out(Device, Fmt, ["FUNCTION", "CALLS", "%"]),
[format_out(Device, Fmt, Line) || Line <- lists:reverse(Lines)],
Expand All @@ -1026,10 +1021,9 @@ format_each(Device, call_memory, Total, Inspected) ->
format_labelled(Device, "WORDS", Total, Inspected).

format_labelled(Device, Label, Total, Inspected) ->
%% viewport size
%% Viewport = case io:columns() of {ok, C} -> C; _ -> 80 end,
%% layout: module and fun/arity columns are resizable, the rest are not
%% convert all lines to strings
%% The width of the value is either total or label
ValueWidth = erlang:max(length(Label), length(integer_to_list(Total))),

{Widths, Lines} = lists:foldl(
fun ({Mod, {F, A}, Calls, Value, VPC, Percent}, {Widths, Ln}) ->
Line = [lists:flatten(io_lib:format("~tw:~tw/~w", [Mod, F, A])),
Expand All @@ -1038,11 +1032,10 @@ format_labelled(Device, Label, Total, Inspected) ->
float_to_list(Percent, [{decimals, 2}])],
NewWidths = [erlang:max(Old, New) || {Old, New} <- lists:zip([string:length(L) || L <- Line], Widths)],
{NewWidths, [Line | Ln]}
end, {[0, 5, length(Label), 8, 5], []}, Inspected),
%% figure our max column widths according to viewport (cut off module/funArity)
FilteredWidths = Widths,
end, {[0, 5, ValueWidth, 8, 5], []}, Inspected),

%% figure out formatting line
Fmt = lists:flatten(io_lib:format("~~.~wts ~~~ws ~~~wts ~~~ws [~~~ws]~~n", FilteredWidths)),
Fmt = lists:flatten(io_lib:format("~~.~wts ~~~ws ~~~wts ~~~ws [~~~ws]~~n", Widths)),
%% print using this format
format_out(Device, Fmt, ["FUNCTION", "CALLS", Label, "PER CALL", "%"]),
[format_out(Device, Fmt, Line) || Line <- lists:reverse(Lines)],
Expand Down
18 changes: 18 additions & 0 deletions lib/tools/test/tprof_SUITE.erl
Expand Up @@ -28,6 +28,7 @@
call_count_ad_hoc/0, call_count_ad_hoc/1,
call_time_ad_hoc/0, call_time_ad_hoc/1,
call_memory_ad_hoc/0, call_memory_ad_hoc/1,
lists_seq_loop/1, call_memory_total/1,
sort/0, sort/1,
rootset/0, rootset/1,
set_on_spawn/0, set_on_spawn/1, seq/1,
Expand All @@ -49,6 +50,7 @@ suite() ->

all() ->
[call_count_ad_hoc, call_time_ad_hoc, call_memory_ad_hoc,
call_memory_total,
sort, rootset, set_on_spawn, live_trace, patterns,
processes, server, hierarchy, code_reload].

Expand Down Expand Up @@ -152,6 +154,22 @@ call_memory_ad_hoc(Config) when is_list(Config) ->
#{timeout => 1000, report => return, type => call_memory}),
?assertMatch({call_memory, [{lists, seq_loop, 3, [{_, 9, 64}]}]}, Profile2).

lists_seq_loop(N) ->
[int_to_bin_twice(M) || M <- lists:seq(1, N)].

int_to_bin_twice(M) ->
B = integer_to_binary(M),
<<B/binary, B/binary>>.

%% Ensure total is not truncated,
%% as per https://github.com/erlang/otp/issues/8139
call_memory_total(_Config) ->
ct:capture_start(),
ok = tprof:profile(?MODULE, lists_seq_loop, [10000], #{type => call_memory}),
ct:capture_stop(),
?assertNotMatch(nomatch, string:find(ct:capture_get(), " 150000 ")),
ok.

sort() ->
[{doc, "Tests sorting methods work"}].

Expand Down

0 comments on commit 7bab1f7

Please sign in to comment.