Re: [RFC/PATCH 3/3] perf tools: Defaults to 'caller' callchain order only if --children is enabled

From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Namhyung Kim <namhyung@kernel.org>
Cc: Ingo Molnar <mingo@kernel.org>,
	Peter Zijlstra <a.p.zijlstra@chello.nl>,
	Jiri Olsa <jolsa@redhat.com>, LKML <linux-kernel@vger.kernel.org>,
	David Ahern <dsahern@gmail.com>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Borislav Petkov <bp@suse.de>,
	Brendan Gregg <brendan.d.gregg@gmail.com>,
	Chandler Carruth <chandlerc@gmail.com>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Stephane Eranian <eranian@google.com>,
	Wang Nan <wangnan0@huawei.com>
Subject: Re: [RFC/PATCH 3/3] perf tools: Defaults to 'caller' callchain order only if --children is enabled
Date: Thu, 22 Oct 2015 11:37:04 -0300	[thread overview]
Message-ID: <20151022143704.GD2455@kernel.org> (raw)
In-Reply-To: <CAM9d7ciOk5ZAoKQNC6EwRvJfP5R3x-pHVRORuMz+vG==xi1nJg@mail.gmail.com>

Em Thu, Oct 22, 2015 at 11:22:40PM +0900, Namhyung Kim escreveu:
> On Thu, Oct 22, 2015 at 11:13 PM, Arnaldo Carvalho de Melo <acme@kernel.org> wrote:
> > Em Thu, Oct 22, 2015 at 03:28:50PM +0900, Namhyung Kim escreveu:
> >> +++ b/tools/perf/util/util.c
> >> @@ -19,7 +19,7 @@
> >>  struct callchain_param       callchain_param = {
> >>       .mode   = CHAIN_GRAPH_ABS,
> >> -     .order  = ORDER_CALLER,
> >> +     .order  = ORDER_CALLEE,

> > So, this isn't a revert of the previous default change, i.e. previously
> > we used CHAIN_GRAPH_REL, should we keep CHAIN_GRAPH_ABS + callee?

> Yes, it's not a revert.  I intentionally didn't change the print type.

Ok, its just that I don't recall seeing the part of the discussion about
keeping the change from REL to ABS.

> > Also you forgot to fix the docs to mention this new default, I did it
> > and the resulting patch, still with GRAPH_ABS follows:

> This is not simple.  As Brendan said, --children is default on, so
> users will see 'caller' ordering that's why I didn't change the doc.

Ok, but the default as it stands after applying this patch is "graph,
calee", so the docs should be changed to reflect that, yes, its not just
that, we need to tell, in the --children doc, that it defaults to
"caller".

Will we also flip the default to --no-children? I would advocate that,
together with showing a info box telling the user about this change and
how to ask for it, including instructions on how to do that via
~/.perfconfig.

> But I should mention it'll use 'callee' ordering when --no-chlidren.
> I'm about to send doc fix patch Ingo requested, so please leave this
> patch as is.

Ok, its just that I thought you was already asleep and was wanting to
make progress on this 8-)

Will wait for your patches and instead try to fix the annotation bug
that leads 'perf report --tui -S some_symbol_name' to exit without
printing anything, that Ingo reported, only happens in --tui, because
--stdio doesn't collects annotation info...

- Arnaldo