* [PATCH 1/1] Fix number of events displayed in header
@ 2012-03-20 19:23 Ashay Rane
2012-03-20 19:42 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 10+ messages in thread
From: Ashay Rane @ 2012-03-20 19:23 UTC (permalink / raw)
To: linux-perf-users, Peter Zijlstra, Paul Mackerras, Ingo Molnar,
Arnaldo Carvalho de Melo
Cc: linux-kernel, Ashay Rane
From: Ashay Rane <ashay.rane@tacc.utexas.edu>
Signed-off-by: Ashay Rane <ashay.rane@tacc.utexas.edu>
---
tools/perf/builtin-report.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 25d34d4..58a20cf 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -202,11 +202,9 @@ static size_t hists__fprintf_nr_sample_events(struct hists *self,
const char *evname, FILE *fp)
{
size_t ret;
- char unit;
- unsigned long nr_events = self->stats.nr_events[PERF_RECORD_SAMPLE];
+ u64 total_period = self->stats.total_period;
- nr_events = convert_unit(nr_events, &unit);
- ret = fprintf(fp, "# Events: %lu%c", nr_events, unit);
+ ret = fprintf(fp, "# Events: %llu", total_period);
if (evname != NULL)
ret += fprintf(fp, " %s", evname);
return ret + fprintf(fp, "\n#\n");
--
1.7.4.1
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 1/1] Fix number of events displayed in header
2012-03-20 19:23 [PATCH 1/1] Fix number of events displayed in header Ashay Rane
@ 2012-03-20 19:42 ` Arnaldo Carvalho de Melo
2012-03-20 19:46 ` David Ahern
0 siblings, 1 reply; 10+ messages in thread
From: Arnaldo Carvalho de Melo @ 2012-03-20 19:42 UTC (permalink / raw)
To: Ashay Rane
Cc: linux-perf-users, Peter Zijlstra, Paul Mackerras, Ingo Molnar,
linux-kernel, Ashay Rane
Em Tue, Mar 20, 2012 at 02:23:16PM -0500, Ashay Rane escreveu:
> From: Ashay Rane <ashay.rane@tacc.utexas.edu>
Too short a changeset comment, please elaborate.
The change at least would have to be changed from
"Events: %llu", total_period
to
"Total period: %llu", total_period
To be consistent in what we call "Events", i.e. right now it is telling
the number of PERF_RECORD_SAMPLE events, with your change it would be
showing the sum of the periods in all samples.
- Arnaldo
> Signed-off-by: Ashay Rane <ashay.rane@tacc.utexas.edu>
> ---
> tools/perf/builtin-report.c | 6 ++----
> 1 files changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 25d34d4..58a20cf 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -202,11 +202,9 @@ static size_t hists__fprintf_nr_sample_events(struct hists *self,
> const char *evname, FILE *fp)
> {
> size_t ret;
> - char unit;
> - unsigned long nr_events = self->stats.nr_events[PERF_RECORD_SAMPLE];
> + u64 total_period = self->stats.total_period;
>
> - nr_events = convert_unit(nr_events, &unit);
> - ret = fprintf(fp, "# Events: %lu%c", nr_events, unit);
> + ret = fprintf(fp, "# Events: %llu", total_period);
> if (evname != NULL)
> ret += fprintf(fp, " %s", evname);
> return ret + fprintf(fp, "\n#\n");
> --
> 1.7.4.1
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 1/1] Fix number of events displayed in header
2012-03-20 19:42 ` Arnaldo Carvalho de Melo
@ 2012-03-20 19:46 ` David Ahern
2012-03-20 19:47 ` Peter Zijlstra
0 siblings, 1 reply; 10+ messages in thread
From: David Ahern @ 2012-03-20 19:46 UTC (permalink / raw)
To: Arnaldo Carvalho de Melo, Ashay Rane
Cc: linux-perf-users, Peter Zijlstra, Paul Mackerras, Ingo Molnar,
linux-kernel, Ashay Rane
On 3/20/12 1:42 PM, Arnaldo Carvalho de Melo wrote:
> Em Tue, Mar 20, 2012 at 02:23:16PM -0500, Ashay Rane escreveu:
>> From: Ashay Rane<ashay.rane@tacc.utexas.edu>
>
> Too short a changeset comment, please elaborate.
>
> The change at least would have to be changed from
>
> "Events: %llu", total_period
>
> to
>
> "Total period: %llu", total_period
>
> To be consistent in what we call "Events", i.e. right now it is telling
> the number of PERF_RECORD_SAMPLE events, with your change it woul0d be
> showing the sum of the periods in all samples.
Does not seem appropriate to change from number of sample events to
period for all event types. This change assumes the event is cycles,
cpu-clock or task-clock.
David
>
> - Arnaldo
>
>> Signed-off-by: Ashay Rane<ashay.rane@tacc.utexas.edu>
>> ---
>> tools/perf/builtin-report.c | 6 ++----
>> 1 files changed, 2 insertions(+), 4 deletions(-)
>>
>> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
>> index 25d34d4..58a20cf 100644
>> --- a/tools/perf/builtin-report.c
>> +++ b/tools/perf/builtin-report.c
>> @@ -202,11 +202,9 @@ static size_t hists__fprintf_nr_sample_events(struct hists *self,
>> const char *evname, FILE *fp)
>> {
>> size_t ret;
>> - char unit;
>> - unsigned long nr_events = self->stats.nr_events[PERF_RECORD_SAMPLE];
>> + u64 total_period = self->stats.total_period;
>>
>> - nr_events = convert_unit(nr_events,&unit);
>> - ret = fprintf(fp, "# Events: %lu%c", nr_events, unit);
>> + ret = fprintf(fp, "# Events: %llu", total_period);
>> if (evname != NULL)
>> ret += fprintf(fp, " %s", evname);
>> return ret + fprintf(fp, "\n#\n");
>> --
>> 1.7.4.1
> --
> To unsubscribe from this list: send the line "unsubscribe linux-perf-users" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 1/1] Fix number of events displayed in header
2012-03-20 19:46 ` David Ahern
@ 2012-03-20 19:47 ` Peter Zijlstra
2012-03-20 19:51 ` Ashay Rane
0 siblings, 1 reply; 10+ messages in thread
From: Peter Zijlstra @ 2012-03-20 19:47 UTC (permalink / raw)
To: David Ahern
Cc: Arnaldo Carvalho de Melo, Ashay Rane, linux-perf-users,
Paul Mackerras, Ingo Molnar, linux-kernel, Ashay Rane
On Tue, 2012-03-20 at 13:46 -0600, David Ahern wrote:
>
> Does not seem appropriate to change from number of sample events to
> period for all event types. This change assumes the event is cycles,
> cpu-clock or task-clock.
I'm not seeing that assumption. That said, I don't see the point of the
change either.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] Fix number of events displayed in header
2012-03-20 19:47 ` Peter Zijlstra
@ 2012-03-20 19:51 ` Ashay Rane
2012-03-20 19:59 ` Peter Zijlstra
0 siblings, 1 reply; 10+ messages in thread
From: Ashay Rane @ 2012-03-20 19:51 UTC (permalink / raw)
To: Peter Zijlstra
Cc: David Ahern, Arnaldo Carvalho de Melo, linux-perf-users,
Paul Mackerras, Ingo Molnar, linux-kernel, Ashay Rane
Hello,
The problem I am seeing is that the `perf record' output does not
comform with the output of `perf stat'. For example, for `hackbench 10
process 1000', I see 19 x 10^9 cycles reported by `perf stat'.
However, `perf report -n' prints "Events: 2K cycles" and `perf report
-n --dso hackbench' prints "Events: 47 cycles".
The difference in the reported counts occurs for PMU events too.
$ perf stat -e cycles ./hackbench 10 process 1000
Running with 10*40 (== 400) tasks.
Time: 4.039
Performance counter stats for './hackbench 10 process 1000':
19,060,433,134 cycles # 0.000 GHz
4.078210213 seconds time elapsed
$ perf record -e cycles ./hackbench 10 process 1000
Running with 10*40 (== 400) tasks.
Time: 4.053
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.165 MB perf.data (~7205 samples) ]
$ perf report -n --stdio --dso hackbench
# dso: hackbench
# ========
# captured on: Mon Mar 12 15:14:57 2012
# hostname : iHitch
# os release : 3.2.8-1-ARCH
# perf version : 3.2-3
# arch : x86_64
# nrcpus online : 2
# nrcpus avail : 2
# cpudesc : Intel(R) Core(TM)2 Duo CPU P8600 @ 2.40GHz
# cpuid : GenuineIntel,6,23,10
# total memory : 2979384 kB
# cmdline : /usr/bin/perf record -e cycles ./hackbench 10 process 1000
# event : name = cycles, type = 0, config = 0x0, config1 = 0x0,
config2 = 0x0, excl_usr = 0, excl_kern = 0, id = { 1611, 1612 }
# HEADER_CPU_TOPOLOGY info available, use -I to display
# HEADER_NUMA_TOPOLOGY info available, use -I to display
# ========
#
# Events: 47 cycles
#
# Overhead Samples Command Symbol
# ........ .......... ......... .........
#
56.16% 24 hackbench receiver
40.21% 21 hackbench sender
3.63% 2 hackbench write@plt
#
# (For a higher level overview, try: perf report --sort comm,dso)
#
$
2012/3/20 Peter Zijlstra <a.p.zijlstra@chello.nl>:
> On Tue, 2012-03-20 at 13:46 -0600, David Ahern wrote:
>>
>> Does not seem appropriate to change from number of sample events to
>> period for all event types. This change assumes the event is cycles,
>> cpu-clock or task-clock.
>
> I'm not seeing that assumption. That said, I don't see the point of the
> change either.
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 1/1] Fix number of events displayed in header
2012-03-20 19:51 ` Ashay Rane
@ 2012-03-20 19:59 ` Peter Zijlstra
2012-03-20 20:04 ` Ashay Rane
0 siblings, 1 reply; 10+ messages in thread
From: Peter Zijlstra @ 2012-03-20 19:59 UTC (permalink / raw)
To: Ashay Rane
Cc: David Ahern, Arnaldo Carvalho de Melo, linux-perf-users,
Paul Mackerras, Ingo Molnar, linux-kernel, Ashay Rane
On Tue, 2012-03-20 at 14:51 -0500, Ashay Rane wrote:
> The problem I am seeing is that the `perf record' output does not
> comform with the output of `perf stat'. For example, for `hackbench 10
> process 1000', I see 19 x 10^9 cycles reported by `perf stat'.
> However, `perf report -n' prints "Events: 2K cycles" and `perf report
> -n --dso hackbench' prints "Events: 47 cycles".
Ah, read it as if perf report is saying: Samples: 47 of event: cycles.
That is, its telling you the cycles event had 47 overflows causing 47
samples to be taken (IP, etc..) and written out.
The point of reporting this number is that it puts the accuracy of the
percentages into perspective. The more samples, the greater the coverage
and more accurate the actual percentage of time spend in the various
functions.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] Fix number of events displayed in header
2012-03-20 19:59 ` Peter Zijlstra
@ 2012-03-20 20:04 ` Ashay Rane
2012-03-20 20:09 ` Peter Zijlstra
0 siblings, 1 reply; 10+ messages in thread
From: Ashay Rane @ 2012-03-20 20:04 UTC (permalink / raw)
To: Peter Zijlstra
Cc: David Ahern, Arnaldo Carvalho de Melo, linux-perf-users,
Paul Mackerras, Ingo Molnar, linux-kernel, Ashay Rane
Oh, okay. Thanks for the clarification!
However, my reasoning behind including the actual count of the events
was that (as far as I can tell) it is not displayed in any of the perf
report outputs. I understand that the count is not precise (because of
sampling errors) but the sampling frequency (-F) can always be
adjusted to get a more accurate number.
With the inclusion of the count, it then becomes possible to compare
multiple different versions of a program. Hence the patch.
Ashay
2012/3/20 Peter Zijlstra <a.p.zijlstra@chello.nl>:
> On Tue, 2012-03-20 at 14:51 -0500, Ashay Rane wrote:
>> The problem I am seeing is that the `perf record' output does not
>> comform with the output of `perf stat'. For example, for `hackbench 10
>> process 1000', I see 19 x 10^9 cycles reported by `perf stat'.
>> However, `perf report -n' prints "Events: 2K cycles" and `perf report
>> -n --dso hackbench' prints "Events: 47 cycles".
>
> Ah, read it as if perf report is saying: Samples: 47 of event: cycles.
>
> That is, its telling you the cycles event had 47 overflows causing 47
> samples to be taken (IP, etc..) and written out.
>
> The point of reporting this number is that it puts the accuracy of the
> percentages into perspective. The more samples, the greater the coverage
> and more accurate the actual percentage of time spend in the various
> functions.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] Fix number of events displayed in header
2012-03-20 20:04 ` Ashay Rane
@ 2012-03-20 20:09 ` Peter Zijlstra
2012-03-20 20:27 ` Ashay Rane
0 siblings, 1 reply; 10+ messages in thread
From: Peter Zijlstra @ 2012-03-20 20:09 UTC (permalink / raw)
To: Ashay Rane
Cc: David Ahern, Arnaldo Carvalho de Melo, linux-perf-users,
Paul Mackerras, Ingo Molnar, linux-kernel, Ashay Rane
On Tue, 2012-03-20 at 15:04 -0500, Ashay Rane wrote:
> Oh, okay. Thanks for the clarification!
>
> However, my reasoning behind including the actual count of the events
> was that (as far as I can tell) it is not displayed in any of the perf
> report outputs. I understand that the count is not precise (because of
> sampling errors) but the sampling frequency (-F) can always be
> adjusted to get a more accurate number.
>
> With the inclusion of the count, it then becomes possible to compare
> multiple different versions of a program. Hence the patch.
I'm okay if you change the wording of the existing message to be
clearer. I'm also okay if you add this extra information if you find
this useful.
In that case, also have a look at the data generated by perf record -s,
this dumps a PERF_RECORD_READ of the actual counter value at the exit of
every task [doesn't work with per-cpu counters].
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] Fix number of events displayed in header
2012-03-20 20:09 ` Peter Zijlstra
@ 2012-03-20 20:27 ` Ashay Rane
0 siblings, 0 replies; 10+ messages in thread
From: Ashay Rane @ 2012-03-20 20:27 UTC (permalink / raw)
To: Peter Zijlstra
Cc: David Ahern, Arnaldo Carvalho de Melo, linux-perf-users,
Paul Mackerras, Ingo Molnar, linux-kernel, Ashay Rane
Great, thanks! A (corrected) patch coming up...
Ashay
2012/3/20 Peter Zijlstra <a.p.zijlstra@chello.nl>:
> On Tue, 2012-03-20 at 15:04 -0500, Ashay Rane wrote:
>> Oh, okay. Thanks for the clarification!
>>
>> However, my reasoning behind including the actual count of the events
>> was that (as far as I can tell) it is not displayed in any of the perf
>> report outputs. I understand that the count is not precise (because of
>> sampling errors) but the sampling frequency (-F) can always be
>> adjusted to get a more accurate number.
>>
>> With the inclusion of the count, it then becomes possible to compare
>> multiple different versions of a program. Hence the patch.
>
> I'm okay if you change the wording of the existing message to be
> clearer. I'm also okay if you add this extra information if you find
> this useful.
>
> In that case, also have a look at the data generated by perf record -s,
> this dumps a PERF_RECORD_READ of the actual counter value at the exit of
> every task [doesn't work with per-cpu counters].
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/1] Fix number of events displayed in header
@ 2012-03-13 22:24 Ashay Rane
0 siblings, 0 replies; 10+ messages in thread
From: Ashay Rane @ 2012-03-13 22:24 UTC (permalink / raw)
To: linux-perf-users
Cc: Peter Zijlstra, Paul Mackerras, Ingo Molnar,
Arnaldo Carvalho de Melo, linux-kernel, Ashay Rane
Signed-off-by: Ashay Rane <ashay.rane@tacc.utexas.edu>
---
tools/perf/builtin-report.c | 6 ++----
1 files changed, 2 insertions(+), 4 deletions(-)
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 25d34d4..58a20cf 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -202,11 +202,9 @@ static size_t hists__fprintf_nr_sample_events(struct hists *self,
const char *evname, FILE *fp)
{
size_t ret;
- char unit;
- unsigned long nr_events = self->stats.nr_events[PERF_RECORD_SAMPLE];
+ u64 total_period = self->stats.total_period;
- nr_events = convert_unit(nr_events, &unit);
- ret = fprintf(fp, "# Events: %lu%c", nr_events, unit);
+ ret = fprintf(fp, "# Events: %llu", total_period);
if (evname != NULL)
ret += fprintf(fp, " %s", evname);
return ret + fprintf(fp, "\n#\n");
--
1.7.4.1
^ permalink raw reply related [flat|nested] 10+ messages in thread
end of thread, other threads:[~2012-03-20 20:27 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-03-20 19:23 [PATCH 1/1] Fix number of events displayed in header Ashay Rane
2012-03-20 19:42 ` Arnaldo Carvalho de Melo
2012-03-20 19:46 ` David Ahern
2012-03-20 19:47 ` Peter Zijlstra
2012-03-20 19:51 ` Ashay Rane
2012-03-20 19:59 ` Peter Zijlstra
2012-03-20 20:04 ` Ashay Rane
2012-03-20 20:09 ` Peter Zijlstra
2012-03-20 20:27 ` Ashay Rane
-- strict thread matches above, loose matches on Subject: below --
2012-03-13 22:24 Ashay Rane
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).