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


Groups > linux.kernel > #1574412 > unrolled thread

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

Started byNamhyung Kim <namhyung@kernel.org>
First post2017-02-06 08:30 +0100
Last post2017-02-06 14:50 +0100
Articles 3 — 2 participants

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.


Contents

  [PATCH 2/3] perf diff: Add diff.order config option Namhyung Kim <namhyung@kernel.org> - 2017-02-06 08:30 +0100
    Re: [PATCH 2/3] perf diff: Add diff.order config option Taeung Song <treeze.taeung@gmail.com> - 2017-02-06 10:50 +0100
      Re: [PATCH 2/3] perf diff: Add diff.order config option Namhyung Kim <namhyung@kernel.org> - 2017-02-06 14:50 +0100

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

FromNamhyung Kim <namhyung@kernel.org>
Date2017-02-06 08:30 +0100
Subject[PATCH 2/3] perf diff: Add diff.order config option
Message-ID<t7LUl-1Ov-1@gated-at.bofh.it>
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

[toc] | [next] | [standalone]


#1574505

FromTaeung Song <treeze.taeung@gmail.com>
Date2017-02-06 10:50 +0100
Message-ID<t7O5P-3el-11@gated-at.bofh.it>
In reply to#1574412
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)
>

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


#1574762

FromNamhyung Kim <namhyung@kernel.org>
Date2017-02-06 14:50 +0100
Message-ID<t7RQ8-5AW-83@gated-at.bofh.it>
In reply to#1574505
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)
> > 

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web