All of lore.kernel.org
 help / color / mirror / Atom feed
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Jiri Olsa <jolsa@redhat.com>, LKML <linux-kernel@vger.kernel.org>,
	David Ahern <dsahern@gmail.com>,
	Stephane Eranian <eranian@google.com>,
	Adrian Hunter <adrian.hunter@intel.com>
Subject: Re: [PATCH 4/7] perf diff: Fix to sort by baseline field by default
Date: Mon, 22 Dec 2014 11:45:13 -0300	[thread overview]
Message-ID: <20141222144513.GB2717@kernel.org> (raw)
In-Reply-To: <1419223455-4362-5-git-send-email-namhyung@kernel.org>

Em Mon, Dec 22, 2014 at 01:44:12PM +0900, Namhyung Kim escreveu:
> The currently perf diff didn't add the baseline and delta (or other
> compute) fields to the sort list so output will be sorted by other
> fields like alphabetical order of DSO or symbol as below example.
> 
> Fix it by adding hpp formats for the fields and provides default
> compare functions.
> 
> Before:
> 
>   $ perf diff
>   # Event 'cycles'
>   #
>   # Baseline    Delta  Shared Object       Symbol
>   # ........  .......  ..................  ...............................
>   #
>                        [bridge]            [k] ip_sabotage_in
>                        [btrfs]             [k] __etree_search.constprop.47
>        0.01%           [btrfs]             [k] btrfs_file_mmap
>        0.01%   -0.01%  [btrfs]             [k] btrfs_getattr
>                        [e1000e]            [k] e1000_watchdog
>        0.00%           [kernel.vmlinux]    [k] PageHuge
>        0.00%           [kernel.vmlinux]    [k] __acct_update_integrals
>        0.00%           [kernel.vmlinux]    [k] __activate_page
>                        [kernel.vmlinux]    [k] __alloc_fd
>        0.02%   +0.02%  [kernel.vmlinux]    [k] __alloc_pages_nodemask
>        ...
> 
> After:
> 
>   # Baseline    Delta  Shared Object       Symbol
>   # ........  .......  ..................  ................................
>   #
>       24.73%   -4.62%  perf                [.] append_chain_children
>        7.96%   -1.29%  perf                [.] dso__find_symbol
>        6.97%   -2.07%  libc-2.20.so        [.] vfprintf
>        4.61%   +0.88%  libc-2.20.so        [.] __fprintf_chk
>        4.41%   +2.43%  perf                [.] sort__comm_cmp
>        4.10%   -0.16%  perf                [.] comm__str
>        4.03%   -0.93%  perf                [.] machine__findnew_thread_time
>        3.82%   +3.09%  perf                [.] __hists__add_entry
>        2.95%   -0.18%  perf                [.] sort__dso_cmp
>        ...

Looks better, here I tried running 'perf record usleep 1'  in quick
succession to then run 'perf diff'  before and after this patch, got:

Before:

[ssdandy ~]$ cat /tmp/before
# Event 'cycles'
#
# Baseline    Delta  Shared Object     Symbol                        
# ........  .......  ................  ..............................
#
            +36.29%  [kernel.vmlinux]  [k] __split_vma.isra.31       
    34.55%           [kernel.vmlinux]  [k] copy_user_generic_unrolled
             +2.45%  [kernel.vmlinux]  [k] local_clock               
     0.11%   +0.02%  [kernel.vmlinux]  [k] native_write_msr_safe     
     2.12%           [kernel.vmlinux]  [k] perf_event_comm_output    
            +61.13%  [kernel.vmlinux]  [k] unmap_page_range          
    63.22%           libc-2.17.so      [.] 0x000000000007c3e0        
[acme@ssdandy ~]$

After:

[acme@ssdandy linux]$ perf diff
# Event 'cycles'
#
# Baseline    Delta  Shared Object     Symbol                        
# ........  .......  ................  ..............................
#
    63.22%           libc-2.17.so      [.] 0x000000000007c3e0        
    34.55%           [kernel.vmlinux]  [k] copy_user_generic_unrolled
     2.12%           [kernel.vmlinux]  [k] perf_event_comm_output    
     0.11%   +0.02%  [kernel.vmlinux]  [k] native_write_msr_safe     
            +36.29%  [kernel.vmlinux]  [k] __split_vma.isra.31       
             +2.45%  [kernel.vmlinux]  [k] local_clock               
            +61.13%  [kernel.vmlinux]  [k] unmap_page_range          
[acme@ssdandy linux]$

Which was ok up to the point where symbols that only appeared on the
second run were not sorted by delta, can you fix that?

Please let me know if it is better to apply this one then a followup to
sort the deltas or if a combined patch to achieve both is best.

I.e.:

[acme@ssdandy linux]$ perf diff
# Event 'cycles'
#
# Baseline    Delta  Shared Object     Symbol                        
# ........  .......  ................  ..............................
#
    63.22%           libc-2.17.so      [.] 0x000000000007c3e0        
    34.55%           [kernel.vmlinux]  [k] copy_user_generic_unrolled
     2.12%           [kernel.vmlinux]  [k] perf_event_comm_output    
     0.11%   +0.02%  [kernel.vmlinux]  [k] native_write_msr_safe     
            +61.13%  [kernel.vmlinux]  [k] unmap_page_range          
            +36.29%  [kernel.vmlinux]  [k] __split_vma.isra.31       
             +2.45%  [kernel.vmlinux]  [k] local_clock               

[acme@ssdandy linux]$

Humm, but then wouldn't we be more interested in sorting _everything_ by
delta?

- Arnaldo

  reply	other threads:[~2014-12-22 14:45 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-12-22  4:44 [PATCHSET 0/7] perf tools: A small random cleanup and fixups Namhyung Kim
2014-12-22  4:44 ` [PATCH 1/7] perf report: Get rid of report__inc_stat() Namhyung Kim
2015-01-28 15:04   ` [tip:perf/core] " tip-bot for Namhyung Kim
2014-12-22  4:44 ` [PATCH 2/7] perf report: Show progress bar for output resorting Namhyung Kim
2015-01-01 21:28   ` [tip:perf/urgent] " tip-bot for Namhyung Kim
2014-12-22  4:44 ` [PATCH 3/7] perf ui/tui: Print backtrace symbols when segfault occurred Namhyung Kim
2015-01-01 21:28   ` [tip:perf/urgent] perf ui/tui: Print backtrace symbols when segfault occurs tip-bot for Namhyung Kim
2014-12-22  4:44 ` [PATCH 4/7] perf diff: Fix to sort by baseline field by default Namhyung Kim
2014-12-22 14:45   ` Arnaldo Carvalho de Melo [this message]
2014-12-23  4:12     ` Namhyung Kim
2014-12-23 13:30       ` Arnaldo Carvalho de Melo
2014-12-22  4:44 ` [PATCH 5/7] perf diff: Get rid of hists__compute_resort() Namhyung Kim
2014-12-22  4:44 ` [PATCH 6/7] perf tools: Append callchains only when requested Namhyung Kim
2015-01-01 21:29   ` [tip:perf/urgent] perf callchain: " tip-bot for Namhyung Kim
2015-01-03  2:25   ` [PATCH 6/7] perf tools: " Arnaldo Carvalho de Melo
2015-01-03 15:01     ` Namhyung Kim
2015-01-05 12:49       ` Arnaldo Carvalho de Melo
2015-01-07  7:28         ` Namhyung Kim
2014-12-22  4:44 ` [PATCH 7/7] perf tools: Set attr.task bit for a tracking event Namhyung Kim
2014-12-22 14:49   ` Arnaldo Carvalho de Melo
2014-12-23  4:06     ` Namhyung Kim
2014-12-23 13:28       ` Arnaldo Carvalho de Melo

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20141222144513.GB2717@kernel.org \
    --to=acme@kernel.org \
    --cc=a.p.zijlstra@chello.nl \
    --cc=adrian.hunter@intel.com \
    --cc=dsahern@gmail.com \
    --cc=eranian@google.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=namhyung@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.