Re: [RFC] Re: [PATCHSET 0/8] perf tools: Support multiple keys in a single hierarchy level (v1)

[Namhyung Kim]

Hi Arnaldo,

On Thu, Mar 03, 2016 at 10:08:15AM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Mar 03, 2016 at 08:16:36AM +0900, Namhyung Kim escreveu:
> > On Wed, Mar 02, 2016 at 01:25:07PM -0300, Arnaldo Carvalho de Melo wrote:
> > > Em Thu, Mar 03, 2016 at 01:12:00AM +0900, Namhyung Kim escreveu:
> > > > This implements what Arnaldo suggested in previous discussion of
> > > > hierarchy patchset [1].  Originally each level in a hierarchy can have
> > > > a single sort key in it, but with patches it's possible to have more
> > > > than one sort keys.  I added the struct perf_hpp_list_node and carry
> > > > it to group output formats (hpp_fmt) in a single level.
> 
> > > > I used ':' character instead of '+' as suggested since the '+' was
> > > > also used to extend existing sort keys (like -s '+cpu').
> 
> > > Could we use ';' instead? That is associated with lists as well, i.e.:
> 
> > >   perf report --hierarchy -s prev_pid;prev_comm,next_pid;next_comm
> 
> > > What do you think? Others?
>  
> > The ';' is interpreted by shell first, so it needs to be quoted.
>  
> >   $ perf report -s comm,dso;sym
> >   bash: sym: command not found
> 
> Right, but this is not some really common operation, so asking for it to
> be quoted is not such an unreasonable demand, and I continue thinking
> that ':' should be left for other purposes, ',' and ';' are better
> suited for expressing lists.
> 
> We will/have stumble/d in this more times, like when we added { for groups:
> 
>   # perf record -e cycles,instructions usleep 1
>   [ perf record: Woken up 1 times to write data ]
>   [ perf record: Captured and wrote 0.017 MB perf.data (4 samples) ]
>   # perf report --header-only | grep group
>   # 
>   # perf record -e {cycles,instructions} usleep 1
>   Workload failed: No such file or directory
>   # perf record -e '{cycles,instructions}' usleep 1
>   [ perf record: Woken up 1 times to write data ]
>   [ perf record: Captured and wrote 0.017 MB perf.data (4 samples) ]
>   # perf report --header-only | grep group
>   # group: {cycles,instructions}
>   # 
> 
> We should try to use tokens that already have some strong meaning
> associated to them instead of using those that are more easily
> available, otherwise our mumbo jumbo will grow even more unwieldly 8-/
> 
> In fact I think that in this case we could even make it look more like
> natural languages and use:
> 
>   perf report --hierarchy -s 'prev_pid,prev_comm;next_pid,next_comm'
> 
> To ask for:
>  
>       prev_pid  prev_comm
>             next_pid  next_comm          
>       - 1234      bash
>               5678     firefox
>       + 8912      hexchat
> 
> I.e. if ';' is present, it is the separator for each hierarchy level,
> with ',' being used for stating the per-hierarchy level fields.

If it's not a common operation and we have used '{ }' for the event
groups, why not using it here too?

  $ perf report --hierarchy -s '{prev_pid,prev_comm},{next_pid,next_comm}'

It's more verbose but more intuitive IMHO.


Thanks,
Namhyung