Re: [RFC 5/5] perf: Enable multiple hist_entry_group output

[Don Zickus <dzickus@redhat.com>]

On Mon, Apr 14, 2014 at 11:19:32AM +0200, Jiri Olsa wrote:
> On Fri, Apr 11, 2014 at 02:34:52PM -0400, Don Zickus wrote:
> > On Fri, Apr 11, 2014 at 07:30:00PM +0200, Jiri Olsa wrote:
> 
> SNIP
> 
> > > and with your changes we could do:
> > > 
> > >    Overhead   CPU  symbol
> > >    ........   ...  ......
> > >    90%        0
> > >         50%        krava1
> > >         20%        krava2
> > >         30%        krava3
> > > 
> > >    10%        1
> > >         50%        krava4
> > >         50%        krava5
> > >   
> > > 
> > > I wonder we could go more generic and allow more nested groups,
> > > like eg allow group sort on cpu and pid (or more):
> > > 
> > >    Overhead   CPU  pid  symbol
> > >    ........   ...  ...  ......
> > >    90%        0
> > >       50%          100
> > >         50%             krava1
> > >         20%             krava2
> > >         30%             krava3
> > >       50%          110
> > >         50%             krava1
> > >         20%             krava2
> > >         30%             krava3
> > > 
> > >    10%        1
> > >       100%         200
> > >         50%             krava4
> > >         50%             krava5
> > > 
> > > 
> > > I glanced over the changes and I wonder we could do it
> > > by chaining hists structs via 'struct hist_entry'
> > > 
> > > like adding 'struct hists' into 'struct hists_entry'
> > > and making the sort_order local for each 'struct hists'
> > 
> > Unless you meant:
> > 
> > hists
> >  \- hist_entry
> >       \- hists
> >            \- hist_entry -> hist_entry -> hist_entry -> hist_entry
> >                               \- hists
> >                                    \- hist_entry -> hist_entry -> hist_entry
> > 
> > where each 'hists' represents a new group and a hist_entry->hists != NULL
> > is a group otherwise just a node?
> 
> right, hist_entry is either node or leaf, I see it like this:
> 
> hists (A)
>   \-hist_entry(A)->(hists B1)
>                      \-hist_entry(B1)
>                      \-hist_entry(B1)
>                      \-hist_entry(B1)
> 
>   \-hist_entry(A)->(hists B2)
>                      \-hist_entry(B2)
> 
>   \-hist_entry(A)->(hists B3)
>                      \-hist_entry(B3)
>                      \-hist_entry(B3)
> 
> 
> hists (A)  - getting data groupped by CPU - hist_entry (A)
> hists (BX) - getting data belonging to CPU group and groupped by symbol (hist_entry BX)
> 
> so hists(A) provides data for 1st column in Overhead,
> and hists(BX) provides data for the 2nd one:
> 
>   Overhead   CPU  symbol
>   ........   ...  ......
>   90%        0
>        50%        krava1
>        20%        krava2
>        30%        krava3
> 
>    5%        1
>       100%        krava4
> 
>    5%        2
>        50%        krava4
>        50%        krava5

Ok.  That at least tries to re-use the structs hist and hist_entry which
is what I struggled with when creating hist_entry_group.  Walking the
entries list will be an interesting challenge and sorting it.  I'll poke
it at it some more and see how things fall out.

Are you ok with the macros from patch 2?  At least the idea, we can change
the names.  I think I would like to continue with the macros to make it
easier to handle these changes, otherwise the code blows up and becomes
difficult to read.

Though I still am not sure how to handle 'pairs' correctly.

Cheers,
Don