[RFC PATCHv3] perf tools: add event grouping capability to "perf stat"

[RFC PATCHv3] perf tools: add event grouping capability to "perf stat"

[Corey Ashford]

Add the ability to create multiple event groups, each with their own leader
using the existing "-e <event>[,<event> ...] [-e <event>[,<event>]]"
syntax.  Each additional -e switch creates a new group, and each event
listed within a -e switch is within that group.

Changes since v1:
- Because of a flub, v2 did not contain the changes I had intended to make,
and instead, v2 had the same patch contents as v1.
- When perf stat is not supplied any events on the command line, put
each default event in its own group.

Signed-off-by: Corey Ashford <cjashfor@linux.vnet.ibm.com>
---
 tools/perf/Documentation/perf-stat.txt |   12 +++++++++---
 tools/perf/builtin-stat.c              |   16 ++++++++++++++--
 tools/perf/util/parse-events.c         |    4 ++++
 tools/perf/util/parse-events.h         |    1 +
 4 files changed, 28 insertions(+), 5 deletions(-)

diff --git a/tools/perf/Documentation/perf-stat.txt b/tools/perf/Documentation/perf-stat.txt
index 4b3a2d4..d522ebd 100644
--- a/tools/perf/Documentation/perf-stat.txt
+++ b/tools/perf/Documentation/perf-stat.txt
@@ -8,8 +8,8 @@ perf-stat - Run a command and gather performance counter statistics
 SYNOPSIS
 --------
 [verse]
-'perf stat' [-e <EVENT> | --event=EVENT] [-S] [-a] <command>
-'perf stat' [-e <EVENT> | --event=EVENT] [-S] [-a] -- <command> [<options>]
+'perf stat' [-e <EVENT>[,<EVENT>...] | --event=<EVENT>[,<EVENT>...]] [-S] [-a] <command>
+'perf stat' [-e <EVENT>[,<EVENT>...] | --event=<EVENT>[,<EVENT>...]] [-S] [-a] -- <command> [<options>]
 
 DESCRIPTION
 -----------
@@ -28,7 +28,13 @@ OPTIONS
 	Select the PMU event. Selection can be a symbolic event name
 	(use 'perf list' to list all events) or a raw PMU
 	event (eventsel+umask) in the form of rNNN where NNN is a
-	 hexadecimal event descriptor.
+	hexadecimal event descriptor.  As shown, multiple events can be
+	separated by commas in each -e/--event switch.  Each additional
+	-e/--event switch creates a new event group.  Grouped events are
+	scheduled onto the PMU hardware at the same time, which is
+	important to know when the PMU is overscheduled.  A good example of
+	this is measuring CPI where both instructions and cycles events
+	need to be scheduled simultaneously to get an accurate estimate.
 
 -i::
 --no-inherit::
diff --git a/tools/perf/builtin-stat.c b/tools/perf/builtin-stat.c
index 8b9afa6..8a17e9b 100644
--- a/tools/perf/builtin-stat.c
+++ b/tools/perf/builtin-stat.c
@@ -162,8 +162,12 @@ static int create_perf_stat_counter(int counter, bool *perm_err)
 		int cpu;
 
 		for (cpu = 0; cpu < nr_cpus; cpu++) {
+			if (counter == grp_leaders[counter])
+				/* first counter in the group is the leader */
+				fd[cpu][grp_leaders[counter]][0] = -1;
 			fd[cpu][counter][0] = sys_perf_event_open(attr,
-					-1, cpumap[cpu], -1, 0);
+					-1, cpumap[cpu],
+					fd[cpu][grp_leaders[counter]][0], 0);
 			if (fd[cpu][counter][0] < 0) {
 				if (errno == EPERM || errno == EACCES)
 					*perm_err = true;
@@ -180,8 +184,12 @@ static int create_perf_stat_counter(int counter, bool *perm_err)
 			attr->enable_on_exec = 1;
 		}
 		for (thread = 0; thread < thread_num; thread++) {
+			if (counter == grp_leaders[counter])
+				/* first counter in the group is the leader */
+				fd[0][grp_leaders[counter]][thread] = -1;
 			fd[0][counter][thread] = sys_perf_event_open(attr,
-				all_tids[thread], -1, -1, 0);
+				all_tids[thread], -1,
+				fd[0][grp_leaders[counter]][thread], 0);
 			if (fd[0][counter][thread] < 0) {
 				if (errno == EPERM || errno == EACCES)
 					*perm_err = true;
@@ -576,6 +584,10 @@ int cmd_stat(int argc, const char **argv, const char *prefix __used)
 	if (!null_run && !nr_counters) {
 		memcpy(attrs, default_attrs, sizeof(default_attrs));
 		nr_counters = ARRAY_SIZE(default_attrs);
+		for (i = 0; i < nr_counters; i++) {
+			/* each default event should be in its own group */
+			grp_leaders[i] = i;
+		}
 	}
 
 	if (system_wide)
diff --git a/tools/perf/util/parse-events.c b/tools/perf/util/parse-events.c
index 4af5bd5..eeecb2a 100644
--- a/tools/perf/util/parse-events.c
+++ b/tools/perf/util/parse-events.c
@@ -12,6 +12,7 @@
 
 int				nr_counters;
 
+int				grp_leaders[MAX_COUNTERS];
 struct perf_event_attr		attrs[MAX_COUNTERS];
 char				*filters[MAX_COUNTERS];
 
@@ -804,11 +805,13 @@ int parse_events(const struct option *opt __used, const char *str, int unset __u
 {
 	struct perf_event_attr attr;
 	enum event_result ret;
+	int grp_leader;
 
 	if (strchr(str, ':'))
 		if (store_event_type(str) < 0)
 			return -1;
 
+	grp_leader = nr_counters;
 	for (;;) {
 		if (nr_counters == MAX_COUNTERS)
 			return -1;
@@ -822,6 +825,7 @@ int parse_events(const struct option *opt __used, const char *str, int unset __u
 			return -1;
 
 		if (ret != EVT_HANDLED_ALL) {
+			grp_leaders[nr_counters] = grp_leader;
 			attrs[nr_counters] = attr;
 			nr_counters++;
 		}
diff --git a/tools/perf/util/parse-events.h b/tools/perf/util/parse-events.h
index fc4ab3f..d820f42 100644
--- a/tools/perf/util/parse-events.h
+++ b/tools/perf/util/parse-events.h
@@ -17,6 +17,7 @@ extern bool have_tracepoints(struct perf_event_attr *pattrs, int nb_events);
 
 extern int			nr_counters;
 
+extern int grp_leaders[MAX_COUNTERS];
 extern struct perf_event_attr attrs[MAX_COUNTERS];
 extern char *filters[MAX_COUNTERS];
 
-- 
1.7.0.4

Re: [RFC PATCHv3] perf tools: add event grouping capability to "perf stat"

[Peter Zijlstra]

On Wed, 2010-11-24 at 17:54 -0800, Corey Ashford wrote:
> Add the ability to create multiple event groups, each with their own leader
> using the existing "-e <event>[,<event> ...] [-e <event>[,<event>]]"
> syntax.  Each additional -e switch creates a new group, and each event
> listed within a -e switch is within that group.
> 
> Changes since v1:
> - Because of a flub, v2 did not contain the changes I had intended to make,
> and instead, v2 had the same patch contents as v1.
> - When perf stat is not supplied any events on the command line, put
> each default event in its own group.

I like this, but could you also extend this to perf-record? its a bit
odd to diverge between the two.

Using Stephane's latest syntax changes you could actually do something
like:

perf record -e task-clock:freq=1000,cycles:period=0

Which would create a group with 1 sampling counter and a counting
counter (at which point we should probably start flipping
PERF_SAMPLE_READ).

Matt was working on supporting that (although not through cmdline
syntax) and teaching perf-report to cope with such output.

Re: [RFC PATCHv3] perf tools: add event grouping capability to "perf stat"

[Frederic Weisbecker]

On Thu, Nov 25, 2010 at 07:32:40AM +0100, Peter Zijlstra wrote:
> On Wed, 2010-11-24 at 17:54 -0800, Corey Ashford wrote:
> > Add the ability to create multiple event groups, each with their own leader
> > using the existing "-e <event>[,<event> ...] [-e <event>[,<event>]]"
> > syntax.  Each additional -e switch creates a new group, and each event
> > listed within a -e switch is within that group.
> > 
> > Changes since v1:
> > - Because of a flub, v2 did not contain the changes I had intended to make,
> > and instead, v2 had the same patch contents as v1.
> > - When perf stat is not supplied any events on the command line, put
> > each default event in its own group.
> 
> I like this, but could you also extend this to perf-record? its a bit
> odd to diverge between the two.
> 
> Using Stephane's latest syntax changes you could actually do something
> like:
> 
> perf record -e task-clock:freq=1000,cycles:period=0

Wouldn't this syntax clash with the flags we have on events already?

the u,k,p flags?




> 
> Which would create a group with 1 sampling counter and a counting
> counter (at which point we should probably start flipping
> PERF_SAMPLE_READ).
> 
> Matt was working on supporting that (although not through cmdline
> syntax) and teaching perf-report to cope with such output.
> 

Re: [RFC PATCHv3] perf tools: add event grouping capability to "perf stat"

[Peter Zijlstra]

On Thu, 2010-11-25 at 08:46 +0100, Frederic Weisbecker wrote:
> > perf record -e task-clock:freq=1000,cycles:period=0
> 
> Wouldn't this syntax clash with the flags we have on events already?
> 
> the u,k,p flags? 

>From stephane's email (to which you were CC'ed) http://lkml.org/lkml/2010/11/23/303

> $ perf record -e cycles:u:period=100000,instructions:k:freq=1500 -a -- sleep 5

The parser isn't particularly pretty, maybe its time to think about a parser generator?

Re: [RFC PATCHv3] perf tools: add event grouping capability to "perf stat"

[Frederic Weisbecker]

On Thu, Nov 25, 2010 at 09:00:09AM +0100, Peter Zijlstra wrote:
> On Thu, 2010-11-25 at 08:46 +0100, Frederic Weisbecker wrote:
> > > perf record -e task-clock:freq=1000,cycles:period=0
> > 
> > Wouldn't this syntax clash with the flags we have on events already?
> > 
> > the u,k,p flags? 
> 
> From stephane's email (to which you were CC'ed) http://lkml.org/lkml/2010/11/23/303

Right, I'm going look at this.

 
> > $ perf record -e cycles:u:period=100000,instructions:k:freq=1500 -a -- sleep 5
> 
> The parser isn't particularly pretty, maybe its time to think about a parser generator?

May be yeah.

Re: [RFC PATCHv3] perf tools: add event grouping capability to "perf stat"

[Stephane Eranian]

On Thu, Nov 25, 2010 at 8:46 AM, Frederic Weisbecker <fweisbec@gmail.com> wrote:
> On Thu, Nov 25, 2010 at 07:32:40AM +0100, Peter Zijlstra wrote:
>> On Wed, 2010-11-24 at 17:54 -0800, Corey Ashford wrote:
>> > Add the ability to create multiple event groups, each with their own leader
>> > using the existing "-e <event>[,<event> ...] [-e <event>[,<event>]]"
>> > syntax.  Each additional -e switch creates a new group, and each event
>> > listed within a -e switch is within that group.
>> >
>> > Changes since v1:
>> > - Because of a flub, v2 did not contain the changes I had intended to make,
>> > and instead, v2 had the same patch contents as v1.
>> > - When perf stat is not supplied any events on the command line, put
>> > each default event in its own group.
>>
>> I like this, but could you also extend this to perf-record? its a bit
>> odd to diverge between the two.
>>
>> Using Stephane's latest syntax changes you could actually do something
>> like:
>>
>> perf record -e task-clock:freq=1000,cycles:period=0
>
My patch would have to be changed slightly to allow period=0.

But yes, lots of people have ask me for the possibility of sampling
on one event but recording the values of others in each sample.
The kernel supports this, I have written a couple of example in libpfm4.
The issue is not so much in perf record, but rather in perf report. How
to report the data? Most of the time this is for post-processing by other
tools.

> Wouldn't this syntax clash with the flags we have on events already?
>
> the u,k,p flags?
>
It does not.

>
>
>
>>
>> Which would create a group with 1 sampling counter and a counting
>> counter (at which point we should probably start flipping
>> PERF_SAMPLE_READ).
>>
>> Matt was working on supporting that (although not through cmdline
>> syntax) and teaching perf-report to cope with such output.
>>
>

Re: [RFC PATCHv3] perf tools: add event grouping capability to "perf stat"

[Stephane Eranian]

On Thu, Nov 25, 2010 at 7:32 AM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> On Wed, 2010-11-24 at 17:54 -0800, Corey Ashford wrote:
>> Add the ability to create multiple event groups, each with their own leader
>> using the existing "-e <event>[,<event> ...] [-e <event>[,<event>]]"
>> syntax.  Each additional -e switch creates a new group, and each event
>> listed within a -e switch is within that group.
>>
>> Changes since v1:
>> - Because of a flub, v2 did not contain the changes I had intended to make,
>> and instead, v2 had the same patch contents as v1.
>> - When perf stat is not supplied any events on the command line, put
>> each default event in its own group.
>
> I like this, but could you also extend this to perf-record? its a bit
> odd to diverge between the two.
>
> Using Stephane's latest syntax changes you could actually do something
> like:
>
> perf record -e task-clock:freq=1000,cycles:period=0
>
> Which would create a group with 1 sampling counter and a counting
> counter (at which point we should probably start flipping
> PERF_SAMPLE_READ).
>

I think using PERF_SAMPLE_READ may expose a problem in the
perf.data format. To correctly parse a sample created with SAMPLE_READ,
you need to know the attr.read_format. But for that you need to know the
event which caused the sample, but for that you need the SAMPLE_ID,
and you don't know if it's there or not. In other words, there is a chicken
and egg problem.

I think the issue is that PERF_RECORD_SAMPLE is missing a mandatory
piece of information: overflow event ID. This must a mandatory field, not
optional as it is today. It is okay when you have only one group, but we'd
like to go beyond that.

Re: [RFC PATCHv3] perf tools: add event grouping capability to "perf stat"

[Stephane Eranian]

On Thu, Nov 25, 2010 at 7:32 AM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> On Wed, 2010-11-24 at 17:54 -0800, Corey Ashford wrote:
>> Add the ability to create multiple event groups, each with their own leader
>> using the existing "-e <event>[,<event> ...] [-e <event>[,<event>]]"
>> syntax.  Each additional -e switch creates a new group, and each event
>> listed within a -e switch is within that group.
>>
>> Changes since v1:
>> - Because of a flub, v2 did not contain the changes I had intended to make,
>> and instead, v2 had the same patch contents as v1.
>> - When perf stat is not supplied any events on the command line, put
>> each default event in its own group.
>
> I like this, but could you also extend this to perf-record? its a bit
> odd to diverge between the two.
>
> Using Stephane's latest syntax changes you could actually do something
> like:
>
> perf record -e task-clock:freq=1000,cycles:period=0
>
> Which would create a group with 1 sampling counter and a counting
> counter (at which point we should probably start flipping
> PERF_SAMPLE_READ).
>

I think using PERF_SAMPLE_READ may expose a problem in the
perf.data format. To correctly parse a sample created with SAMPLE_READ,
you need to know the attr.read_format. But for that you need to know the
event which caused the sample, but for that you need the SAMPLE_ID,
and you don't know if it's there or not. In other words, there is a chicken
and egg problem.

I think the issue is that PERF_RECORD_SAMPLE is missing a mandatory
piece of information: overflow event ID. This must a mandatory field, not
optional as it is today. It is okay when you have only one group, but we'd
like to go beyond that.

Re: [RFC PATCHv3] perf tools: add event grouping capability to "perf stat"

[Stephane Eranian]

On Thu, Nov 25, 2010 at 2:19 PM, Stephane Eranian <eranian@google.com> wrote:
> On Thu, Nov 25, 2010 at 7:32 AM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
>> On Wed, 2010-11-24 at 17:54 -0800, Corey Ashford wrote:
>>> Add the ability to create multiple event groups, each with their own leader
>>> using the existing "-e <event>[,<event> ...] [-e <event>[,<event>]]"
>>> syntax.  Each additional -e switch creates a new group, and each event
>>> listed within a -e switch is within that group.
>>>
>>> Changes since v1:
>>> - Because of a flub, v2 did not contain the changes I had intended to make,
>>> and instead, v2 had the same patch contents as v1.
>>> - When perf stat is not supplied any events on the command line, put
>>> each default event in its own group.
>>
>> I like this, but could you also extend this to perf-record? its a bit
>> odd to diverge between the two.
>>
>> Using Stephane's latest syntax changes you could actually do something
>> like:
>>
>> perf record -e task-clock:freq=1000,cycles:period=0
>>
>> Which would create a group with 1 sampling counter and a counting
>> counter (at which point we should probably start flipping
>> PERF_SAMPLE_READ).
>>
>
> I think using PERF_SAMPLE_READ may expose a problem in the
> perf.data format. To correctly parse a sample created with SAMPLE_READ,
> you need to know the attr.read_format. But for that you need to know the
> event which caused the sample, but for that you need the SAMPLE_ID,
> and you don't know if it's there or not. In other words, there is a chicken
> and egg problem.
>
> I think the issue is that PERF_RECORD_SAMPLE is missing a mandatory
> piece of information: overflow event ID. This must a mandatory field, not
> optional as it is today. It is okay when you have only one group, but we'd
> like to go beyond that.
>
Second thought on this, I think the problem is in the kernel and not so much
in the perf.data file. Kernel must provide enough information to correlate
samples to events. It must do in a way that is not optional. Otherwise, as
soon as you have multiple groups, you won't be able to parse SAMPLE_READ.

Re: [RFC PATCHv3] perf tools: add event grouping capability to "perf stat"

[Peter Zijlstra]

On Thu, 2010-11-25 at 14:19 +0100, Stephane Eranian wrote:

> I think using PERF_SAMPLE_READ may expose a problem in the
> perf.data format. To correctly parse a sample created with SAMPLE_READ,
> you need to know the attr.read_format. But for that you need to know the
> event which caused the sample, but for that you need the SAMPLE_ID,
> and you don't know if it's there or not. In other words, there is a chicken
> and egg problem.
> 
> I think the issue is that PERF_RECORD_SAMPLE is missing a mandatory
> piece of information: overflow event ID. This must a mandatory field, not
> optional as it is today. It is okay when you have only one group, but we'd
> like to go beyond that.



I'm not sure I get it, there's both PERF_FORMAT_ID and PERF_SAMPLE_ID,
so afaict there's a working combination for what you want to do.

Re: [RFC PATCHv3] perf tools: add event grouping capability to "perf stat"

[Stephane Eranian]

On Thu, Nov 25, 2010 at 3:02 PM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> On Thu, 2010-11-25 at 14:19 +0100, Stephane Eranian wrote:
>
>> I think using PERF_SAMPLE_READ may expose a problem in the
>> perf.data format. To correctly parse a sample created with SAMPLE_READ,
>> you need to know the attr.read_format. But for that you need to know the
>> event which caused the sample, but for that you need the SAMPLE_ID,
>> and you don't know if it's there or not. In other words, there is a chicken
>> and egg problem.
>>
>> I think the issue is that PERF_RECORD_SAMPLE is missing a mandatory
>> piece of information: overflow event ID. This must a mandatory field, not
>> optional as it is today. It is okay when you have only one group, but we'd
>> like to go beyond that.
>
>
>
> I'm not sure I get it, there's both PERF_FORMAT_ID and PERF_SAMPLE_ID,
> so afaict there's a working combination for what you want to do.
>
Ok, I had forgotten about PERF_SAMPLE_ID. But that means that if the
number of groups > 1, then if you use PERF_SAMPLE_READ, you MUST
also use PERF_SAMPLE_ID. Otherwise you cannot get back to the event
that generated the sample and thus attr.read_format.

Re: [RFC PATCHv3] perf tools: add event grouping capability to "perf stat"

[Peter Zijlstra]

On Thu, 2010-11-25 at 15:07 +0100, Stephane Eranian wrote:
> On Thu, Nov 25, 2010 at 3:02 PM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> > On Thu, 2010-11-25 at 14:19 +0100, Stephane Eranian wrote:
> >
> >> I think using PERF_SAMPLE_READ may expose a problem in the
> >> perf.data format. To correctly parse a sample created with SAMPLE_READ,
> >> you need to know the attr.read_format. But for that you need to know the
> >> event which caused the sample, but for that you need the SAMPLE_ID,
> >> and you don't know if it's there or not. In other words, there is a chicken
> >> and egg problem.
> >>
> >> I think the issue is that PERF_RECORD_SAMPLE is missing a mandatory
> >> piece of information: overflow event ID. This must a mandatory field, not
> >> optional as it is today. It is okay when you have only one group, but we'd
> >> like to go beyond that.
> >
> >
> >
> > I'm not sure I get it, there's both PERF_FORMAT_ID and PERF_SAMPLE_ID,
> > so afaict there's a working combination for what you want to do.
> >
> Ok, I had forgotten about PERF_SAMPLE_ID. But that means that if the
> number of groups > 1, then if you use PERF_SAMPLE_READ, you MUST
> also use PERF_SAMPLE_ID. Otherwise you cannot get back to the event
> that generated the sample and thus attr.read_format.

perf-record already does that:

        if (nr_counters > 1)
                attr->sample_type |= PERF_SAMPLE_ID;

Re: [RFC PATCHv3] perf tools: add event grouping capability to "perf stat"

[Stephane Eranian]

On Thu, Nov 25, 2010 at 3:12 PM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> On Thu, 2010-11-25 at 15:07 +0100, Stephane Eranian wrote:
>> On Thu, Nov 25, 2010 at 3:02 PM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
>> > On Thu, 2010-11-25 at 14:19 +0100, Stephane Eranian wrote:
>> >
>> >> I think using PERF_SAMPLE_READ may expose a problem in the
>> >> perf.data format. To correctly parse a sample created with SAMPLE_READ,
>> >> you need to know the attr.read_format. But for that you need to know the
>> >> event which caused the sample, but for that you need the SAMPLE_ID,
>> >> and you don't know if it's there or not. In other words, there is a chicken
>> >> and egg problem.
>> >>
>> >> I think the issue is that PERF_RECORD_SAMPLE is missing a mandatory
>> >> piece of information: overflow event ID. This must a mandatory field, not
>> >> optional as it is today. It is okay when you have only one group, but we'd
>> >> like to go beyond that.
>> >
>> >
>> >
>> > I'm not sure I get it, there's both PERF_FORMAT_ID and PERF_SAMPLE_ID,
>> > so afaict there's a working combination for what you want to do.
>> >
>> Ok, I had forgotten about PERF_SAMPLE_ID. But that means that if the
>> number of groups > 1, then if you use PERF_SAMPLE_READ, you MUST
>> also use PERF_SAMPLE_ID. Otherwise you cannot get back to the event
>> that generated the sample and thus attr.read_format.
>
> perf-record already does that:
>
>        if (nr_counters > 1)
>                attr->sample_type |= PERF_SAMPLE_ID;
>
Ok, that's good.
The other thing I saw, is that perf report assumes that sample_type is the
same for all events, otherwise it dies.

Re: [RFC PATCHv3] perf tools: add event grouping capability to "perf stat"

[Peter Zijlstra]

On Thu, 2010-11-25 at 15:18 +0100, Stephane Eranian wrote:
> The other thing I saw, is that perf report assumes that sample_type is the
> same for all events, otherwise it dies. 

Yeah, event parsing in general needs some TLC..

Re: [RFC PATCHv3] perf tools: add event grouping capability to "perf stat"

[Lin Ming]

On Thu, Nov 25, 2010 at 10:22 PM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> On Thu, 2010-11-25 at 15:18 +0100, Stephane Eranian wrote:
>> The other thing I saw, is that perf report assumes that sample_type is the
>> same for all events, otherwise it dies.
>
> Yeah, event parsing in general needs some TLC..

Just curious, what does "TLC" stand for?

Re: [RFC PATCHv3] perf tools: add event grouping capability to "perf stat"

[Peter Zijlstra]

On Thu, 2010-11-25 at 23:10 +0800, Lin Ming wrote:
> On Thu, Nov 25, 2010 at 10:22 PM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> > On Thu, 2010-11-25 at 15:18 +0100, Stephane Eranian wrote:
> >> The other thing I saw, is that perf report assumes that sample_type is the
> >> same for all events, otherwise it dies.
> >
> > Yeah, event parsing in general needs some TLC..
> 
> Just curious, what does "TLC" stand for?

http://www.urbandictionary.com/define.php?term=TLC

I think #2 and #3 apply ;-)

Re: [RFC PATCHv3] perf tools: add event grouping capability to "perf stat"

[Lin Ming]

On Thu, Nov 25, 2010 at 11:15 PM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> On Thu, 2010-11-25 at 23:10 +0800, Lin Ming wrote:
>> On Thu, Nov 25, 2010 at 10:22 PM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
>> > On Thu, 2010-11-25 at 15:18 +0100, Stephane Eranian wrote:
>> >> The other thing I saw, is that perf report assumes that sample_type is the
>> >> same for all events, otherwise it dies.
>> >
>> > Yeah, event parsing in general needs some TLC..
>>
>> Just curious, what does "TLC" stand for?
>
> http://www.urbandictionary.com/define.php?term=TLC
>
> I think #2 and #3 apply ;-)

Interesting, Tender Love and Care.
I learn it.

Re: [RFC PATCHv3] perf tools: add event grouping capability to "perf stat"

[Arnaldo Carvalho de Melo]

Em Thu, Nov 25, 2010 at 03:18:58PM +0100, Stephane Eranian escreveu:
> On Thu, Nov 25, 2010 at 3:12 PM, Peter Zijlstra <a.p.zijlstra@chello.nl> wrote:
> > perf-record already does that:
> >
> >        if (nr_counters > 1)
> >                attr->sample_type |= PERF_SAMPLE_ID;
> >
> Ok, that's good.
> The other thing I saw, is that perf report assumes that sample_type is the
> same for all events, otherwise it dies.

Right, we need to have object attributes, and then pass them to
perf_session__new, so that it creates the counters, etc, instead of
having top, record, etc doing it in ad hoc ways.

I'll try to attack this soon, after tinishing the sample_type_id_all,
that in the kernel is per attribute, but in the tooling side suffers
from the one sample_type per session, not per attribute problem too.

- Arnaldo

Re: [RFC PATCHv3] perf tools: add event grouping capability to "perf stat"

[Corey Ashford]

On 11/24/2010 10:32 PM, Peter Zijlstra wrote:
> On Wed, 2010-11-24 at 17:54 -0800, Corey Ashford wrote:
>> Add the ability to create multiple event groups, each with their own leader
>> using the existing "-e<event>[,<event>  ...] [-e<event>[,<event>]]"
>> syntax.  Each additional -e switch creates a new group, and each event
>> listed within a -e switch is within that group.
>>
>> Changes since v1:
>> - Because of a flub, v2 did not contain the changes I had intended to make,
>> and instead, v2 had the same patch contents as v1.
>> - When perf stat is not supplied any events on the command line, put
>> each default event in its own group.
>
> I like this, but could you also extend this to perf-record? its a bit
> odd to diverge between the two.
>
> Using Stephane's latest syntax changes you could actually do something
> like:
>
> perf record -e task-clock:freq=1000,cycles:period=0
>
> Which would create a group with 1 sampling counter and a counting
> counter (at which point we should probably start flipping
> PERF_SAMPLE_READ).

Yes, that would be useful.

>
> Matt was working on supporting that (although not through cmdline
> syntax) and teaching perf-report to cope with such output.

I did briefly consider adding this capability to perf record, but I knew 
it would be a lot more complicated.

This perf stat capability is something we added to an internal version, 
and have been using it for more than 6 months.   It's quite helpful for 
verifying that the kernel code for an arch is implemented correctly.

As an alternative approach, how about if instead of changing the 
existing syntax to perf stat, I instead add a -g/--group option which 
takes groups of events?

That way we won't really be diverging perf record and perf stat; we'll 
just have a feature that can at some point later in time be added to 
perf record when all of the details are worked out.

- Corey