Date:	Tue, 20 Oct 2015 21:23:12 +0800
From:	"Wangnan (F)" <wangnan0@...wei.com>
To:	Arnaldo Carvalho de Melo <acme@...nel.org>,
	Ingo Molnar <mingo@...nel.org>
CC:	<linux-kernel@...r.kernel.org>,
	Arnaldo Carvalho de Melo <acme@...hat.com>,
	Adrian Hunter <adrian.hunter@...el.com>,
	Borislav Petkov <bp@...e.de>,
	Chandler Carruth <chandlerc@...il.com>,
	David Ahern <dsahern@...il.com>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Jiri Olsa <jolsa@...hat.com>,
	Namhyung Kim <namhyung@...nel.org>,
	Stephane Eranian <eranian@...gle.com>,
	pi3orama <pi3orama@....com>
Subject: Re: [PATCH 13/16] perf callchain: Switch default to 'graph,0.5,caller'

Hi Arnaldo,

On 2015/10/6 5:03, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo <acme@...hat.com>
>
> Which is the most common default found in other similar tools.

Could you please show me some example about "other similar tools"?
For me, in most of the case I prefer callee order because most of my
task is to explain the reason why some code get executed too much times
than expected.

Also, I think changing default settings should be careful.

This is my story: after switching to new version of perf, in a period of
time there are plenty of perf users in my company be confused by the
first column of 'perf report' because the sum of the percentage listed
there is much higher than 100%. They find me because they think this is
a bug in perf which breaks their routinely profiling work.  The
"problem" is caused by the adding of "--children". New perf makes
'--children' as the default behavior at the first time it support that
option, but the old perf shows things similar to '--no-children'.
However, it is hard to explain the principle of call stack accumulation
and why we need '--children' to those perf users (they learned perf's
command line from others, and don't have enought to read perf
documentations or even help output. Althought the title of the first
column is changed to 'Children', I don't think they can understand the
meaning of it. I think some of them didn't even notice there's an
addition column in their output. They just confused and angry). Also,
and as you can expect, this change breaks some scripts. In those days I
have to make our IM tool response the information of "--no-children"
automatically.

This patch changes the default output again. Similar thing will happen
another time. I think this time I can make some preparation, for example,
prepare new script to restore old behavior?

Thank you.

> Requested-by: Ingo Molnar <mingo@...nel.org>
> Cc: Adrian Hunter <adrian.hunter@...el.com>
> Cc: Borislav Petkov <bp@...e.de>
> Cc: Chandler Carruth <chandlerc@...il.com>
> Cc: David Ahern <dsahern@...il.com>
> Cc: Frederic Weisbecker <fweisbec@...il.com>
> Cc: Jiri Olsa <jolsa@...hat.com>
> Cc: Namhyung Kim <namhyung@...nel.org>
> Cc: Stephane Eranian <eranian@...gle.com>
> Cc: Wang Nan <wangnan0@...wei.com>
> Link: https://www.youtube.com/watch?v=nXaxk27zwlk
> Link: http://lkml.kernel.org/n/tip-v8lq36aispvdwgxdmt9p9jd9@git.kernel.org
> Signed-off-by: Arnaldo Carvalho de Melo <acme@...hat.com>
> ---
>   tools/perf/Documentation/perf-report.txt | 2 +-
>   tools/perf/builtin-report.c              | 4 ++--
>   tools/perf/util/util.c                   | 4 ++--
>   3 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
> index ce499035e6d8..e4fdeeb51123 100644
> --- a/tools/perf/Documentation/perf-report.txt
> +++ b/tools/perf/Documentation/perf-report.txt
> @@ -192,7 +192,7 @@ OPTIONS
>   	when available. Usually more convenient to use --branch-history
>   	for this.
>   
> -	Default: fractal,0.5,callee,function.
> +	Default: graph,0.5,caller
>   
>   --children::
>   	Accumulate callchain of children to parent entry so that then can
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index b5623639f67d..3b23b25d1589 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -633,7 +633,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
>   	bool has_br_stack = false;
>   	int branch_mode = -1;
>   	bool branch_call_mode = false;
> -	char callchain_default_opt[] = "fractal,0.5,callee";
> +	char callchain_default_opt[] = "graph,0.5,caller";
>   	const char * const report_usage[] = {
>   		"perf report [<options>]",
>   		NULL
> @@ -701,7 +701,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __maybe_unused)
>   		    "Only display entries with parent-match"),
>   	OPT_CALLBACK_DEFAULT('g', "call-graph", &report, "output_type,min_percent[,print_limit],call_order[,branch]",
>   		     "Display callchains using output_type (graph, flat, fractal, or none) , min percent threshold, optional print limit, callchain order, key (function or address), add branches. "
> -		     "Default: fractal,0.5,callee,function", &report_parse_callchain_opt, callchain_default_opt),
> +		     "Default: graph,0.5,caller", &report_parse_callchain_opt, callchain_default_opt),
>   	OPT_BOOLEAN(0, "children", &symbol_conf.cumulate_callchain,
>   		    "Accumulate callchains of children and show total overhead as well"),
>   	OPT_INTEGER(0, "max-stack", &report.max_stack,
> diff --git a/tools/perf/util/util.c b/tools/perf/util/util.c
> index ce465b259e52..c1bf9ff210b0 100644
> --- a/tools/perf/util/util.c
> +++ b/tools/perf/util/util.c
> @@ -17,9 +17,9 @@
>   #include "callchain.h"
>   
>   struct callchain_param	callchain_param = {
> -	.mode	= CHAIN_GRAPH_REL,
> +	.mode	= CHAIN_GRAPH_ABS,
>   	.min_percent = 0.5,
> -	.order  = ORDER_CALLEE,
> +	.order  = ORDER_CALLER,
>   	.key	= CCKEY_FUNCTION
>   };
>   


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Powered by blists - more mailing lists