Re: [PATCH] perf: Add a new sort order: SORT_INCLUSIVE (v4)

[Frederic Weisbecker <fweisbec@gmail.com>]

On Thu, Mar 15, 2012 at 10:58:38AM -0700, Arun Sharma wrote:
> On 3/15/12 7:14 AM, Frederic Weisbecker wrote:
> 
> >I still feel concerned about this.
> >
> >If I have only one event with a period of 1 and with that callchain:
> >
> >	a ->  b ->  c
> >
> >Then I produce three hists
> >
> >	1) a ->  b ->  c
> >	2) a ->  b
> >	3) a
> >
> >Each hist have a period of 1, but the total period is 1.
> >So the end result should be (IIUC):
> >
> >100%    foo     a
> >100%    foo     b
> >                 |
> >                 --- a
> >100%    foo     c
> >                 |
> >                 --- b
> >                     |
> >                     --- c
> >
> 
> That is correct. The first column no longer adds up to 100%.

So do we really want this?

>  		
> >And the percentages on callchain branches will have the same kind
> >of weird things.
> 
> I expect --sort inclusive to be used with -g graph,0.5,caller. I can
> polish this in the next rev where a single top level flag will set this up.
>
> The percentages on the branches should still be accurate (as a
> percentage of total_period). Please let me know if this is not the
> case.
> >
> >So I'm not sure this is a good direction. I'd rather advocate to create
> >true hists for each callers, all having the same real period as the leaf.
> >
> 
> Please see the v5 I just posted. The callers have a true histogram
> entry in every sense, except that period_self == 0.
> 
> If we don't do this, total_period will be inflated.

Yeah right I've just tried and callchains look right. I'm just puzzled
by the percentages:

+  98,99%  [k] execve
+  98,99%  [k] stub_execve
+  98,99%  [k] do_execve
+  98,99%  [k] do_execve_common
+  98,99%  [k] sys_execve
+  53,12%  [k] __libc_start_main
+  53,12%  [k] cmd_record
+  53,12%  [k] T.101
+  53,12%  [k] main
+  53,12%  [k] run_builtin
+  52,11%  [k] perf_evlist__prepare_workload
+  52,09%  [k] T.1163

> 
> >Also this feature reminds me a lot the -b option in perf report.
> >Branch sorting and callchain inclusive sorting are a bit different in
> >the way they handle the things but the core idea is the same. Callchains
> >are branches as well.
> >
> 
> Yes - I kept asking why the branch stack stuff doesn't use the
> existing callchain logic.

Because I fear that loops branches could make the tree representation useless.

> 
> >Branch sorting (-b) adds a hist for every branch taken, and the period
> >is always 1. I wonder if this makes more sense than using the original
> >period of the event for all branches of the event. Not sure.
> >
> >Anyway I wonder if both features can be better integrated. After all
> >they are about the same thing. The difference is that the source of
> >the branches is not the same and that callchains can be depicted into
> >trees.
> >
> >So perhaps we can have -b specifying the desired source, in case both
> >are present: -b callchain and -b branch. Both at the same time wouldn't
> >make much sense I think.
> >
> >And the source could default to either if we don't have callchain and
> >branch at the same time in the events.
> >
> >Just an idea...
> 
> I haven't played much with the branch stack logic. Will do so and get back.
> 
> In the meanwhile, my impression is that there are two high level use cases:
> 
> * Compiler optimizers, tracing JITs etc
> 
> Which try to focus on a single branch and try to understand what
> happened with that branch
> 
> * Programmers who're trying to understand the behavior of the code
> they wrote in production
> 
> I think the branch-stack stuff primarily caters to the former and
> inclusive callchain stuff to the latter. I was thinking that getting
> the branch-stack data into callchains will make the data more useful
> to more people.

I don't know. "if/else" generated branch could be relevant when represented
in a tree like we do for callchains. But I fear this doesn't work anymore
once we deal with loops.

> 
>  -Arun