File 7393-Consider-total-when-computing-tprof-layout... of Package erlang

Overview Repositories Revisions Requests Users Attributes Meta

File 7393-Consider-total-when-computing-tprof-layout-width.patch of Package erlang (Revision 08963e5b214430ddd1a5b61ca2dabc24)

Currently displaying revision 08963e5b214430ddd1a5b61ca2dabc24 , Show latest

From 7bab1f7a0160e66f3172b8e18ca58f2a6f9878ad Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Valim?= <jose.valim@dashbit.co>
Date: Sun, 18 Feb 2024 19:02:11 +0100
Subject: [PATCH] Consider total when computing tprof layout width

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 #8139.
---
 lib/tools/src/tprof.erl        | 23 ++++++++---------------
 lib/tools/test/tprof_SUITE.erl | 18 ++++++++++++++++++
 2 files changed, 26 insertions(+), 15 deletions(-)

diff --git a/lib/tools/src/tprof.erl b/lib/tools/src/tprof.erl
index 8fee157ee1..2592bfe561 100644
--- a/lib/tools/src/tprof.erl
+++ b/lib/tools/src/tprof.erl
@@ -1001,10 +1001,6 @@ 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])),
@@ -1012,10 +1008,9 @@ format_each(Device, call_count, _Total, Inspected) ->
             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)],
@@ -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])),
@@ -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)],
diff --git a/lib/tools/test/tprof_SUITE.erl b/lib/tools/test/tprof_SUITE.erl
index dc12e41cf8..0ede626035 100644
--- a/lib/tools/test/tprof_SUITE.erl
+++ b/lib/tools/test/tprof_SUITE.erl
@@ -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,
@@ -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].
 
@@ -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"}].
 
-- 
2.35.3

Places

File 7393-Consider-total-when-computing-tprof-layout-width.patch of Package erlang (Revision 08963e5b214430ddd1a5b61ca2dabc24)

Places