Re: [PATCH 2/2] perf top: support hotkey to change sort order

[Arnaldo Carvalho de Melo]

Em Fri, Mar 20, 2020 at 03:24:14PM +0800, Jin Yao escreveu:
> It would be nice if we can use a hotkey in perf top browser to
> select a event for sorting.
> 
> For example,
> perf top --group -e cycles,instructions,cache-misses
> 
> Samples
>                 Overhead  Shared Object             Symbol
>   40.03%  45.71%   0.03%  div                       [.] main
>   20.46%  14.67%   0.21%  libc-2.27.so              [.] __random_r
>   20.01%  19.54%   0.02%  libc-2.27.so              [.] __random
>    9.68%  10.68%   0.00%  div                       [.] compute_flag
>    4.32%   4.70%   0.00%  libc-2.27.so              [.] rand
>    3.84%   3.43%   0.00%  div                       [.] rand@plt
>    0.05%   0.05%   2.33%  libc-2.27.so              [.] __strcmp_sse2_unaligned
>    0.04%   0.08%   2.43%  perf                      [.] perf_hpp__is_dynamic_en
>    0.04%   0.02%   6.64%  perf                      [.] rb_next
>    0.04%   0.01%   3.87%  perf                      [.] dso__find_symbol
>    0.04%   0.04%   1.77%  perf                      [.] sort__dso_cmp
> 
> When user press hotkey '2' (event index, starting from 0), it indicates
> to sort output by the third event in group (cache-misses).
> 
> Samples
>                 Overhead  Shared Object               Symbol
>    4.07%   1.28%   6.68%  perf                        [.] rb_next
>    3.57%   3.98%   4.11%  perf                        [.] __hists__insert_output
>    3.67%  11.24%   3.60%  perf                        [.] perf_hpp__is_dynamic_e
>    3.67%   3.20%   3.20%  perf                        [.] hpp__sort_overhead
>    0.81%   0.06%   3.01%  perf                        [.] dso__find_symbol
>    1.62%   5.47%   2.51%  perf                        [.] hists__match
>    2.70%   1.86%   2.47%  libc-2.27.so                [.] _int_malloc
>    0.19%   0.00%   2.29%  [kernel]                    [k] copy_page
>    0.41%   0.32%   1.98%  perf                        [.] hists__decay_entries
>    1.84%   3.67%   1.68%  perf                        [.] sort__dso_cmp
>    0.16%   0.00%   1.63%  [kernel]                    [k] clear_page_erms
> 
> Now the output is sorted by cache-misses.
> 
> Signed-off-by: Jin Yao <yao.jin@xxxxxxxxxxxxxxx>
> ---
>  tools/perf/builtin-top.c | 10 ++++++++--
>  1 file changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
> index 144043637cec..b39f6ffb874e 100644
> --- a/tools/perf/builtin-top.c
> +++ b/tools/perf/builtin-top.c
> @@ -616,6 +616,7 @@ static void *display_thread_tui(void *arg)
>  		.arg		= top,
>  		.refresh	= top->delay_secs,
>  	};
> +	int ret;
>  
>  	/* In order to read symbols from other namespaces perf to  needs to call
>  	 * setns(2).  This isn't permitted if the struct_fs has multiple users.
> @@ -626,6 +627,7 @@ static void *display_thread_tui(void *arg)
>  
>  	prctl(PR_SET_NAME, "perf-top-UI", 0, 0, 0);
>  
> +repeat:
>  	perf_top__sort_new_samples(top);
>  
>  	/*
> @@ -638,13 +640,17 @@ static void *display_thread_tui(void *arg)
>  		hists->uid_filter_str = top->record_opts.target.uid_str;
>  	}
>  
> -	perf_evlist__tui_browse_hists(top->evlist, help, &hbt,
> +	ret = perf_evlist__tui_browse_hists(top->evlist, help, &hbt,
>  				      top->min_percent,
>  				      &top->session->header.env,
>  				      !top->record_opts.overwrite,
>  				      &top->annotation_opts);
>  
> -	stop_top();
> +	if (ret == K_RELOAD)
> +		goto repeat;
> +	else
> +		stop_top();
> +

That is really nice and small, but shouldn't we flush all the histograms
that were in place, sorted by the previous key? I think we have a 'z'
for zeroing samples that may be what we need, take a look, please,

- Arnaldo

>  	return NULL;
>  }
>  
> -- 
> 2.17.1
> 

-- 

- Arnaldo