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

Groups > linux.kernel > #1578225 > unrolled thread

[PATCHSET 0/4] perf diff: Introduce delta-abs compute method (v2)

Started byNamhyung Kim <namhyung@kernel.org>
First post2017-02-10 08:40 +0100
Last post2017-02-10 11:40 +0100
Articles 5 — 2 participants

Back to article view | Back to linux.kernel

Contents

  [PATCHSET 0/4] perf diff: Introduce delta-abs compute method (v2) Namhyung Kim <namhyung@kernel.org> - 2017-02-10 08:40 +0100
    [PATCH v2 3/4] perf diff: Add diff.compute config option Namhyung Kim <namhyung@kernel.org> - 2017-02-10 08:40 +0100
    Re: [PATCHSET 0/4] perf diff: Introduce delta-abs compute method (v2) Ingo Molnar <mingo@kernel.org> - 2017-02-10 08:50 +0100
      Re: [PATCHSET 0/4] perf diff: Introduce delta-abs compute method (v2) Namhyung Kim <namhyung@kernel.org> - 2017-02-10 10:20 +0100
        Re: [PATCHSET 0/4] perf diff: Introduce delta-abs compute method (v2) Ingo Molnar <mingo@kernel.org> - 2017-02-10 11:40 +0100

#1578225 — [PATCHSET 0/4] perf diff: Introduce delta-abs compute method (v2)

FromNamhyung Kim <namhyung@kernel.org>
Date2017-02-10 08:40 +0100
Subject[PATCHSET 0/4] perf diff: Introduce delta-abs compute method (v2)
Message-ID<t9dYd-q5-5@gated-at.bofh.it>
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.

 * v2 changes
  - rebase onto acme/perf/core
  - change default option to '-o 1 -c delta-abs'


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.  And patch 4 changes the default setting.

The code is avaiable at 'perf/diff-delta-abs-v2' branch in

  git://git.kernel.org/pub/scm/linux/kernel/git/namhyung/linux-perf.git

Thanks,
Namhyung


Namhyung Kim (4):
  perf diff: Add 'delta-abs' compute method
  perf diff: Add diff.order config option
  perf diff: Add diff.compute config option
  perf diff: Change default setting to "delta-abs"

 tools/perf/Documentation/perf-config.txt | 12 +++++
 tools/perf/Documentation/perf-diff.txt   | 15 ++++--
 tools/perf/builtin-diff.c                | 78 ++++++++++++++++++++++++++++++--
 3 files changed, 98 insertions(+), 7 deletions(-)

-- 
2.11.0

[toc] | [next] | [standalone]

#1578227 — [PATCH v2 3/4] perf diff: Add diff.compute config option

FromNamhyung Kim <namhyung@kernel.org>
Date2017-02-10 08:40 +0100
Subject[PATCH v2 3/4] perf diff: Add diff.compute config option
Message-ID<t9dYe-q5-19@gated-at.bofh.it>
In reply to#1578225
The diff.compute config variable is to set the default compute method of
perf diff command (-c option).  Possible values 'delta' (default),
'delta-abs', 'ratio' and 'wdiff'.

Cc: Taeung Song <treeze.taeung@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Documentation/perf-config.txt |  5 +++++
 tools/perf/Documentation/perf-diff.txt   |  5 +++--
 tools/perf/builtin-diff.c                | 16 +++++++++++++++-
 3 files changed, 23 insertions(+), 3 deletions(-)

diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt
index 5b54d47ef713..f2d758dc1edc 100644
--- a/tools/perf/Documentation/perf-config.txt
+++ b/tools/perf/Documentation/perf-config.txt
@@ -505,6 +505,11 @@ Variables
 		Setting it to 1 will sort the result by delta (or other
 		compute method selected).
 
+	diff.compute::
+		This options sets the method of computing diff result.
+		Possible values are 'delta', 'delta-abs', 'ratio' and
+		'wdiff'.  Default is 'delta'.
+
 SEE ALSO
 --------
 linkperf:perf[1]
diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt
index 6ba3bf582d79..70f490408262 100644
--- a/tools/perf/Documentation/perf-diff.txt
+++ b/tools/perf/Documentation/perf-diff.txt
@@ -86,8 +86,9 @@ OPTIONS
 
 -c::
 --compute::
-        Differential computation selection - delta,ratio,wdiff,delta-abs (default is delta).
-        See COMPARISON METHODS section for more info.
+        Differential computation selection - delta,ratio,wdiff,delta-abs
+	(default is delta).  Default can be changed using diff.compute
+	config option.  See COMPARISON METHODS section for more info.
 
 -p::
 --period::
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 181ff996e039..4b4004d41c6a 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -86,7 +86,7 @@ const char *compute_names[COMPUTE_MAX] = {
 	[COMPUTE_WEIGHTED_DIFF] = "wdiff",
 };
 
-static int compute;
+static int compute = COMPUTE_DELTA;
 
 static int compute_2_hpp[COMPUTE_MAX] = {
 	[COMPUTE_DELTA]		= PERF_HPP_DIFF__DELTA,
@@ -1299,6 +1299,20 @@ static int diff__config(const char *var, const char *value,
 		sort_compute = perf_config_int(var, value);
 		return 0;
 	}
+	if (!strcmp(var, "diff.compute")) {
+		if (!strcmp(value, "delta"))
+			compute = COMPUTE_DELTA;
+		else if (!strcmp(value, "delta-abs"))
+			compute = COMPUTE_DELTA_ABS;
+		else if (!strcmp(value, "ratio"))
+			compute = COMPUTE_RATIO;
+		else if (!strcmp(value, "wdiff"))
+			compute = COMPUTE_WEIGHTED_DIFF;
+		else {
+			pr_err("Invalid compute method: %s\n", value);
+			return -1;
+		}
+	}
 
 	return 0;
 }
-- 
2.11.0

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

#1578229

FromIngo Molnar <mingo@kernel.org>
Date2017-02-10 08:50 +0100
Message-ID<t9e7T-tH-1@gated-at.bofh.it>
In reply to#1578225
* Namhyung Kim <namhyung@kernel.org> wrote:

> 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.
> 
>  * v2 changes
>   - rebase onto acme/perf/core
>   - change default option to '-o 1 -c delta-abs'
> 
> 
> 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

Nice!

BTW., to me the second output looks a lot more intuitive and useful than the 
default one.

Would it be possible to flip over the default to the 'most useful' options, and 
see whether anyone complains?

Thanks,

	Ingo

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

#1578324

FromNamhyung Kim <namhyung@kernel.org>
Date2017-02-10 10:20 +0100
Message-ID<t9fwZ-1tL-7@gated-at.bofh.it>
In reply to#1578229
Hi Ingo,

On Fri, Feb 10, 2017 at 08:41:11AM +0100, Ingo Molnar wrote:
> 
> * Namhyung Kim <namhyung@kernel.org> wrote:
> 
> > 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.
> > 
> >  * v2 changes
> >   - rebase onto acme/perf/core
> >   - change default option to '-o 1 -c delta-abs'
> > 
> > 
> > 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
> 
> Nice!
> 
> BTW., to me the second output looks a lot more intuitive and useful than the 
> default one.

Thanks!

> 
> Would it be possible to flip over the default to the 'most useful' options, and 
> see whether anyone complains?

The patch 4 makes it default.

By the way, the current default (sort by baseline) looks like below:

  $ perf diff | 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
     2.40%   +0.95%  [kernel.kallsyms]  [k] bit_putcs
     1.35%   -0.71%  [kernel.kallsyms]  [k] smp_call_function_single
     0.62%   +1.23%  [kernel.kallsyms]  [k] mutex_lock
     0.31%   +0.79%  [kernel.kallsyms]  [k] link_path_walk
     0.16%   +0.45%  [kernel.kallsyms]  [k] menu_select
     0.00%   +0.57%  [kernel.kallsyms]  [k] __rcu_read_unlock

While this example doesn't show much difference, it could be..

Thanks,
Namhyung

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

#1578369

FromIngo Molnar <mingo@kernel.org>
Date2017-02-10 11:40 +0100
Message-ID<t9gMp-2dz-1@gated-at.bofh.it>
In reply to#1578324
* Namhyung Kim <namhyung@kernel.org> wrote:

> > Would it be possible to flip over the default to the 'most useful' options, and 
> > see whether anyone complains?
> 
> The patch 4 makes it default.

/me must be blind!

Yes, looks really nice that way.

Thanks,

	Ingo

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web