2010-11-25 01:59:13

by Corey Ashford

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

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 <[email protected]>
---
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


2010-11-25 06:32:52

by Peter Zijlstra

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

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.

2010-11-25 07:46:37

by Frederic Weisbecker

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

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.
>

2010-11-25 08:00:17

by Peter Zijlstra

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

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?

2010-11-25 08:10:16

by Frederic Weisbecker

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

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.

2010-11-25 09:16:32

by Stephane Eranian

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

On Thu, Nov 25, 2010 at 8:46 AM, Frederic Weisbecker <[email protected]> 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.
>>
>

2010-11-25 13:19:54

by Stephane Eranian

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

On Thu, Nov 25, 2010 at 7:32 AM, Peter Zijlstra <[email protected]> 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.

2010-11-25 13:20:06

by Stephane Eranian

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

On Thu, Nov 25, 2010 at 7:32 AM, Peter Zijlstra <[email protected]> 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.

2010-11-25 13:22:56

by Stephane Eranian

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

On Thu, Nov 25, 2010 at 2:19 PM, Stephane Eranian <[email protected]> wrote:
> On Thu, Nov 25, 2010 at 7:32 AM, Peter Zijlstra <[email protected]> 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.

2010-11-25 14:02:10

by Peter Zijlstra

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

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.

2010-11-25 14:07:30

by Stephane Eranian

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

On Thu, Nov 25, 2010 at 3:02 PM, Peter Zijlstra <[email protected]> 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.

2010-11-25 14:12:24

by Peter Zijlstra

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

On Thu, 2010-11-25 at 15:07 +0100, Stephane Eranian wrote:
> On Thu, Nov 25, 2010 at 3:02 PM, Peter Zijlstra <[email protected]> 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;


2010-11-25 14:19:05

by Stephane Eranian

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

On Thu, Nov 25, 2010 at 3:12 PM, Peter Zijlstra <[email protected]> wrote:
> On Thu, 2010-11-25 at 15:07 +0100, Stephane Eranian wrote:
>> On Thu, Nov 25, 2010 at 3:02 PM, Peter Zijlstra <[email protected]> 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.

2010-11-25 14:22:59

by Peter Zijlstra

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

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..

2010-11-25 15:10:39

by Lin Ming

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

On Thu, Nov 25, 2010 at 10:22 PM, Peter Zijlstra <[email protected]> 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?

2010-11-25 15:15:16

by Peter Zijlstra

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

On Thu, 2010-11-25 at 23:10 +0800, Lin Ming wrote:
> On Thu, Nov 25, 2010 at 10:22 PM, Peter Zijlstra <[email protected]> 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 ;-)

2010-11-25 15:19:22

by Lin Ming

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

On Thu, Nov 25, 2010 at 11:15 PM, Peter Zijlstra <[email protected]> wrote:
> On Thu, 2010-11-25 at 23:10 +0800, Lin Ming wrote:
>> On Thu, Nov 25, 2010 at 10:22 PM, Peter Zijlstra <[email protected]> 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.

2010-11-25 16:49:49

by Arnaldo Carvalho de Melo

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

Em Thu, Nov 25, 2010 at 03:18:58PM +0100, Stephane Eranian escreveu:
> On Thu, Nov 25, 2010 at 3:12 PM, Peter Zijlstra <[email protected]> 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

2010-11-26 19:22:09

by Corey Ashford

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

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