Re: [PATCH 08/10] perf tools: Add --raw-trace option

[Namhyung Kim <namhyung@kernel.org>]

On Sun, Dec 20, 2015 at 03:58:42PM +0100, Jiri Olsa wrote:
> On Wed, Dec 16, 2015 at 12:35:41AM +0900, Namhyung Kim wrote:
> > The --raw-trace option is to prevent pretty printing by event's
> > print_fmt or plugin.  Besides that, each dynamic sort key now receives
> > 'raw' suffix separated by '/' to apply the raw trace to a specific
> > field.
> > 
> >   $ perf report -s comm,kmem:kmalloc.gfp_flags
> >   ...
> >   # Overhead  Command            gfp_flags
> >   # ........  .......  ...................
> >   #
> >       99.89%  perf       GFP_NOFS|GFP_ZERO
> >        0.06%  sleep             GFP_KERNEL
> >        0.03%  perf     GFP_KERNEL|GFP_ZERO
> >        0.01%  perf              GFP_KERNEL
> > 
> > Now
> > 
> >   $ perf report -s comm,kmem:kmalloc.gfp_flags --raw-trace
> > or
> >   $ perf report -s comm,kmem:kmalloc.gfp_flags/raw
> >   ...
> >   # Overhead  Command   gfp_flags
> >   # ........  .......  ..........
> >   #
> >       99.89%  perf          32848
> >        0.06%  sleep           208
> >        0.03%  perf          32976
> >        0.01%  perf            208
> > 
> > Suggested-by: Jiri Olsa <jolsa@redhat.com>
> > Cc: Steven Rostedt <rostedt@goodmis.org>
> > Signed-off-by: Namhyung Kim <namhyung@kernel.org>
> 
> I think the default is good with the 'trace' sort key as it is now:
> 
> 	$ ./perf report -s trace 
> 	  27.01%  offlineimap:17701 [120] S ==> swapper/1:0 [120]
> 	  27.00%  swapper/1:0 [120] R ==> offlineimap:17701 [120]
> 
> 
> But for '--raw-trace' option rather than displaying fields in the '=' notation:
> 
> 	$ ./perf report -s trace --raw-trace
> 	  27.01%   prev_comm=offlineimap prev_pid=17701 prev_prio=120 prev_state=1 next_comm=swapper/1 next_pid=0 next_prio=120
> 	  27.00%   prev_comm=swapper/1 prev_pid=0 prev_prio=120 prev_state=0 next_comm=offlineimap next_pid=17701 next_prio=120
> 
> 
> it would be more readable to register all available columns as is possible to do now with:
> 
> 	$ ./perf report -s prev_comm,prev_pid,prev_prio,prev_state,next_comm,next_pid,next_prio
> 	Overhead         prev_comm    prev_pid   prev_prio          prev_state         next_comm    next_pid   next_prio
> 	  27.01%       offlineimap       17701         120                   1         swapper/1           0         120
> 	  27.00%         swapper/1           0         120                   0       offlineimap       17701         120
> 
> 
> it would be default output for 'perf report -s trace --raw-trace' command
> 
> how about that?


I agree with the latter is much more readable.  But the dynamic sort
keys are event specific while 'trace' is generic.  So if there are
more than one events, dynamic sort keys should be generated for each
event and they'll show "N/A" for other events.

Also I don't like the --raw-trace option affects number of sort keys.
Maybe it'd be better adding 'EVENT.*' dynamic sort key for your case?

Thanks,
Namhyung