[PATCH v2 3/9] perf annotate: Fix wrong --show-total-period option showing number of samples

From: Taeung Song
Date: Thu Jul 13 2017 - 13:46:13 EST

Currently the --show-total-period option of perf-annotate
is different from perf-report's.

For example, perf-report ordinarily shows period and number of samples.

# Overhead Samples Period Command Shared Object Symbol
# ........ ............ ............ ....... .............. ............
#
9.83% 102 84813704 test test [.] knapsack

But --show-total-period of perf-annotate has the problem that show
number of samples, not period.
So fix this option to show period instead of number of samples.

Before:

Percent | Source code & Disassembly of old for cycles:ppp (102 samples)
-----------------------------------------------------------------------------
:
:
:
: Disassembly of section .text:
:
: 0000000000400816 <knapsack>:
: knapsack():
1 : 400816: push %rbp

After:

Percent | Source code & Disassembly of old for cycles:ppp (84813704 event count)
----------------------------------------------------------------------------------
:
:
:
: Disassembly of section .text:
:
: 0000000000400816 <knapsack>:
: knapsack():
743737 : 400816: push %rbp

Reported-by: Namhyung Kim <namhyung@xxxxxxxxxx>
Cc: Milian Wolff <milian.wolff@xxxxxxxx>
Cc: Jiri Olsa <jolsa@xxxxxxxxxx>
Signed-off-by: Taeung Song <treeze.taeung@xxxxxxxxx>
---
tools/perf/builtin-annotate.c | 4 +--
tools/perf/builtin-report.c | 13 ++++++----
tools/perf/builtin-top.c | 6 +++--
tools/perf/util/annotate.c | 57 +++++++++++++++++++++++++++++++++++++------
tools/perf/util/annotate.h | 4 ++-
5 files changed, 65 insertions(+), 19 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 7a5dc7e..f314661 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -177,14 +177,12 @@ static int perf_evsel__add_sample(struct perf_evsel *evsel,
*/
process_branch_stack(sample->branch_stack, al, sample);

- sample->period = 1;
sample->weight = 1;
-
he = hists__add_entry(hists, al, NULL, NULL, NULL, sample, true);
if (he == NULL)
return -ENOMEM;

- ret = hist_entry__inc_addr_samples(he, evsel->idx, al->addr);
+ ret = hist_entry__inc_addr_samples(he, sample, evsel->idx, al->addr);
hists__inc_nr_samples(hists, true);
return ret;
}
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 79a33eb..5f1894c 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -113,13 +113,14 @@ static int hist_iter__report_callback(struct hist_entry_iter *iter,
struct report *rep = arg;
struct hist_entry *he = iter->he;
struct perf_evsel *evsel = iter->evsel;
+ struct perf_sample *sample = iter->sample;
struct mem_info *mi;
struct branch_info *bi;

if (!ui__has_annotation())
return 0;

- hist__account_cycles(iter->sample->branch_stack, al, iter->sample,
+ hist__account_cycles(sample->branch_stack, al, sample,
rep->nonany_branch_mode);

if (sort__mode == SORT_MODE__BRANCH) {
@@ -136,14 +137,16 @@ static int hist_iter__report_callback(struct hist_entry_iter *iter,
if (err)
goto out;

- err = hist_entry__inc_addr_samples(he, evsel->idx, al->addr);
+ err = hist_entry__inc_addr_samples(he, sample,
+ evsel->idx, al->addr);

} else if (symbol_conf.cumulate_callchain) {
if (single)
- err = hist_entry__inc_addr_samples(he, evsel->idx,
- al->addr);
+ err = hist_entry__inc_addr_samples(he, sample,
+ evsel->idx, al->addr);
} else {
- err = hist_entry__inc_addr_samples(he, evsel->idx, al->addr);
+ err = hist_entry__inc_addr_samples(he, sample,
+ evsel->idx, al->addr);
}

out:
diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index 022486d..09885a4 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -183,6 +183,7 @@ static void ui__warn_map_erange(struct map *map, struct symbol *sym, u64 ip)

static void perf_top__record_precise_ip(struct perf_top *top,
struct hist_entry *he,
+ struct perf_sample *sample,
int counter, u64 ip)
{
struct annotation *notes;
@@ -199,7 +200,7 @@ static void perf_top__record_precise_ip(struct perf_top *top,
if (pthread_mutex_trylock(&notes->lock))
return;

- err = hist_entry__inc_addr_samples(he, counter, ip);
+ err = hist_entry__inc_addr_samples(he, sample, counter, ip);

pthread_mutex_unlock(&notes->lock);

@@ -671,7 +672,8 @@ static int hist_iter__top_callback(struct hist_entry_iter *iter,
struct perf_evsel *evsel = iter->evsel;

if (perf_hpp_list.sym && single)
- perf_top__record_precise_ip(top, he, evsel->idx, al->addr);
+ perf_top__record_precise_ip(top, he, iter->sample,
+ evsel->idx, al->addr);

hist__account_cycles(iter->sample->branch_stack, al, iter->sample,
!(top->record_opts.branch_stack & PERF_SAMPLE_BRANCH_ANY));
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index 9c2a6e0..7311a00 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -816,9 +816,33 @@ int addr_map_symbol__inc_samples(struct addr_map_symbol *ams, int evidx)
return symbol__inc_addr_samples(ams->sym, ams->map, evidx, ams->al_addr);
}

-int hist_entry__inc_addr_samples(struct hist_entry *he, int evidx, u64 ip)
+int hist_entry__inc_addr_samples(struct hist_entry *he, struct perf_sample *sample,
+ int evidx, u64 addr)
{
- return symbol__inc_addr_samples(he->ms.sym, he->ms.map, evidx, ip);
+ struct symbol *sym = he->ms.sym;
+ struct annotation *notes;
+ struct sym_hist *h;
+ s64 offset;
+
+ if (sym == NULL)
+ return 0;
+
+ notes = symbol__get_annotation(sym, false);
+ if (notes == NULL)
+ return -ENOMEM;
+
+ if ((addr < sym->start || addr >= sym->end) &&
+ (addr != sym->end || sym->start != sym->end))
+ return -ERANGE;
+
+ offset = addr - sym->start;
+ h = annotation__histogram(notes, evidx);
+ h->total_samples++;
+ h->addr[offset].nr_samples++;
+ h->total_period += sample->period;
+ h->addr[offset].period += sample->period;
+
+ return 0;
}

static void disasm_line__init_ins(struct disasm_line *dl, struct arch *arch, struct map *map)
@@ -934,6 +958,7 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset,
double percent = 0.0;

sample->nr_samples = 0;
+ sample->period = 0;

if (src_line) {
size_t sizeof_src_line = sizeof(*src_line) +
@@ -953,12 +978,16 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset,
} else {
struct sym_hist *h = annotation__histogram(notes, evidx);
unsigned int hits = 0;
+ unsigned int p = 0;

- while (offset < end)
- hits += h->addr[offset++].nr_samples;
+ while (offset < end) {
+ hits += h->addr[offset].nr_samples;
+ p += h->addr[offset++].period;
+ }

if (h->total_samples) {
sample->nr_samples = hits;
+ sample->period = p;
percent = 100.0 * hits / h->total_samples;
}
}
@@ -1131,8 +1160,8 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
color = get_percent_color(percent);

if (symbol_conf.show_total_period)
- color_fprintf(stdout, color, " %7" PRIu64,
- sample.nr_samples);
+ color_fprintf(stdout, color, " %11" PRIu64,
+ sample.period);
else
color_fprintf(stdout, color, " %7.2f", percent);
}
@@ -1162,6 +1191,10 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
if (perf_evsel__is_group_event(evsel))
width *= evsel->nr_members;

+ if (symbol_conf.show_total_period)
+ width += perf_evsel__is_group_event(evsel) ?
+ 4 * evsel->nr_members : 4;
+
if (!*dl->line)
printf(" %*s:\n", width, " ");
else
@@ -1704,6 +1737,7 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map,

h = annotation__histogram(notes, evidx + k);
nr_samples = h->addr[i].nr_samples;
+
if (h->total_samples)
percent = 100.0 * nr_samples / h->total_samples;

@@ -1777,6 +1811,7 @@ static void symbol__annotate_hits(struct symbol *sym, struct perf_evsel *evsel)
if (h->addr[offset].nr_samples != 0)
printf("%*" PRIx64 ": %" PRIu64 "\n", BITS_PER_LONG / 2,
sym->start + offset, h->addr[offset].nr_samples);
+
printf("%*s: %" PRIu64 "\n", BITS_PER_LONG / 2, "h->total_samples", h->total_samples);
}

@@ -1812,8 +1847,14 @@ int symbol__annotate_printf(struct symbol *sym, struct map *map,
if (perf_evsel__is_group_event(evsel))
width *= evsel->nr_members;

- graph_dotted_len = printf(" %-*.*s| Source code & Disassembly of %s for %s (%" PRIu64 " samples)\n",
- width, width, "Percent", d_filename, evsel_name, h->total_samples);
+ if (symbol_conf.show_total_period)
+ width += perf_evsel__is_group_event(evsel) ?
+ 4 * evsel->nr_members : 4;
+
+ graph_dotted_len = printf(" %-*.*s| Source code & Disassembly of %s for %s (%" PRIu64 " %s)\n",
+ width, width, "Percent", d_filename, evsel_name,
+ symbol_conf.show_total_period ? h->total_period : h->total_samples,
+ symbol_conf.show_total_period ? "event count" : "samples");

printf("%-*.*s----\n",
graph_dotted_len, graph_dotted_len, graph_dotted_line);
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index bef1d02..4406352 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -88,6 +88,7 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset,

struct sym_hist {
u64 total_samples;
+ u64 total_period;
struct sym_hist_entry addr[0];
};

@@ -160,7 +161,8 @@ int addr_map_symbol__account_cycles(struct addr_map_symbol *ams,
struct addr_map_symbol *start,
unsigned cycles);

-int hist_entry__inc_addr_samples(struct hist_entry *he, int evidx, u64 addr);
+int hist_entry__inc_addr_samples(struct hist_entry *he, struct perf_sample *sample,
+ int evidx, u64 addr);

int symbol__alloc_hist(struct symbol *sym);
void symbol__annotate_zero_histograms(struct symbol *sym);
--
2.7.4