Re: [RFC/PATCHSET 00/17] perf tools: Add support for hierachy view (v2)

[Taeung Song]

Hi, Namhyung

On 01/21/2016 12:08 AM, Namhyung Kim wrote:
> Hi Taeung,
>
> On Wed, Jan 20, 2016 at 04:49:29PM +0900, Taeung Song wrote:
> > On 01/20/2016 09:34 AM, Namhyung Kim wrote:
> > > On Tue, Jan 19, 2016 at 05:59:41PM -0300, Arnaldo Carvalho de Melo wrote:
> > > > Em Sun, Jan 17, 2016 at 01:03:00AM +0900, Namhyung Kim escreveu:
> > > > > Hello,
> > > > >
> > > > > This is v2 attempt of my earlier patchset [1].  This patchset
> > > > > implements a new feature that collects hist entries in a hierachical
> > > > > manner.  That means lower-level entries belong to an upper-level
> > > > > entry.  The entry hierachy is built on the sort keys given, so users
> > > > > can set it whatever they want.  It only shows top-level entries first,
> > > > > and user can expand/collapse it dynamically.
> > > > >
> > > > > This time I implemented it for every output browser including TUI.
> > > > > A screenshot on TUI looks like below:
> > > > >
> > > > > For normal output:
> > > > >
> > > > >    $ perf report --tui
> > > > >    Samples: 3K of event 'cycles:pp', Event count (approx.): 1695979674
> > > > >      Overhead  Command        Shared Object         Symbol
> > > > >    ------------------------------------------------------------------------
> > > > >    -    7.57%  swapper        [kernel.vmlinux]      [k] intel_idle
> > > > >         intel_idle
> > > > >         cpuidle_enter_state
> > > > >         cpuidle_enter
> > > > >         call_cpuidle
> > > > >       + cpu_startup_entry
> > > > >    +    1.16   firefox        firefox               [.] 0x00000000000019433
> > > > >    +    0.97%  firefox        libpthread-2.22.so    [.] pthread_mutex_lock
> > > > >    ...
> > > > >
> > > > >
> > > > > With hierarchy view,
> > > >
> > > > Ok, tested, this is really nice, I think it should be the default, from
> > > > where to drill down, we could have a '--no-hierarchy', Ingo?
> > >
> > > Yeah, we already have --no-hierarchy (as a side effect of having
> > > --hierarchy) but I don't want to change the default now since existing
> > > users will complain.  Now we have 'tips' in the perf report browser,
> > > maybe it's enough to add a line to suggest to use it (and it's already
> > > done by this patchset).  I remember the time we changed default for
> > > '--children' and many people complained about it.
> > >
> > > We maybe change the default later but I think it's better to have some
> > > time to people can play with it and find it useful. :)  And, as always,
> > > we can have a config option to control the default.
> >
> > If adding this config option,
> > can this be included in 'hist' section ?
> > If it isn't, 'report' and 'top' section ?
> > i.e.
> >
> > [report]
> >      hierarchy = true
> > [top]
> >      hierarchy = false
>
> Either is fine.  But as we already have report.children and
> top.children, I'd follow the convention.  Also I think we should set
> priority of the two configs - children and hierarchy.  IMHO hierarchy
> should be considered first.
>
> Or maybe we could have 'report.output-default' being one of
> 'hierarchy', 'children', or 'normal'.  This way we can set the default
> behavior easily including possible future changes.

Oh, IMHO I think the latter is better than the former.
If using 'report.output-default' instead of 'report.children'
and 'report.hierarchy' etc integrating the configs,
it seems to be tidy.
Whatever this config variables will be set as,
after this patchset are merged I'll ask about this configs, again.

Thanks,
Taeung