Skip to contentSkip to conversation

perf: fix broken number of samples for perf report -n

PATCH In review Stephane Eranian 2 participants 2 messages Oct 3, 2011 perf events

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 perf: fix broken number of samples for perf report -n

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

---

tools/perf/util/hist.c +1 -0
index 677e1da..649eb99 100644
@@ -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,
#1 lore raw

Em Mon, Oct 03, 2011 at 11:38:15AM +0200, Stephane Eranian escreveu:

Quoted 6 lines

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.

Quoted 8 lines

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