Re: [PATCH 13/16] perf callchain: Switch default to 'graph,0.5,caller'
From: Arnaldo Carvalho de Melo
Date: Tue Oct 20 2015 - 09:38:26 EST
Em Tue, Oct 20, 2015 at 09:23:12PM +0800, Wangnan (F) escreveu:
> On 2015/10/6 5:03, Arnaldo Carvalho de Melo wrote:
> >From: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> >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?
I was bitten by the --children thing and took some time to get used to
it, so I can relate to that...
I think we should revert this change in callchain default, enough
complaints... Ingo, since you suggested that change, what are your
thoughts?
Changing defaults is hard, there is also the horizontal scrolling that
made we repurpose the right and left arrows, sigh, that one will cause
some confusion as well...
It seems we'll need way more preparation for such changes, more
infrastructure to ease the transition, questioning if the user wants
that, etc, growing pains :-\
- Arnaldo
> Thank you.
>
> >Requested-by: Ingo Molnar <mingo@xxxxxxxxxx>
> >Cc: Adrian Hunter <adrian.hunter@xxxxxxxxx>
> >Cc: Borislav Petkov <bp@xxxxxxx>
> >Cc: Chandler Carruth <chandlerc@xxxxxxxxx>
> >Cc: David Ahern <dsahern@xxxxxxxxx>
> >Cc: Frederic Weisbecker <fweisbec@xxxxxxxxx>
> >Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
> >Cc: Namhyung Kim <namhyung@xxxxxxxxxx>
> >Cc: Stephane Eranian <eranian@xxxxxxxxxx>
> >Cc: Wang Nan <wangnan0@xxxxxxxxxx>
> >Link: https://www.youtube.com/watch?v=nXaxk27zwlk
> >Link: http://lkml.kernel.org/n/tip-v8lq36aispvdwgxdmt9p9jd9@xxxxxxxxxxxxxx
> >Signed-off-by: Arnaldo Carvalho de Melo <acme@xxxxxxxxxx>
> >---
> > 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/