From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751468AbdBFJou (ORCPT ); Mon, 6 Feb 2017 04:44:50 -0500 Received: from mail-pg0-f66.google.com ([74.125.83.66]:33638 "EHLO mail-pg0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750980AbdBFJot (ORCPT ); Mon, 6 Feb 2017 04:44:49 -0500 Subject: Re: [PATCH 2/3] perf diff: Add diff.order config option To: Namhyung Kim References: <20170206072037.8189-1-namhyung@kernel.org> <20170206072037.8189-3-namhyung@kernel.org> Cc: Arnaldo Carvalho de Melo , Ingo Molnar , Peter Zijlstra , Jiri Olsa , LKML , Minchan Kim From: Taeung Song Message-ID: <26102984-3bb7-ea2b-fe24-77809af5d61a@gmail.com> Date: Mon, 6 Feb 2017 18:44:42 +0900 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: <20170206072037.8189-3-namhyung@kernel.org> Content-Type: text/plain; charset=windows-1252; format=flowed Content-Transfer-Encoding: 7bit Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 > Signed-off-by: Namhyung Kim > --- > 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 > #include > @@ -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) >