Re: [PATCHv3 0/3] perf tool: Add new event group management

2012-07-18 Thread Jiri Olsa
On Wed, Jul 18, 2012 at 08:34:10AM -0400, Ulrich Drepper wrote:
> On Wed, Jul 18, 2012 at 6:21 AM, Jiri Olsa  wrote:
> > Well, I personally like the '{}' syntax more than '--group-events or 
> > --group-reads
> > option in front', it feels more user friendly.. anyway, we can easily have 
> > both ways.
> 
> I like the actual visual grouping better, too.
> 
> Also, it doesn't require us to define what
> 
>-e E1,E2 --group-events -e E3,E4
> 
> means.  Does --group-events also apply to the first parameter?
> 
> 
> > As for the group attributes and group leader sampling, I don't mind omitting
> > them at this point and get back to that if we find it useful in future.
> 
> Just define the first event the leader.  What reason is there which
> prevents this?

yep, no problem.. first one is the leader

The group sampling is just another feature where you enable sampling
only for the leader and the rest of the events in the group are only
being read on leader's sample - they don't sample by themselves.

But this feature needs more testing, and event modifier syntax rethinking ;)
I'll send it later..

> I can only second what Andi wrote: just get it done quickly.  This is
> functionality that is desperately needed.

I have new patchset ready with group modifiers adding modifiers
to events in the group, like:

 {cycles:u,instructions:u}:p

adds 'p' to both events in the group
(in current patchset group modifier overwrites events modifiers)

if there's strong opinion/reasons against it, I can simply remove
it, but it looks helpful

jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv3 0/3] perf tool: Add new event group management

2012-07-18 Thread Ulrich Drepper
On Wed, Jul 18, 2012 at 6:21 AM, Jiri Olsa  wrote:
> Well, I personally like the '{}' syntax more than '--group-events or 
> --group-reads
> option in front', it feels more user friendly.. anyway, we can easily have 
> both ways.

I like the actual visual grouping better, too.

Also, it doesn't require us to define what

   -e E1,E2 --group-events -e E3,E4

means.  Does --group-events also apply to the first parameter?


> As for the group attributes and group leader sampling, I don't mind omitting
> them at this point and get back to that if we find it useful in future.

Just define the first event the leader.  What reason is there which
prevents this?


I can only second what Andi wrote: just get it done quickly.  This is
functionality that is desperately needed.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv3 0/3] perf tool: Add new event group management

2012-07-18 Thread Jiri Olsa
On Tue, Jul 17, 2012 at 09:15:23AM +0200, Stephane Eranian wrote:
> On Mon, Jul 9, 2012 at 1:05 PM, Jiri Olsa  wrote:
> >
> > On Fri, Jul 06, 2012 at 03:42:54AM +0200, Stephane Eranian wrote:
> > > On Fri, Jul 6, 2012 at 3:32 AM, Ulrich Drepper  wrote:
> > > > On Thu, Jul 5, 2012 at 12:15 PM, Stephane Eranian  
> > > > wrote:
> > > >> I don't understand why you actually need the :2 suffix. There can
> > > >> only be one leader. So assume it is the first one. Users have to
> > > >> know the first one is the leader which seems like a natural thing
> > > >> to do for me. It would make you syntax less ugly than it already
> > > >> is.
> > > >
> > > > In a blue sky world I would have done this.  In fact, this is what I
> > > > tried before reading the sources to find out there is no group support
> > > > so far.  But given that multiple -e options already have a meaning I
> > > > would be hesitant to change this.
> > >
> > > That's why I said you activate grouping via -e only when you have
> > > the --group-events or --group-reads option in front. That would
> > > not change the meaning of the multiple -e when none of those
> > > group options are specified.
> >
> > I discussed this with peter..
> >
> >  the {} thing allows: 1) multiple groups in a single -e, 2) group 
> > attributes
> >
> And what's the value of 1) exactly? What's wrong with passing multiple -e ?
> The only group attribute I can think of would be :u, :k. Not so much typing.
> 
> > as for the leader sampling, we can have the first event to become the leader
> > by default (omit the leader index modifier) and enable the leader sampling 
> > by
> > another modifier:
> >
> I don't understand this sentence.
> 
> >  right, just make it a single 'l' (el not one) to indicate 'leader' 
> > sampling
> >
> To me ,this looks a bit of an over-engineered design and it is not based on
> any actual user requests. Don't get me wrong, grouping is useful and required
> but nobody has ever asked for that level of flexibility. The syntax you have
> now is already very rich for my taste.

Well, I personally like the '{}' syntax more than '--group-events or 
--group-reads
option in front', it feels more user friendly.. anyway, we can easily have both 
ways.

As for the group attributes and group leader sampling, I don't mind omitting
them at this point and get back to that if we find it useful in future.

jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv3 0/3] perf tool: Add new event group management

2012-07-18 Thread Jiri Olsa
On Tue, Jul 17, 2012 at 09:15:23AM +0200, Stephane Eranian wrote:
 On Mon, Jul 9, 2012 at 1:05 PM, Jiri Olsa jo...@redhat.com wrote:
 
  On Fri, Jul 06, 2012 at 03:42:54AM +0200, Stephane Eranian wrote:
   On Fri, Jul 6, 2012 at 3:32 AM, Ulrich Drepper drep...@gmail.com wrote:
On Thu, Jul 5, 2012 at 12:15 PM, Stephane Eranian eran...@google.com 
wrote:
I don't understand why you actually need the :2 suffix. There can
only be one leader. So assume it is the first one. Users have to
know the first one is the leader which seems like a natural thing
to do for me. It would make you syntax less ugly than it already
is.
   
In a blue sky world I would have done this.  In fact, this is what I
tried before reading the sources to find out there is no group support
so far.  But given that multiple -e options already have a meaning I
would be hesitant to change this.
  
   That's why I said you activate grouping via -e only when you have
   the --group-events or --group-reads option in front. That would
   not change the meaning of the multiple -e when none of those
   group options are specified.
 
  I discussed this with peter..
 
  peterz the {} thing allows: 1) multiple groups in a single -e, 2) group 
  attributes
 
 And what's the value of 1) exactly? What's wrong with passing multiple -e ?
 The only group attribute I can think of would be :u, :k. Not so much typing.
 
  as for the leader sampling, we can have the first event to become the leader
  by default (omit the leader index modifier) and enable the leader sampling 
  by
  another modifier:
 
 I don't understand this sentence.
 
  peterz right, just make it a single 'l' (el not one) to indicate 'leader' 
  sampling
 
 To me ,this looks a bit of an over-engineered design and it is not based on
 any actual user requests. Don't get me wrong, grouping is useful and required
 but nobody has ever asked for that level of flexibility. The syntax you have
 now is already very rich for my taste.

Well, I personally like the '{}' syntax more than '--group-events or 
--group-reads
option in front', it feels more user friendly.. anyway, we can easily have both 
ways.

As for the group attributes and group leader sampling, I don't mind omitting
them at this point and get back to that if we find it useful in future.

jirka
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv3 0/3] perf tool: Add new event group management

2012-07-18 Thread Ulrich Drepper
On Wed, Jul 18, 2012 at 6:21 AM, Jiri Olsa jo...@redhat.com wrote:
 Well, I personally like the '{}' syntax more than '--group-events or 
 --group-reads
 option in front', it feels more user friendly.. anyway, we can easily have 
 both ways.

I like the actual visual grouping better, too.

Also, it doesn't require us to define what

   -e E1,E2 --group-events -e E3,E4

means.  Does --group-events also apply to the first parameter?


 As for the group attributes and group leader sampling, I don't mind omitting
 them at this point and get back to that if we find it useful in future.

Just define the first event the leader.  What reason is there which
prevents this?


I can only second what Andi wrote: just get it done quickly.  This is
functionality that is desperately needed.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv3 0/3] perf tool: Add new event group management

2012-07-18 Thread Jiri Olsa
On Wed, Jul 18, 2012 at 08:34:10AM -0400, Ulrich Drepper wrote:
 On Wed, Jul 18, 2012 at 6:21 AM, Jiri Olsa jo...@redhat.com wrote:
  Well, I personally like the '{}' syntax more than '--group-events or 
  --group-reads
  option in front', it feels more user friendly.. anyway, we can easily have 
  both ways.
 
 I like the actual visual grouping better, too.
 
 Also, it doesn't require us to define what
 
-e E1,E2 --group-events -e E3,E4
 
 means.  Does --group-events also apply to the first parameter?
 
 
  As for the group attributes and group leader sampling, I don't mind omitting
  them at this point and get back to that if we find it useful in future.
 
 Just define the first event the leader.  What reason is there which
 prevents this?

yep, no problem.. first one is the leader

The group sampling is just another feature where you enable sampling
only for the leader and the rest of the events in the group are only
being read on leader's sample - they don't sample by themselves.

But this feature needs more testing, and event modifier syntax rethinking ;)
I'll send it later..

 I can only second what Andi wrote: just get it done quickly.  This is
 functionality that is desperately needed.

I have new patchset ready with group modifiers adding modifiers
to events in the group, like:

 {cycles:u,instructions:u}:p

adds 'p' to both events in the group
(in current patchset group modifier overwrites events modifiers)

if there's strong opinion/reasons against it, I can simply remove
it, but it looks helpful

jirka
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv3 0/3] perf tool: Add new event group management

2012-07-17 Thread Andi Kleen
> To me ,this looks a bit of an over-engineered design and it is not based on
> any actual user requests. Don't get me wrong, grouping is useful and required
> but nobody has ever asked for that level of flexibility. The syntax you have
> now is already very rich for my taste.

The user input I hear is just get the basic support working ASAP.

-Andi
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv3 0/3] perf tool: Add new event group management

2012-07-17 Thread Stephane Eranian
On Mon, Jul 9, 2012 at 1:05 PM, Jiri Olsa  wrote:
>
> On Fri, Jul 06, 2012 at 03:42:54AM +0200, Stephane Eranian wrote:
> > On Fri, Jul 6, 2012 at 3:32 AM, Ulrich Drepper  wrote:
> > > On Thu, Jul 5, 2012 at 12:15 PM, Stephane Eranian  
> > > wrote:
> > >> I don't understand why you actually need the :2 suffix. There can
> > >> only be one leader. So assume it is the first one. Users have to
> > >> know the first one is the leader which seems like a natural thing
> > >> to do for me. It would make you syntax less ugly than it already
> > >> is.
> > >
> > > In a blue sky world I would have done this.  In fact, this is what I
> > > tried before reading the sources to find out there is no group support
> > > so far.  But given that multiple -e options already have a meaning I
> > > would be hesitant to change this.
> >
> > That's why I said you activate grouping via -e only when you have
> > the --group-events or --group-reads option in front. That would
> > not change the meaning of the multiple -e when none of those
> > group options are specified.
>
> I discussed this with peter..
>
>  the {} thing allows: 1) multiple groups in a single -e, 2) group 
> attributes
>
And what's the value of 1) exactly? What's wrong with passing multiple -e ?
The only group attribute I can think of would be :u, :k. Not so much typing.

> as for the leader sampling, we can have the first event to become the leader
> by default (omit the leader index modifier) and enable the leader sampling by
> another modifier:
>
I don't understand this sentence.

>  right, just make it a single 'l' (el not one) to indicate 'leader' 
> sampling
>
To me ,this looks a bit of an over-engineered design and it is not based on
any actual user requests. Don't get me wrong, grouping is useful and required
but nobody has ever asked for that level of flexibility. The syntax you have
now is already very rich for my taste.




>
> jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv3 0/3] perf tool: Add new event group management

2012-07-17 Thread Stephane Eranian
On Mon, Jul 9, 2012 at 1:05 PM, Jiri Olsa jo...@redhat.com wrote:

 On Fri, Jul 06, 2012 at 03:42:54AM +0200, Stephane Eranian wrote:
  On Fri, Jul 6, 2012 at 3:32 AM, Ulrich Drepper drep...@gmail.com wrote:
   On Thu, Jul 5, 2012 at 12:15 PM, Stephane Eranian eran...@google.com 
   wrote:
   I don't understand why you actually need the :2 suffix. There can
   only be one leader. So assume it is the first one. Users have to
   know the first one is the leader which seems like a natural thing
   to do for me. It would make you syntax less ugly than it already
   is.
  
   In a blue sky world I would have done this.  In fact, this is what I
   tried before reading the sources to find out there is no group support
   so far.  But given that multiple -e options already have a meaning I
   would be hesitant to change this.
 
  That's why I said you activate grouping via -e only when you have
  the --group-events or --group-reads option in front. That would
  not change the meaning of the multiple -e when none of those
  group options are specified.

 I discussed this with peter..

 peterz the {} thing allows: 1) multiple groups in a single -e, 2) group 
 attributes

And what's the value of 1) exactly? What's wrong with passing multiple -e ?
The only group attribute I can think of would be :u, :k. Not so much typing.

 as for the leader sampling, we can have the first event to become the leader
 by default (omit the leader index modifier) and enable the leader sampling by
 another modifier:

I don't understand this sentence.

 peterz right, just make it a single 'l' (el not one) to indicate 'leader' 
 sampling

To me ,this looks a bit of an over-engineered design and it is not based on
any actual user requests. Don't get me wrong, grouping is useful and required
but nobody has ever asked for that level of flexibility. The syntax you have
now is already very rich for my taste.





 jirka
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv3 0/3] perf tool: Add new event group management

2012-07-17 Thread Andi Kleen
 To me ,this looks a bit of an over-engineered design and it is not based on
 any actual user requests. Don't get me wrong, grouping is useful and required
 but nobody has ever asked for that level of flexibility. The syntax you have
 now is already very rich for my taste.

The user input I hear is just get the basic support working ASAP.

-Andi
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv3 0/3] perf tool: Add new event group management

2012-07-09 Thread Peter Zijlstra
On Mon, 2012-07-09 at 13:05 +0200, Jiri Olsa wrote:
>  the {} thing allows: 1) multiple groups in a single -e

The advantage would be when you're passing such things on via
environment variables or somesuch.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv3 0/3] perf tool: Add new event group management

2012-07-09 Thread Jiri Olsa
On Fri, Jul 06, 2012 at 03:42:54AM +0200, Stephane Eranian wrote:
> On Fri, Jul 6, 2012 at 3:32 AM, Ulrich Drepper  wrote:
> > On Thu, Jul 5, 2012 at 12:15 PM, Stephane Eranian  
> > wrote:
> >> I don't understand why you actually need the :2 suffix. There can
> >> only be one leader. So assume it is the first one. Users have to
> >> know the first one is the leader which seems like a natural thing
> >> to do for me. It would make you syntax less ugly than it already
> >> is.
> >
> > In a blue sky world I would have done this.  In fact, this is what I
> > tried before reading the sources to find out there is no group support
> > so far.  But given that multiple -e options already have a meaning I
> > would be hesitant to change this.
> 
> That's why I said you activate grouping via -e only when you have
> the --group-events or --group-reads option in front. That would
> not change the meaning of the multiple -e when none of those
> group options are specified.

I discussed this with peter..

 the {} thing allows: 1) multiple groups in a single -e, 2) group 
attributes 

as for the leader sampling, we can have the first event to become the leader
by default (omit the leader index modifier) and enable the leader sampling by
another modifier:

 right, just make it a single 'l' (el not one) to indicate 'leader' 
sampling


jirka
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv3 0/3] perf tool: Add new event group management

2012-07-09 Thread Jiri Olsa
On Fri, Jul 06, 2012 at 03:42:54AM +0200, Stephane Eranian wrote:
 On Fri, Jul 6, 2012 at 3:32 AM, Ulrich Drepper drep...@gmail.com wrote:
  On Thu, Jul 5, 2012 at 12:15 PM, Stephane Eranian eran...@google.com 
  wrote:
  I don't understand why you actually need the :2 suffix. There can
  only be one leader. So assume it is the first one. Users have to
  know the first one is the leader which seems like a natural thing
  to do for me. It would make you syntax less ugly than it already
  is.
 
  In a blue sky world I would have done this.  In fact, this is what I
  tried before reading the sources to find out there is no group support
  so far.  But given that multiple -e options already have a meaning I
  would be hesitant to change this.
 
 That's why I said you activate grouping via -e only when you have
 the --group-events or --group-reads option in front. That would
 not change the meaning of the multiple -e when none of those
 group options are specified.

I discussed this with peter..

peterz the {} thing allows: 1) multiple groups in a single -e, 2) group 
attributes 

as for the leader sampling, we can have the first event to become the leader
by default (omit the leader index modifier) and enable the leader sampling by
another modifier:

peterz right, just make it a single 'l' (el not one) to indicate 'leader' 
sampling


jirka
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [PATCHv3 0/3] perf tool: Add new event group management

2012-07-09 Thread Peter Zijlstra
On Mon, 2012-07-09 at 13:05 +0200, Jiri Olsa wrote:
 peterz the {} thing allows: 1) multiple groups in a single -e

The advantage would be when you're passing such things on via
environment variables or somesuch.
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/