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

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

[Namhyung Kim]

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

[PATCH 1/3] perf diff: Add 'delta-abs' compute method

[Namhyung Kim]

The 'delta-abs' compute method 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
     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.00%   +0.57%  [kernel.kallsyms]  [k] __rcu_read_unlock
     0.16%   +0.45%  [kernel.kallsyms]  [k] menu_select
     0.72%   -0.44%  [kernel.kallsyms]  [k] lookup_fast

Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Documentation/perf-diff.txt |  6 ++++-
 tools/perf/builtin-diff.c              | 46 ++++++++++++++++++++++++++++++++--
 2 files changed, 49 insertions(+), 3 deletions(-)

diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt
index 3e9490b9c533..af80284cd2f6 100644
--- a/tools/perf/Documentation/perf-diff.txt
+++ b/tools/perf/Documentation/perf-diff.txt
@@ -86,7 +86,7 @@ OPTIONS
 
 -c::
 --compute::
-        Differential computation selection - delta,ratio,wdiff (default is delta).
+        Differential computation selection - delta,ratio,wdiff,delta-abs (default is delta).
         See COMPARISON METHODS section for more info.
 
 -p::
@@ -181,6 +181,10 @@ delta
     relative to how entries are filtered.  Use --percentage=absolute to
     prevent such fluctuation.
 
+delta-abs
+~~~~~~~~~
+Same as 'delta` method, but sort the result with the absolute values.
+
 ratio
 ~~~~~
 If specified the 'Ratio' column is displayed with value 'r' computed as:
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 933aeec46f4a..781c9e60bd21 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -30,6 +30,7 @@ enum {
 	PERF_HPP_DIFF__RATIO,
 	PERF_HPP_DIFF__WEIGHTED_DIFF,
 	PERF_HPP_DIFF__FORMULA,
+	PERF_HPP_DIFF__DELTA_ABS,
 
 	PERF_HPP_DIFF__MAX_INDEX
 };
@@ -73,11 +74,13 @@ enum {
 	COMPUTE_DELTA,
 	COMPUTE_RATIO,
 	COMPUTE_WEIGHTED_DIFF,
+	COMPUTE_DELTA_ABS,
 	COMPUTE_MAX,
 };
 
 const char *compute_names[COMPUTE_MAX] = {
 	[COMPUTE_DELTA] = "delta",
+	[COMPUTE_DELTA_ABS] = "delta-abs",
 	[COMPUTE_RATIO] = "ratio",
 	[COMPUTE_WEIGHTED_DIFF] = "wdiff",
 };
@@ -86,6 +89,7 @@ static int compute;
 
 static int compute_2_hpp[COMPUTE_MAX] = {
 	[COMPUTE_DELTA]		= PERF_HPP_DIFF__DELTA,
+	[COMPUTE_DELTA_ABS]	= PERF_HPP_DIFF__DELTA_ABS,
 	[COMPUTE_RATIO]		= PERF_HPP_DIFF__RATIO,
 	[COMPUTE_WEIGHTED_DIFF]	= PERF_HPP_DIFF__WEIGHTED_DIFF,
 };
@@ -111,6 +115,10 @@ static struct header_column {
 		.name  = "Delta",
 		.width = 7,
 	},
+	[PERF_HPP_DIFF__DELTA_ABS] = {
+		.name  = "Delta Abs",
+		.width = 7,
+	},
 	[PERF_HPP_DIFF__RATIO] = {
 		.name  = "Ratio",
 		.width = 14,
@@ -298,6 +306,7 @@ static int formula_fprintf(struct hist_entry *he, struct hist_entry *pair,
 {
 	switch (compute) {
 	case COMPUTE_DELTA:
+	case COMPUTE_DELTA_ABS:
 		return formula_delta(he, pair, buf, size);
 	case COMPUTE_RATIO:
 		return formula_ratio(he, pair, buf, size);
@@ -461,6 +470,7 @@ static void hists__precompute(struct hists *hists)
 
 			switch (compute) {
 			case COMPUTE_DELTA:
+			case COMPUTE_DELTA_ABS:
 				compute_delta(he, pair);
 				break;
 			case COMPUTE_RATIO:
@@ -498,6 +508,13 @@ __hist_entry__cmp_compute(struct hist_entry *left, struct hist_entry *right,
 
 		return cmp_doubles(l, r);
 	}
+	case COMPUTE_DELTA_ABS:
+	{
+		double l = fabs(left->diff.period_ratio_delta);
+		double r = fabs(right->diff.period_ratio_delta);
+
+		return cmp_doubles(l, r);
+	}
 	case COMPUTE_RATIO:
 	{
 		double l = left->diff.period_ratio;
@@ -564,7 +581,7 @@ hist_entry__cmp_compute_idx(struct hist_entry *left, struct hist_entry *right,
 	if (!p_left || !p_right)
 		return p_left ? -1 : 1;
 
-	if (c != COMPUTE_DELTA) {
+	if (c != COMPUTE_DELTA && c != COMPUTE_DELTA_ABS) {
 		/*
 		 * The delta can be computed without the baseline, but
 		 * others are not.  Put those entries which have no
@@ -607,6 +624,15 @@ hist_entry__cmp_delta(struct perf_hpp_fmt *fmt,
 }
 
 static int64_t
+hist_entry__cmp_delta_abs(struct perf_hpp_fmt *fmt,
+		      struct hist_entry *left, struct hist_entry *right)
+{
+	struct data__file *d = fmt_to_data_file(fmt);
+
+	return hist_entry__cmp_compute(right, left, COMPUTE_DELTA_ABS, d->idx);
+}
+
+static int64_t
 hist_entry__cmp_ratio(struct perf_hpp_fmt *fmt,
 		      struct hist_entry *left, struct hist_entry *right)
 {
@@ -633,6 +659,14 @@ hist_entry__cmp_delta_idx(struct perf_hpp_fmt *fmt __maybe_unused,
 }
 
 static int64_t
+hist_entry__cmp_delta_abs_idx(struct perf_hpp_fmt *fmt __maybe_unused,
+			      struct hist_entry *left, struct hist_entry *right)
+{
+	return hist_entry__cmp_compute_idx(right, left, COMPUTE_DELTA_ABS,
+					   sort_compute);
+}
+
+static int64_t
 hist_entry__cmp_ratio_idx(struct perf_hpp_fmt *fmt __maybe_unused,
 			  struct hist_entry *left, struct hist_entry *right)
 {
@@ -775,7 +809,7 @@ static const struct option options[] = {
 	OPT_BOOLEAN('b', "baseline-only", &show_baseline_only,
 		    "Show only items with match in baseline"),
 	OPT_CALLBACK('c', "compute", &compute,
-		     "delta,ratio,wdiff:w1,w2 (default delta)",
+		     "delta,delta-abs,ratio,wdiff:w1,w2 (default delta)",
 		     "Entries differential computation selection",
 		     setup_compute),
 	OPT_BOOLEAN('p', "period", &show_period,
@@ -945,6 +979,7 @@ hpp__entry_pair(struct hist_entry *he, struct hist_entry *pair,
 
 	switch (idx) {
 	case PERF_HPP_DIFF__DELTA:
+	case PERF_HPP_DIFF__DELTA_ABS:
 		if (pair->diff.computed)
 			diff = pair->diff.period_ratio_delta;
 		else
@@ -1118,6 +1153,10 @@ static void data__hpp_register(struct data__file *d, int idx)
 		fmt->color = hpp__color_wdiff;
 		fmt->sort  = hist_entry__cmp_wdiff;
 		break;
+	case PERF_HPP_DIFF__DELTA_ABS:
+		fmt->color = hpp__color_delta;
+		fmt->sort  = hist_entry__cmp_delta_abs;
+		break;
 	default:
 		fmt->sort  = hist_entry__cmp_nop;
 		break;
@@ -1195,6 +1234,9 @@ static int ui_init(void)
 	case COMPUTE_WEIGHTED_DIFF:
 		fmt->sort = hist_entry__cmp_wdiff_idx;
 		break;
+	case COMPUTE_DELTA_ABS:
+		fmt->sort = hist_entry__cmp_delta_abs_idx;
+		break;
 	default:
 		BUG_ON(1);
 	}
-- 
2.11.0

[PATCH 2/3] perf diff: Add diff.order config option

[Namhyung Kim]

In many cases, I need to look at differences between two data so I often
used the -o option to sort the result base on the difference first.
It'd be nice to have a config option to set it by default.

The diff.order config option is to set the default value of -o/--order
option.

Cc: Taeung Song <treeze.taeung@gmail.com>
Signed-off-by: Namhyung Kim <namhyung@kernel.org>
---
 tools/perf/Documentation/perf-config.txt |  7 +++++++
 tools/perf/Documentation/perf-diff.txt   |  6 +++++-
 tools/perf/builtin-diff.c                | 14 ++++++++++++++
 3 files changed, 26 insertions(+), 1 deletion(-)

diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt
index 9365b75fd04f..5b54d47ef713 100644
--- a/tools/perf/Documentation/perf-config.txt
+++ b/tools/perf/Documentation/perf-config.txt
@@ -498,6 +498,13 @@ Variables
 		But if this option is 'no-cache', it will not update the build-id cache.
 		'skip' skips post-processing and does not update the cache.
 
+diff.*::
+	diff.order::
+		This option sets the number of column to sort the result.
+		Default is 0 which means sorting by baseline.
+		Setting it to 1 will sort the result by delta (or other
+		compute method selected).
+
 SEE ALSO
 --------
 linkperf:perf[1]
diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt
index af80284cd2f6..6ba3bf582d79 100644
--- a/tools/perf/Documentation/perf-diff.txt
+++ b/tools/perf/Documentation/perf-diff.txt
@@ -99,7 +99,11 @@ OPTIONS
 
 -o::
 --order::
-       Specify compute sorting column number.
+       Specify compute sorting column number.  0 means sorting by baseline
+       overhead (default) and 1 means sorting by computed value of column 1
+       (data from the first file other base baseline).  Values more than 1
+       can be used only if enough data files are provided.
+       Default value can be set using diff.order config option.
 
 --percentage::
 	Determine how to display the overhead percentage of filtered entries.
diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
index 781c9e60bd21..181ff996e039 100644
--- a/tools/perf/builtin-diff.c
+++ b/tools/perf/builtin-diff.c
@@ -17,6 +17,7 @@
 #include "util/symbol.h"
 #include "util/util.h"
 #include "util/data.h"
+#include "util/config.h"
 
 #include <stdlib.h>
 #include <math.h>
@@ -1291,6 +1292,17 @@ static int data_init(int argc, const char **argv)
 	return 0;
 }
 
+static int diff__config(const char *var, const char *value,
+			void *cb __maybe_unused)
+{
+	if (!strcmp(var, "diff.order")) {
+		sort_compute = perf_config_int(var, value);
+		return 0;
+	}
+
+	return 0;
+}
+
 int cmd_diff(int argc, const char **argv, const char *prefix __maybe_unused)
 {
 	int ret = hists__init();
@@ -1298,6 +1310,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix __maybe_unused)
 	if (ret < 0)
 		return ret;
 
+	perf_config(diff__config, NULL);
+
 	argc = parse_options(argc, argv, options, diff_usage, 0);
 
 	if (symbol__init(NULL) < 0)
-- 
2.11.0

[PATCH 3/3] perf diff: Add diff.compute config option

[Namhyung Kim]

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

Re: [PATCH 2/3] perf diff: Add diff.order config option

[Taeung Song]

Hi, Namhyung :)

On 02/06/2017 04:20 PM, Namhyung Kim wrote:
> In many cases, I need to look at differences between two data so I often
> used the -o option to sort the result base on the difference first.
> It'd be nice to have a config option to set it by default.
>
> The diff.order config option is to set the default value of -o/--order
> option.
>
> Cc: Taeung Song <treeze.taeung@gmail.com>
> Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> ---
>  tools/perf/Documentation/perf-config.txt |  7 +++++++
>  tools/perf/Documentation/perf-diff.txt   |  6 +++++-
>  tools/perf/builtin-diff.c                | 14 ++++++++++++++
>  3 files changed, 26 insertions(+), 1 deletion(-)
>
> diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt
> index 9365b75fd04f..5b54d47ef713 100644
> --- a/tools/perf/Documentation/perf-config.txt
> +++ b/tools/perf/Documentation/perf-config.txt
> @@ -498,6 +498,13 @@ Variables
>  		But if this option is 'no-cache', it will not update the build-id cache.
>  		'skip' skips post-processing and does not update the cache.
>
> +diff.*::
> +	diff.order::
> +		This option sets the number of column to sort the result.
> +		Default is 0 which means sorting by baseline.
> +		Setting it to 1 will sort the result by delta (or other
> +		compute method selected).
> +
>  SEE ALSO
>  --------
>  linkperf:perf[1]
> diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt
> index af80284cd2f6..6ba3bf582d79 100644
> --- a/tools/perf/Documentation/perf-diff.txt
> +++ b/tools/perf/Documentation/perf-diff.txt
> @@ -99,7 +99,11 @@ OPTIONS
>
>  -o::
>  --order::
> -       Specify compute sorting column number.
> +       Specify compute sorting column number.  0 means sorting by baseline
> +       overhead (default) and 1 means sorting by computed value of column 1
> +       (data from the first file other base baseline).  Values more than 1
> +       can be used only if enough data files are provided.
> +       Default value can be set using diff.order config option.
>
>  --percentage::
>  	Determine how to display the overhead percentage of filtered entries.
> diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
> index 781c9e60bd21..181ff996e039 100644
> --- a/tools/perf/builtin-diff.c
> +++ b/tools/perf/builtin-diff.c
> @@ -17,6 +17,7 @@
>  #include "util/symbol.h"
>  #include "util/util.h"
>  #include "util/data.h"
> +#include "util/config.h"
>
>  #include <stdlib.h>
>  #include <math.h>
> @@ -1291,6 +1292,17 @@ static int data_init(int argc, const char **argv)
>  	return 0;
>  }
>
> +static int diff__config(const char *var, const char *value,
> +			void *cb __maybe_unused)
> +{

What about adding if statement as below ?
(such as builtin-ftrace.c:perf_ftrace_config)

          if (prefixcmp(var, "diff."))
                  return 0;

> +	if (!strcmp(var, "diff.order")) {

And.. var + 5 instead of var as below ?
(such as perf.c:pager_command_config)

          if (!strcmp(var + 5, "order"))

It is just my opinion. :)

Thanks,
Taeung

> +		sort_compute = perf_config_int(var, value);
> +		return 0;
> +	}
> +
> +	return 0;
> +}
> +
>  int cmd_diff(int argc, const char **argv, const char *prefix __maybe_unused)
>  {
>  	int ret = hists__init();
> @@ -1298,6 +1310,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix __maybe_unused)
>  	if (ret < 0)
>  		return ret;
>
> +	perf_config(diff__config, NULL);
> +
>  	argc = parse_options(argc, argv, options, diff_usage, 0);
>
>  	if (symbol__init(NULL) < 0)
>

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

[Jiri Olsa]

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

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

Re: [PATCH 2/3] perf diff: Add diff.order config option

[Namhyung Kim]

Hi Taeung,

On Mon, Feb 06, 2017 at 06:44:42PM +0900, Taeung Song wrote:
> Hi, Namhyung :)
> 
> On 02/06/2017 04:20 PM, Namhyung Kim wrote:
> > In many cases, I need to look at differences between two data so I often
> > used the -o option to sort the result base on the difference first.
> > It'd be nice to have a config option to set it by default.
> > 
> > The diff.order config option is to set the default value of -o/--order
> > option.
> > 
> > Cc: Taeung Song <treeze.taeung@gmail.com>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> > ---
> >  tools/perf/Documentation/perf-config.txt |  7 +++++++
> >  tools/perf/Documentation/perf-diff.txt   |  6 +++++-
> >  tools/perf/builtin-diff.c                | 14 ++++++++++++++
> >  3 files changed, 26 insertions(+), 1 deletion(-)
> > 
> > diff --git a/tools/perf/Documentation/perf-config.txt b/tools/perf/Documentation/perf-config.txt
> > index 9365b75fd04f..5b54d47ef713 100644
> > --- a/tools/perf/Documentation/perf-config.txt
> > +++ b/tools/perf/Documentation/perf-config.txt
> > @@ -498,6 +498,13 @@ Variables
> >  		But if this option is 'no-cache', it will not update the build-id cache.
> >  		'skip' skips post-processing and does not update the cache.
> > 
> > +diff.*::
> > +	diff.order::
> > +		This option sets the number of column to sort the result.
> > +		Default is 0 which means sorting by baseline.
> > +		Setting it to 1 will sort the result by delta (or other
> > +		compute method selected).
> > +
> >  SEE ALSO
> >  --------
> >  linkperf:perf[1]
> > diff --git a/tools/perf/Documentation/perf-diff.txt b/tools/perf/Documentation/perf-diff.txt
> > index af80284cd2f6..6ba3bf582d79 100644
> > --- a/tools/perf/Documentation/perf-diff.txt
> > +++ b/tools/perf/Documentation/perf-diff.txt
> > @@ -99,7 +99,11 @@ OPTIONS
> > 
> >  -o::
> >  --order::
> > -       Specify compute sorting column number.
> > +       Specify compute sorting column number.  0 means sorting by baseline
> > +       overhead (default) and 1 means sorting by computed value of column 1
> > +       (data from the first file other base baseline).  Values more than 1
> > +       can be used only if enough data files are provided.
> > +       Default value can be set using diff.order config option.
> > 
> >  --percentage::
> >  	Determine how to display the overhead percentage of filtered entries.
> > diff --git a/tools/perf/builtin-diff.c b/tools/perf/builtin-diff.c
> > index 781c9e60bd21..181ff996e039 100644
> > --- a/tools/perf/builtin-diff.c
> > +++ b/tools/perf/builtin-diff.c
> > @@ -17,6 +17,7 @@
> >  #include "util/symbol.h"
> >  #include "util/util.h"
> >  #include "util/data.h"
> > +#include "util/config.h"
> > 
> >  #include <stdlib.h>
> >  #include <math.h>
> > @@ -1291,6 +1292,17 @@ static int data_init(int argc, const char **argv)
> >  	return 0;
> >  }
> > 
> > +static int diff__config(const char *var, const char *value,
> > +			void *cb __maybe_unused)
> > +{
> 
> What about adding if statement as below ?
> (such as builtin-ftrace.c:perf_ftrace_config)
> 
>          if (prefixcmp(var, "diff."))
>                  return 0;
> 
> > +	if (!strcmp(var, "diff.order")) {
> 
> And.. var + 5 instead of var as below ?
> (such as perf.c:pager_command_config)
> 
>          if (!strcmp(var + 5, "order"))
> 
> It is just my opinion. :)

Yep, looks good.  Will change if I send v2.

Thanks,
Namhyung


> 
> > +		sort_compute = perf_config_int(var, value);
> > +		return 0;
> > +	}
> > +
> > +	return 0;
> > +}
> > +
> >  int cmd_diff(int argc, const char **argv, const char *prefix __maybe_unused)
> >  {
> >  	int ret = hists__init();
> > @@ -1298,6 +1310,8 @@ int cmd_diff(int argc, const char **argv, const char *prefix __maybe_unused)
> >  	if (ret < 0)
> >  		return ret;
> > 
> > +	perf_config(diff__config, NULL);
> > +
> >  	argc = parse_options(argc, argv, options, diff_usage, 0);
> > 
> >  	if (symbol__init(NULL) < 0)
> > 

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

[Namhyung Kim]

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

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

[Arnaldo Carvalho de Melo]

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

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

[Namhyung Kim]

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

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

[Arnaldo Carvalho de Melo]

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

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

[Namhyung Kim]

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