Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1574415 > unrolled thread

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

Back to article view | Back to linux.kernel

Contents

  [PATCHSET 0/3] perf diff: Introduce delta-abs compute method Namhyung Kim <namhyung@kernel.org> - 2017-02-06 08:30 +0100
    Re: [PATCHSET 0/3] perf diff: Introduce delta-abs compute method Jiri Olsa <jolsa@redhat.com> - 2017-02-06 11:30 +0100
      Re: [PATCHSET 0/3] perf diff: Introduce delta-abs compute method Namhyung Kim <namhyung@kernel.org> - 2017-02-06 14:50 +0100
        Re: [PATCHSET 0/3] perf diff: Introduce delta-abs compute method Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-02-06 15:20 +0100
    Re: [PATCHSET 0/3] perf diff: Introduce delta-abs compute method Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-02-06 14:00 +0100
      Re: [PATCHSET 0/3] perf diff: Introduce delta-abs compute method Namhyung Kim <namhyung@kernel.org> - 2017-02-06 15:30 +0100
        Re: [PATCHSET 0/3] perf diff: Introduce delta-abs compute method Arnaldo Carvalho de Melo <acme@kernel.org> - 2017-02-07 17:10 +0100
          Re: [PATCHSET 0/3] perf diff: Introduce delta-abs compute method Namhyung Kim <namhyung@kernel.org> - 2017-02-10 08:30 +0100

#1574415 — [PATCHSET 0/3] perf diff: Introduce delta-abs compute method

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.

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

[toc] | [next] | [standalone]

#1574534

On Mon, Feb 06, 2017 at 04:20:34PM +0900, Namhyung Kim wrote:

SNIP

> 
>   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
> 

hum, do I miss some -o fix?

[jolsa@krava perf]$ ./perf diff -o 1 -c delta-abs
Segmentation fault (core dumped)

it's not delta-abs specific, I'm getting the crash for others

thanks,
jirka


(gdb) r diff -o 1 -c delta-abs
Starting program: /home/jolsa/kernel/linux-perf/tools/perf/perf diff -o 1 -c delta-abs
Missing separate debuginfos, use: dnf debuginfo-install glibc-2.23.1-11.fc24.x86_64
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib64/libthread_db.so.1".
Detaching after fork from child process 12611.

Program received signal SIGSEGV, Segmentation fault.
0x0000000000000000 in ?? ()
Missing separate debuginfos, use: dnf debuginfo-install audit-libs-2.7.1-1.fc24.x86_64 bzip2-libs-1.0.6-21.fc24.x86_64 elfutils-libelf-0.168-1.fc24.x86_64 elfutils-libs-0.168-1.fc24.x86_64 libcap-ng-0.7.8-1.fc24.x86_64 libunwind-1.1-11.fc24.x86_64 nss-softokn-freebl-3.28.1-1.0.fc24.x86_64 numactl-libs-2.0.11-2.fc24.x86_64 openssl-libs-1.0.2j-3.fc24.x86_64 perl-libs-5.22.3-368.fc24.x86_64 python-libs-2.7.13-1.fc24.x86_64 slang-2.3.0-5.fc24.x86_64 xz-libs-5.2.2-2.fc24.x86_64 zlib-1.2.8-10.fc24.x86_64
(gdb) bt
#0  0x0000000000000000 in ?? ()
#1  0x000000000059c3c2 in fprintf_line (hists=0x2159980, hpp=0x7fffffffda60, line=0, fp=0x7ffff55c3600 <_IO_2_1_stdout_>)
    at ui/stdio/hist.c:677
#2  0x000000000059c4af in hists__fprintf_standard_headers (hists=0x2159980, hpp=0x7fffffffda60, fp=0x7ffff55c3600 <_IO_2_1_stdout_>)
    at ui/stdio/hist.c:700
#3  0x000000000059c707 in hists__fprintf_headers (hists=0x2159980, fp=0x7ffff55c3600 <_IO_2_1_stdout_>) at ui/stdio/hist.c:745
#4  0x000000000059c7b9 in hists__fprintf (hists=0x2159980, show_header=true, max_rows=0, max_cols=0, min_pcnt=0, 
    fp=0x7ffff55c3600 <_IO_2_1_stdout_>, use_callchain=false) at ui/stdio/hist.c:769
#5  0x000000000042b6aa in hists__process (hists=0x2159980) at builtin-diff.c:694
#6  0x000000000042b944 in data_process () at builtin-diff.c:753
#7  0x000000000042bb2b in __cmd_diff () at builtin-diff.c:790
#8  0x000000000042d0ed in cmd_diff (argc=0, argv=0x7fffffffe3d0, prefix=0x0) at builtin-diff.c:1349
#9  0x00000000004b817e in run_builtin (p=0xa10be0 <commands+96>, argc=5, argv=0x7fffffffe3d0) at perf.c:359
#10 0x00000000004b83eb in handle_internal_command (argc=5, argv=0x7fffffffe3d0) at perf.c:421
#11 0x00000000004b8530 in run_argv (argcp=0x7fffffffe21c, argv=0x7fffffffe210) at perf.c:467
#12 0x00000000004b8933 in main (argc=5, argv=0x7fffffffe3d0) at perf.c:614

[toc] | [prev] | [next] | [standalone]

#1574734

On Mon, Feb 06, 2017 at 11:26:17AM +0100, Jiri Olsa wrote:
> On Mon, Feb 06, 2017 at 04:20:34PM +0900, Namhyung Kim wrote:
> 
> SNIP
> 
> > 
> >   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
> > 
> 
> hum, do I miss some -o fix?
> 
> [jolsa@krava perf]$ ./perf diff -o 1 -c delta-abs
> Segmentation fault (core dumped)
> 
> it's not delta-abs specific, I'm getting the crash for others

Yes, the fixes are in perf/urgent and it seems not sync'ed to
perf/core yet.  If you pull my branch it has the fixes as well.

Thanks,
Namhyung


> 
> thanks,
> jirka
> 
> 
> (gdb) r diff -o 1 -c delta-abs
> Starting program: /home/jolsa/kernel/linux-perf/tools/perf/perf diff -o 1 -c delta-abs
> Missing separate debuginfos, use: dnf debuginfo-install glibc-2.23.1-11.fc24.x86_64
> [Thread debugging using libthread_db enabled]
> Using host libthread_db library "/lib64/libthread_db.so.1".
> Detaching after fork from child process 12611.
> 
> Program received signal SIGSEGV, Segmentation fault.
> 0x0000000000000000 in ?? ()
> Missing separate debuginfos, use: dnf debuginfo-install audit-libs-2.7.1-1.fc24.x86_64 bzip2-libs-1.0.6-21.fc24.x86_64 elfutils-libelf-0.168-1.fc24.x86_64 elfutils-libs-0.168-1.fc24.x86_64 libcap-ng-0.7.8-1.fc24.x86_64 libunwind-1.1-11.fc24.x86_64 nss-softokn-freebl-3.28.1-1.0.fc24.x86_64 numactl-libs-2.0.11-2.fc24.x86_64 openssl-libs-1.0.2j-3.fc24.x86_64 perl-libs-5.22.3-368.fc24.x86_64 python-libs-2.7.13-1.fc24.x86_64 slang-2.3.0-5.fc24.x86_64 xz-libs-5.2.2-2.fc24.x86_64 zlib-1.2.8-10.fc24.x86_64
> (gdb) bt
> #0  0x0000000000000000 in ?? ()
> #1  0x000000000059c3c2 in fprintf_line (hists=0x2159980, hpp=0x7fffffffda60, line=0, fp=0x7ffff55c3600 <_IO_2_1_stdout_>)
>     at ui/stdio/hist.c:677
> #2  0x000000000059c4af in hists__fprintf_standard_headers (hists=0x2159980, hpp=0x7fffffffda60, fp=0x7ffff55c3600 <_IO_2_1_stdout_>)
>     at ui/stdio/hist.c:700
> #3  0x000000000059c707 in hists__fprintf_headers (hists=0x2159980, fp=0x7ffff55c3600 <_IO_2_1_stdout_>) at ui/stdio/hist.c:745
> #4  0x000000000059c7b9 in hists__fprintf (hists=0x2159980, show_header=true, max_rows=0, max_cols=0, min_pcnt=0, 
>     fp=0x7ffff55c3600 <_IO_2_1_stdout_>, use_callchain=false) at ui/stdio/hist.c:769
> #5  0x000000000042b6aa in hists__process (hists=0x2159980) at builtin-diff.c:694
> #6  0x000000000042b944 in data_process () at builtin-diff.c:753
> #7  0x000000000042bb2b in __cmd_diff () at builtin-diff.c:790
> #8  0x000000000042d0ed in cmd_diff (argc=0, argv=0x7fffffffe3d0, prefix=0x0) at builtin-diff.c:1349
> #9  0x00000000004b817e in run_builtin (p=0xa10be0 <commands+96>, argc=5, argv=0x7fffffffe3d0) at perf.c:359
> #10 0x00000000004b83eb in handle_internal_command (argc=5, argv=0x7fffffffe3d0) at perf.c:421
> #11 0x00000000004b8530 in run_argv (argcp=0x7fffffffe21c, argv=0x7fffffffe210) at perf.c:467
> #12 0x00000000004b8933 in main (argc=5, argv=0x7fffffffe3d0) at perf.c:614

[toc] | [prev] | [next] | [standalone]

#1574813

Em Mon, Feb 06, 2017 at 10:44:03PM +0900, Namhyung Kim escreveu:
> On Mon, Feb 06, 2017 at 11:26:17AM +0100, Jiri Olsa wrote:
> > On Mon, Feb 06, 2017 at 04:20:34PM +0900, Namhyung Kim wrote:
> > > Namhyung Kim (3):
> > >   perf diff: Add 'delta-abs' compute method
> > >   perf diff: Add diff.order config option
> > >   perf diff: Add diff.compute config option

> > hum, do I miss some -o fix?

> > [jolsa@krava perf]$ ./perf diff -o 1 -c delta-abs
> > Segmentation fault (core dumped)

> > it's not delta-abs specific, I'm getting the crash for others
 
> Yes, the fixes are in perf/urgent and it seems not sync'ed to
> perf/core yet.  If you pull my branch it has the fixes as well.

Just merged tip/perf/urgent into acme/perf/core, just pushed it to
git.kernel.org,

- Arnaldo
 
> Thanks,
> Namhyung
> 
> 
> > 
> > thanks,
> > jirka
> > 
> > 
> > (gdb) r diff -o 1 -c delta-abs
> > Starting program: /home/jolsa/kernel/linux-perf/tools/perf/perf diff -o 1 -c delta-abs
> > Missing separate debuginfos, use: dnf debuginfo-install glibc-2.23.1-11.fc24.x86_64
> > [Thread debugging using libthread_db enabled]
> > Using host libthread_db library "/lib64/libthread_db.so.1".
> > Detaching after fork from child process 12611.
> > 
> > Program received signal SIGSEGV, Segmentation fault.
> > 0x0000000000000000 in ?? ()
> > Missing separate debuginfos, use: dnf debuginfo-install audit-libs-2.7.1-1.fc24.x86_64 bzip2-libs-1.0.6-21.fc24.x86_64 elfutils-libelf-0.168-1.fc24.x86_64 elfutils-libs-0.168-1.fc24.x86_64 libcap-ng-0.7.8-1.fc24.x86_64 libunwind-1.1-11.fc24.x86_64 nss-softokn-freebl-3.28.1-1.0.fc24.x86_64 numactl-libs-2.0.11-2.fc24.x86_64 openssl-libs-1.0.2j-3.fc24.x86_64 perl-libs-5.22.3-368.fc24.x86_64 python-libs-2.7.13-1.fc24.x86_64 slang-2.3.0-5.fc24.x86_64 xz-libs-5.2.2-2.fc24.x86_64 zlib-1.2.8-10.fc24.x86_64
> > (gdb) bt
> > #0  0x0000000000000000 in ?? ()
> > #1  0x000000000059c3c2 in fprintf_line (hists=0x2159980, hpp=0x7fffffffda60, line=0, fp=0x7ffff55c3600 <_IO_2_1_stdout_>)
> >     at ui/stdio/hist.c:677
> > #2  0x000000000059c4af in hists__fprintf_standard_headers (hists=0x2159980, hpp=0x7fffffffda60, fp=0x7ffff55c3600 <_IO_2_1_stdout_>)
> >     at ui/stdio/hist.c:700
> > #3  0x000000000059c707 in hists__fprintf_headers (hists=0x2159980, fp=0x7ffff55c3600 <_IO_2_1_stdout_>) at ui/stdio/hist.c:745
> > #4  0x000000000059c7b9 in hists__fprintf (hists=0x2159980, show_header=true, max_rows=0, max_cols=0, min_pcnt=0, 
> >     fp=0x7ffff55c3600 <_IO_2_1_stdout_>, use_callchain=false) at ui/stdio/hist.c:769
> > #5  0x000000000042b6aa in hists__process (hists=0x2159980) at builtin-diff.c:694
> > #6  0x000000000042b944 in data_process () at builtin-diff.c:753
> > #7  0x000000000042bb2b in __cmd_diff () at builtin-diff.c:790
> > #8  0x000000000042d0ed in cmd_diff (argc=0, argv=0x7fffffffe3d0, prefix=0x0) at builtin-diff.c:1349
> > #9  0x00000000004b817e in run_builtin (p=0xa10be0 <commands+96>, argc=5, argv=0x7fffffffe3d0) at perf.c:359
> > #10 0x00000000004b83eb in handle_internal_command (argc=5, argv=0x7fffffffe3d0) at perf.c:421
> > #11 0x00000000004b8530 in run_argv (argcp=0x7fffffffe21c, argv=0x7fffffffe210) at perf.c:467
> > #12 0x00000000004b8933 in main (argc=5, argv=0x7fffffffe3d0) at perf.c:614

[toc] | [prev] | [next] | [standalone]

#1574651

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

[toc] | [prev] | [next] | [standalone]

#1574833

Hi Arnaldo,

On Mon, Feb 06, 2017 at 09:51:49AM -0300, Arnaldo Carvalho de Melo wrote:
> 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?

Good.  Also, as I said in the changelog, it needs to change default
value of -o option to 1 in order to make the 'delta-abs' effective.

> 
> 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".

The -q/--quiet looks ok since it corresponds to -v/--verbose option.
But I'm not sure about the number option.

In case of git, it'll stop processing commits after the given number
of them, so it will reduce significant processing time IMHO.  However,
in perf, we need to process whole data anyway and sort at the final
stage, and then stop displaying entries after the given number.

Maybe it's just a shortcut of piping to the head command.  Then I
don't feel the strong desire to have it as we have pager, TUI and GUI
already.

Thanks,
Namhyung


> 
> 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

[toc] | [prev] | [next] | [standalone]

#1575830

Em Mon, Feb 06, 2017 at 11:26:16PM +0900, Namhyung Kim escreveu:
> Hi Arnaldo,
> 
> On Mon, Feb 06, 2017 at 09:51:49AM -0300, Arnaldo Carvalho de Melo wrote:
> > Em Mon, Feb 06, 2017 at 04:20:34PM +0900, Namhyung Kim escreveu:
> > I agree on having the default changed to 'delta-abs', Ingo?
 
> Good.  Also, as I said in the changelog, it needs to change default
> value of -o option to 1 in order to make the 'delta-abs' effective.

ok
 
> > 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".
> 
> The -q/--quiet looks ok since it corresponds to -v/--verbose option.

Ok, agreed on this one.

> But I'm not sure about the number option.
 
> In case of git, it'll stop processing commits after the given number
> of them, so it will reduce significant processing time IMHO.  However,
> in perf, we need to process whole data anyway and sort at the final
> stage, and then stop displaying entries after the given number.
 
> Maybe it's just a shortcut of piping to the head command.  Then I

I wasn't thinking about the processing savings from stopping to process
at that many lines, my suggestion was just about making the command line
more compact, to type less.

If that can also map to processing savings, the better.

- Arnaldo

> don't feel the strong desire to have it as we have pager, TUI and GUI
> already.
> 
> Thanks,
> Namhyung
> 
> 
> > 
> > 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

[toc] | [prev] | [next] | [standalone]

#1578216

Hi Arnaldo,

On Tue, Feb 07, 2017 at 01:02:14PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Feb 06, 2017 at 11:26:16PM +0900, Namhyung Kim escreveu:
> > Hi Arnaldo,
> > 
> > On Mon, Feb 06, 2017 at 09:51:49AM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Mon, Feb 06, 2017 at 04:20:34PM +0900, Namhyung Kim escreveu:
> > > I agree on having the default changed to 'delta-abs', Ingo?
>  
> > Good.  Also, as I said in the changelog, it needs to change default
> > value of -o option to 1 in order to make the 'delta-abs' effective.
> 
> ok
>  
> > > 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".
> > 
> > The -q/--quiet looks ok since it corresponds to -v/--verbose option.
> 
> Ok, agreed on this one.
> 
> > But I'm not sure about the number option.
>  
> > In case of git, it'll stop processing commits after the given number
> > of them, so it will reduce significant processing time IMHO.  However,
> > in perf, we need to process whole data anyway and sort at the final
> > stage, and then stop displaying entries after the given number.
>  
> > Maybe it's just a shortcut of piping to the head command.  Then I
> 
> I wasn't thinking about the processing savings from stopping to process
> at that many lines, my suggestion was just about making the command line
> more compact, to type less.
> 
> If that can also map to processing savings, the better.

Ok, I'll take a look at it later.  I just want to finish this work first.

Thanks,
Namhyung

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web