Re: [PATCHSET 0/3] perf diff: Introduce delta-abs compute method

[Arnaldo Carvalho de Melo]

Em Mon, Feb 06, 2017 at 04:20:34PM +0900, Namhyung Kim escreveu:
> Hello,
> 
> This patchset adds 'delta-abs' compute method to -c/--compute option.
> The 'delta-abs' is same as 'delta' but shows entries with bigger
> absolute delta first instead of sorting numerically.  This is only
> useful together with -o option.
> 
> Below is default output (-c delta):
> 
>   $ perf diff -o 1 -c delta | grep -v ^# | head
>     42.22%   +4.97%  [kernel.kallsyms]  [k] cfb_imageblit
>      0.62%   +1.23%  [kernel.kallsyms]  [k] mutex_lock
>              +1.15%  [kernel.kallsyms]  [k] copy_user_generic_string
>      2.40%   +0.95%  [kernel.kallsyms]  [k] bit_putcs
>      0.31%   +0.79%  [kernel.kallsyms]  [k] link_path_walk
>              +0.64%  [kernel.kallsyms]  [k] kmem_cache_alloc
>      0.00%   +0.57%  [kernel.kallsyms]  [k] __rcu_read_unlock
>              +0.45%  [kernel.kallsyms]  [k] alloc_set_pte
>      0.16%   +0.45%  [kernel.kallsyms]  [k] menu_select
>              +0.41%  ld-2.24.so         [.] do_lookup_x
> 
> Now with 'delta-abs' it shows entries have bigger delta value either
> positive or negative.
> 
>   $ perf diff -o 1 -c delta-abs | grep -v ^# | head
>     42.22%   +4.97%  [kernel.kallsyms]  [k] cfb_imageblit
>     12.72%   -3.01%  [kernel.kallsyms]  [k] intel_idle
>      9.72%   -1.31%  [unknown]          [.] 0x0000000000411343
>      0.62%   +1.23%  [kernel.kallsyms]  [k] mutex_lock
>              +1.15%  [kernel.kallsyms]  [k] copy_user_generic_string
>      2.40%   +0.95%  [kernel.kallsyms]  [k] bit_putcs
>      0.31%   +0.79%  [kernel.kallsyms]  [k] link_path_walk
>      1.35%   -0.71%  [kernel.kallsyms]  [k] smp_call_function_single
>              +0.64%  [kernel.kallsyms]  [k] kmem_cache_alloc
>      0.00%   +0.57%  [kernel.kallsyms]  [k] __rcu_read_unlock
> 
> The patch 2 and 3 are to add config options to control the default
> behavior of perf diff command.  I think that it's worth consider
> changing the default to use 'delta-abs' method since users want to see
> where the difference occurs actually (either positive or negative) IMHO.

I agree on having the default changed to 'delta-abs', Ingo?

Namhyung, and perhaps we should have a single letter option to do that
'| grep -v ^#' bit :-) and perhaps we also should have, for all tools
the equivalent of that "| head", that git log has:

[acme@jouet linux]$ git log --oneline -5
d7cb3a507d23 Merge tag 'perf-core-for-mingo-4.11-20170201' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core
5443624bedd0 perf/x86/intel/pt: Add format strings for PTWRITE and power event tracing
b05d1093987a perf ftrace: Add ftrace.tracer config option
43d41deb71fe perf tools: Create for_each_event macro for tracepoints iteration
a26305363d4b perf test: Add libbpf pinning test
[acme@jouet linux]$

That '-5' to show just the first 5 lines worth of output.

With all that we would have:

  perf diff -o 1 -q10

As the equivalent to "perf diff -o 1 -c delta-abs | grep -v ^# | head".

Ah, adding Paul McKenney to the CC list, he may have something to add
here.

- Arnaldo
 
> The code is avaiable at 'perf/diff-delta-abs-v1' branch in
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git
> 
> Thanks,
> Namhyung
> 
> 
> Namhyung Kim (3):
>   perf diff: Add 'delta-abs' compute method
>   perf diff: Add diff.order config option
>   perf diff: Add diff.compute config option
> 
>  tools/perf/Documentation/perf-config.txt | 12 +++++
>  tools/perf/Documentation/perf-diff.txt   | 15 +++++--
>  tools/perf/builtin-diff.c                | 76 ++++++++++++++++++++++++++++++--
>  3 files changed, 97 insertions(+), 6 deletions(-)
> 
> -- 
> 2.11.0