Re: [PATCH 09/10] perf, tools: Support DividedBy header in JSON event list

2016-10-17 Thread Andi Kleen
> 
> so..
> 
> - you put 'DividedBy' into JSON event's defition any further
>   explanation how or why the format we use for event defs will
>   be used now used to describe ratios
> 
> - then you force perf stat to merge together all 'same' uncore events
>   to get just one number..

The ratios don't need that. They work fine without merging.
It's an independent feature.

The merging is just a convenience feature for some of the uncore pmus to make
the output much more readable. For example the cbox pmu is duplicated
for each core, and we have systems with 21 cores per socket now.

So without merging you end up with something like the output below.

The second variant is much more readable.

> 
> - then you display that ratio (just the number) in perf stat metrics output
>   without any explanation or description

The event name is already expressive enough. I find it fairly
straight forward that there is a ratio attached with a count.

> 
> I dont see that as a nicely fit, more like hack

I would call it a simple solution that works well.

I don't see any easy path for full scripting, and also I think
it would be vastly overengineered here needing a lot of 
infastructure that isn't really needed.


-Andi

% perf stat --no-merge -a  -e unc_c_llc_lookup.any sleep 1

 Performance counter stats for 'system wide':

   694,976 Bytes unc_c_llc_lookup.any   
 
   706,304 Bytes unc_c_llc_lookup.any   
 
   956,608 Bytes unc_c_llc_lookup.any   
 
   782,720 Bytes unc_c_llc_lookup.any   
 
   605,696 Bytes unc_c_llc_lookup.any   
 
   442,816 Bytes unc_c_llc_lookup.any   
 
   659,328 Bytes unc_c_llc_lookup.any   
 
   509,312 Bytes unc_c_llc_lookup.any   
 
   263,936 Bytes unc_c_llc_lookup.any   
 
   592,448 Bytes unc_c_llc_lookup.any   
 
   672,448 Bytes unc_c_llc_lookup.any   
 
   608,640 Bytes unc_c_llc_lookup.any   
 
   641,024 Bytes unc_c_llc_lookup.any   
 
   856,896 Bytes unc_c_llc_lookup.any   
 
   808,832 Bytes unc_c_llc_lookup.any   
 
   684,864 Bytes unc_c_llc_lookup.any   
 
   710,464 Bytes unc_c_llc_lookup.any   
 
   538,304 Bytes unc_c_llc_lookup.any   
 

   1.002577660 seconds time elapsed
 


% perf stat  -a  -e unc_c_llc_lookup.any sleep 1

 Performance counter stats for 'system wide':

 2,685,120 Bytes unc_c_llc_lookup.any   
 

   1.002648032 seconds time elapsed



Re: [PATCH 09/10] perf, tools: Support DividedBy header in JSON event list

2016-10-17 Thread Jiri Olsa
On Mon, Oct 17, 2016 at 07:43:12PM +0200, Jiri Olsa wrote:
> On Mon, Oct 17, 2016 at 09:27:54AM -0700, Andi Kleen wrote:
> > On Mon, Oct 17, 2016 at 01:44:43PM +0200, Jiri Olsa wrote:
> > > On Thu, Oct 13, 2016 at 02:15:31PM -0700, Andi Kleen wrote:
> > > > From: Andi Kleen 
> > > > 
> > > > Add support for parsing the DividedBy header in the JSON event lists and
> > > > storing them in the alias structure.
> > > 
> > > I wish you'd add JSON tags always one by one as you did in here ;-)
> > > 
> > > however Ithink we'll need more info here:
> > >   - what's the value?
> > >   - what's it going to be used for?
> > 
> > That's all described in the next patch. But I can copy the description.
> > 
> > >   - looks like formula stuff, why post processing via python/perl can't 
> > > be used in this case?
> > 
> > It would be fairly complicated to interface that with event lists, and
> > also still wouldn't work with standard perf stat. 
> > 
> > DividedBy already covers the majority of interesting cases and fits
> > nicely with the existing frame work. If we wanted more complex
> > formulas something with python would be probably needed, but I don't see
> > the need yet.
> 
> so..
> 
> - you put 'DividedBy' into JSON event's defition any further
  ^ without ;-)

>   explanation how or why the format we use for event defs will
>   be used now used to describe ratios
> 
> - then you force perf stat to merge together all 'same' uncore events
>   to get just one number..
> 
> - then you display that ratio (just the number) in perf stat metrics output
>   without any explanation or description
> 
> I dont see that as a nicely fit, more like hack
> 
> please let's go first to discuss the DividedBy being included
> in JSON defs, which is fragile topic to begin with
> 
> thanks,
> jirka


Re: [PATCH 09/10] perf, tools: Support DividedBy header in JSON event list

2016-10-17 Thread Jiri Olsa
On Mon, Oct 17, 2016 at 09:27:54AM -0700, Andi Kleen wrote:
> On Mon, Oct 17, 2016 at 01:44:43PM +0200, Jiri Olsa wrote:
> > On Thu, Oct 13, 2016 at 02:15:31PM -0700, Andi Kleen wrote:
> > > From: Andi Kleen 
> > > 
> > > Add support for parsing the DividedBy header in the JSON event lists and
> > > storing them in the alias structure.
> > 
> > I wish you'd add JSON tags always one by one as you did in here ;-)
> > 
> > however Ithink we'll need more info here:
> >   - what's the value?
> >   - what's it going to be used for?
> 
> That's all described in the next patch. But I can copy the description.
> 
> >   - looks like formula stuff, why post processing via python/perl can't be 
> > used in this case?
> 
> It would be fairly complicated to interface that with event lists, and
> also still wouldn't work with standard perf stat. 
> 
> DividedBy already covers the majority of interesting cases and fits
> nicely with the existing frame work. If we wanted more complex
> formulas something with python would be probably needed, but I don't see
> the need yet.

so..

- you put 'DividedBy' into JSON event's defition any further
  explanation how or why the format we use for event defs will
  be used now used to describe ratios

- then you force perf stat to merge together all 'same' uncore events
  to get just one number..

- then you display that ratio (just the number) in perf stat metrics output
  without any explanation or description

I dont see that as a nicely fit, more like hack

please let's go first to discuss the DividedBy being included
in JSON defs, which is fragile topic to begin with

thanks,
jirka


Re: [PATCH 09/10] perf, tools: Support DividedBy header in JSON event list

2016-10-17 Thread Andi Kleen
On Mon, Oct 17, 2016 at 01:44:43PM +0200, Jiri Olsa wrote:
> On Thu, Oct 13, 2016 at 02:15:31PM -0700, Andi Kleen wrote:
> > From: Andi Kleen 
> > 
> > Add support for parsing the DividedBy header in the JSON event lists and
> > storing them in the alias structure.
> 
> I wish you'd add JSON tags always one by one as you did in here ;-)
> 
> however Ithink we'll need more info here:
>   - what's the value?
>   - what's it going to be used for?

That's all described in the next patch. But I can copy the description.

>   - looks like formula stuff, why post processing via python/perl can't be 
> used in this case?

It would be fairly complicated to interface that with event lists, and
also still wouldn't work with standard perf stat. 

DividedBy already covers the majority of interesting cases and fits
nicely with the existing frame work. If we wanted more complex
formulas something with python would be probably needed, but I don't see
the need yet.

-Andi


Re: [PATCH 09/10] perf, tools: Support DividedBy header in JSON event list

2016-10-17 Thread Jiri Olsa
On Thu, Oct 13, 2016 at 02:15:31PM -0700, Andi Kleen wrote:
> From: Andi Kleen 
> 
> Add support for parsing the DividedBy header in the JSON event lists and
> storing them in the alias structure.

I wish you'd add JSON tags always one by one as you did in here ;-)

however Ithink we'll need more info here:
  - what's the value?
  - what's it going to be used for?
  - looks like formula stuff, why post processing via python/perl can't be used 
in this case?

jirka