Re: [PATCHSET 00/10] perf tools: Support dynamic sort keys for tracepoints (v3)

[Namhyung Kim <namhyung@kernel.org>]

Hi Arnaldo,

On Mon, Jan 04, 2016 at 06:17:46PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Mon, Dec 21, 2015 at 11:26:43PM +0900, Namhyung Kim escreveu:
> > Hello,
> > 
> > This is an attempt to improve perf to deal with tracepoint events
> > better.  The perf tools can handle tracepoint events but perf report
> > on them is less useful since they're always sampled in a fixed
> > location and not provide event specific info.  We can use perf script
> > but I always wishes there's more convenient way to see the result.
> > 
> >  * changes in v3)
> >   - save trace_output for dynamic entries  (Jiri)
> >   - update field length for each entry
> 
> Thanks, applied, I made a few changes, related to libtraceevent function
> names, split a patch in two, please take a look at my perf/core branch,
> should all be ok.

Looks good to me, thanks!

> 
> Ah, please consider looking for field names directly, i.e. make this
> work for a kmem:kmalloc event:
> 
>     perf report -s comm,gfp_flags
> 
> :-)

This is supported already if the perf.data only has the kmem:kmalloc
event.

> 
> I think this can get mapped even when multiple events are available,
> i.e. use that for the events that have such a field.

Agreed.  Related events can have same field name and it'd be nice to
add all of them if possible.  I'll cook a patch for it.


> 
> It could even get more relaxed, like:
> 
>     perf report -s comm,flags
> 
> No?

Not sure.  I'd like to keep same behavior as other sort keys.  IOW it
currently finds sort keys using prefix match not substring.  For
example, 'sym' will be match to 'symbol' but 'bol' won't.  So matching
'flags' to 'gfp_flags' seems inconsistent.

In addition, it uses pevent_find_any_field() which uses strcmp()
internally.  That means the field name should be matched exactly.

However, from an usability perspective, I agree that it'd be nice to
support that kind of shortcuts.  What about adding a simple wildcard
style like:

  perf report -s comm,*_flags

or

  perf report -s comm,gfp*

?

Thanks,
Namhyung