Re: [PATCH 01/17] perf hists: Basic support of hierarchical report view

[Namhyung Kim <namhyung@kernel.org>]

Hi Jiri,

On Thu, Jan 21, 2016 at 02:35:58PM +0100, Jiri Olsa wrote:
> On Thu, Jan 21, 2016 at 09:55:52PM +0900, Namhyung Kim wrote:
> > > would it make more sense to do this in 'in-tree addition' path?
> > > and keep the resort functions to do only resort stuff
> > 
> > I don't follow.  There're 3 path to handle hist entries - let's say
> > them as 'addition', 'collapsing', and 'resort'.  This function does
> > the 'collapsing' part - it was originally intended to merge sharable
> > entries (namely for same 'comm' among different threads).  But I used
> > it to build a hierarchy since I found it useful as follows:
> > 
> >   1. it requires smaller change than doing it in the 'addition' path
> >   2. it can reuse current callback-based 'addition' paths so mem- and
> >      branch-mode can be supported easily (but it needs test..).
> >   3. the 'addition' path can be parallelized so it'll increase memory
> >      footprint if it build temporary local hierarchies during the path.
> > 
> > The 'resort' path always do sorting only..
> 
> well, you are adding/duplicating entries now in resort path
> and that is not just 'sorting only'

As I said, this is not the 'resort' path, the 'resort' path is the
subject of patch 02/17.


> 
> you allow only sort and tracepoint entries to be added in
> hierrarych view, so there's no resort needed, but still it
> could be added in future? not sure

Resorting is still needed since it should sort entries by overhead
(period).  Anyway, in this 'collapsing' path, we need to build a
hierarchy to be sorted at the resort path.


> 
> it still makes more sense to me to do this in 'addition' path,
> because you basically add new entries
> 
> but have no other grounds for this also I might be missing something ;-)

What about thinking like this?

1. addition   - add samples into hist entries
2. collapsing - build complete hists (hierarchy) to be sorted
3. resort     - sort final entries based on the sort keys

Thanks,
Namhyung