Messages in this thread | | | Date | Mon, 6 Feb 2017 22:41:59 +0900 | From | Namhyung Kim <> | Subject | Re: [PATCH 2/3] perf diff: Add diff.order config option |
| |
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) > >
| |