* [PATCH] perf: fix broken number of samples for perf report -n
@ 2011-10-03 9:38 Stephane Eranian
2011-10-03 16:25 ` Arnaldo Carvalho de Melo
0 siblings, 1 reply; 2+ messages in thread
From: Stephane Eranian @ 2011-10-03 9:38 UTC (permalink / raw)
To: linux-kernel; +Cc: acme, peterz, mingo, dsahern
The perf report -n option was broken because it was not reporting
the correct number of samples depending on the sorting mode. By
default, samples are sorted by comm,dso,sym. That means that
samples for the same command (binary) get collapsed.
The hists__collapse_insert_entry() had a bug whereby
it was aggregating the number of events observed (periods)
but not the number of samples. Consequently, the number
of samples reported could be below reality. The percentage
remained correct because based on the periods.
This patch fixes the problem by also aggregating
the number of samples. Here is an example:
$ perf report -n --stdio
# Events: 13K cycles
#
# Overhead Samples Command Shared Object Symbol
# ........ .......... ....... .................... ...................
#
12.38% 842 pong [kernel.kallsyms] [k] __lock_acquire
Here pong (a ctxsw stress test), is the only program running
and thus it is the only one responsible for the lock_acquire samples.
If we change the sorting mode:
$ perf report -n --stdio --sort=sym
# Events: 13K cycles
#
# Overhead Samples Symbol
# ........ .......... ...........................
#
12.38% 1732 [k] __lock_acquire
The actual number of samples is shown.
With the fix:
$ perf report -n --stdio
# Events: 13K cycles
#
# Overhead Samples Command Shared Object Symbol
# ........ .......... ....... .................... ...................
#
12.38% 1732 pong [kernel.kallsyms] [k] __lock_acquire
Signed-off-by: Stephane Eranian <eranian@google.com>
---
diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 677e1da..649eb99 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -239,6 +239,7 @@ static bool hists__collapse_insert_entry(struct hists *self,
if (!cmp) {
iter->period += he->period;
+ iter->nr_events += he->nr_events;
if (symbol_conf.use_callchain) {
callchain_cursor_reset(&self->callchain_cursor);
callchain_merge(&self->callchain_cursor, iter->callchain,
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] perf: fix broken number of samples for perf report -n
2011-10-03 9:38 [PATCH] perf: fix broken number of samples for perf report -n Stephane Eranian
@ 2011-10-03 16:25 ` Arnaldo Carvalho de Melo
0 siblings, 0 replies; 2+ messages in thread
From: Arnaldo Carvalho de Melo @ 2011-10-03 16:25 UTC (permalink / raw)
To: Stephane Eranian; +Cc: linux-kernel, peterz, mingo, dsahern
Em Mon, Oct 03, 2011 at 11:38:15AM +0200, Stephane Eranian escreveu:
>
> The perf report -n option was broken because it was not reporting
> the correct number of samples depending on the sorting mode. By
> default, samples are sorted by comm,dso,sym. That means that
> samples for the same command (binary) get collapsed.
>
> The hists__collapse_insert_entry() had a bug whereby
> it was aggregating the number of events observed (periods)
> but not the number of samples. Consequently, the number
> of samples reported could be below reality. The percentage
> remained correct because based on the periods.
>
> This patch fixes the problem by also aggregating
> the number of samples. Here is an example:
Eagle eyes, this collapse_resort thing has its issues, been fighting
some last week, making 'perf top' use struct hist_entry, killing code.
Double checking that now, thanks!
- Arnaldo
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2011-10-03 16:25 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-10-03 9:38 [PATCH] perf: fix broken number of samples for perf report -n Stephane Eranian
2011-10-03 16:25 ` Arnaldo Carvalho de Melo
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox