Re: [PATCH 3/4] perf annotate: Support --show-nr-samples option

From: Taeung Song
Date: Thu Jul 13 2017 - 09:11:53 EST


Arnaldo,

Additionally I think we need to calculate the percentage with the sample period,
not number of samples like perf-report.
What do you think about it ?

For examples, perf-annotate figure out the percentage with number of samples like below
ui/gtk/annotate.c:

23 static int perf_gtk__get_percent(char *buf, size_t size, struct symbol *sym,
24 struct disasm_line *dl, int evidx)
25 {
...
36 symhist = annotation__histogram(symbol__annotation(sym), evidx);
37 if (!symbol_conf.event_group && !symhist->addr[dl->offset])
38 return 0;
39
40 percent = 100.0 * symhist->addr[dl->offset] / symhist->sum;

But perf-report do it with the sample period like below.
ui/hist.c:

363 HPP_PERCENT_FNS(overhead, period)
364 HPP_PERCENT_FNS(overhead_sys, period_sys)
365 HPP_PERCENT_FNS(overhead_us, period_us)
366 HPP_PERCENT_FNS(overhead_guest_sys, period_guest_sys)
367 HPP_PERCENT_FNS(overhead_guest_us, period_guest_us)
368 HPP_PERCENT_ACC_FNS(overhead_acc, period)


Thanks,
Taeung

On 07/13/2017 05:13 AM, Arnaldo Carvalho de Melo wrote:
Em Wed, Jul 12, 2017 at 07:14:14AM +0900, Taeung Song escreveu:
Add --show-nr-samples option to perf-annotate
so that it corresponds with perf-report.
Additionally h->sum is properly renamed h->nr_samples.

Please first do a patch renaming ->sum to ->nr_samples, then a patch
supportign --show-nr-samples.

We can have this new --show-nr-samples option, but perhaps the best way
would be to have a hotkey (I guess we have for period, right) for this?

At some point would be good to have some 'S' or other hotkey to save the
current set of output modifiers (nr-samples column, period column, etc)
in the .perfconfig file.

- Arnaldo
Cc: 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 | 2 ++
tools/perf/ui/gtk/annotate.c | 2 +-
tools/perf/util/annotate.c | 30 +++++++++++++++++-------------
tools/perf/util/annotate.h | 2 +-
4 files changed, 21 insertions(+), 15 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index f314661..3cb0223 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -444,6 +444,8 @@ int cmd_annotate(int argc, const char **argv)
"Show event group information together"),
OPT_BOOLEAN(0, "show-total-period", &symbol_conf.show_total_period,
"Show a column with the sum of periods"),
+ OPT_BOOLEAN('n', "show-nr-samples", &symbol_conf.show_nr_samples,
+ "Show a column with the number of samples"),
OPT_CALLBACK_DEFAULT(0, "stdio-color", NULL, "mode",
"'always' (default), 'never' or 'auto' only applicable to --stdio mode",
stdio__config_color, "always"),
diff --git a/tools/perf/ui/gtk/annotate.c b/tools/perf/ui/gtk/annotate.c
index d736fd5..0217619 100644
--- a/tools/perf/ui/gtk/annotate.c
+++ b/tools/perf/ui/gtk/annotate.c
@@ -37,7 +37,7 @@ static int perf_gtk__get_percent(char *buf, size_t size, struct symbol *sym,
if (!symbol_conf.event_group && !symhist->addr[dl->offset].nr_samples)
return 0;
- percent = 100.0 * symhist->addr[dl->offset].nr_samples / symhist->sum;
+ percent = 100.0 * symhist->addr[dl->offset].nr_samples / symhist->nr_samples;
markup = perf_gtk__get_percent_color(percent);
if (markup)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index f7aeb5f..b19e734 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -713,7 +713,7 @@ static int __symbol__inc_addr_samples(struct symbol *sym, struct map *map,
offset = addr - sym->start;
h = annotation__histogram(notes, evidx);
- h->sum++;
+ h->nr_samples++;
h->addr[offset].nr_samples++;
pr_debug3("%#" PRIx64 " %s: period++ [addr: %#" PRIx64 ", %#" PRIx64
@@ -837,7 +837,7 @@ int hist_entry__inc_addr_samples(struct hist_entry *he, struct perf_sample *samp
offset = addr - sym->start;
h = annotation__histogram(notes, evidx);
- h->sum++;
+ h->nr_samples++;
h->addr[offset].nr_samples++;
h->total_period += sample->period;
h->addr[offset].period += sample->period;
@@ -985,10 +985,10 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset,
p += h->addr[offset++].period;
}
- if (h->sum) {
+ if (h->nr_samples) {
*nr_samples = hits;
*period = p;
- percent = 100.0 * hits / h->sum;
+ percent = 100.0 * hits / h->nr_samples;
}
}
@@ -1165,6 +1165,9 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
if (symbol_conf.show_total_period)
color_fprintf(stdout, color, " %11" PRIu64,
period);
+ else if (symbol_conf.show_nr_samples)
+ color_fprintf(stdout, color, " %7" PRIu64,
+ nr_samples);
else
color_fprintf(stdout, color, " %7.2f", percent);
}
@@ -1711,13 +1714,13 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map,
struct sym_hist *h = annotation__histogram(notes, evidx);
struct rb_root tmp_root = RB_ROOT;
int nr_pcnt = 1;
- u64 h_sum = h->sum;
+ u64 h_sum = h->nr_samples;
size_t sizeof_src_line = sizeof(struct source_line);
if (perf_evsel__is_group_event(evsel)) {
for (i = 1; i < evsel->nr_members; i++) {
h = annotation__histogram(notes, evidx + i);
- h_sum += h->sum;
+ h_sum += h->nr_samples;
}
nr_pcnt = evsel->nr_members;
sizeof_src_line += (nr_pcnt - 1) * sizeof(src_line->samples);
@@ -1743,8 +1746,8 @@ 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->sum)
- percent = 100.0 * nr_samples / h->sum;
+ if (h->nr_samples)
+ percent = 100.0 * nr_samples / h->nr_samples;
if (percent > percent_max)
percent_max = percent;
@@ -1816,7 +1819,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->sum", h->sum);
+ printf("%*s: %" PRIu64 "\n", BITS_PER_LONG / 2, "h->nr_samples", h->nr_samples);
}
int symbol__annotate_printf(struct symbol *sym, struct map *map,
@@ -1857,9 +1860,10 @@ int symbol__annotate_printf(struct symbol *sym, struct map *map,
graph_dotted_len = printf(" %-*.*s| Source code & Disassembly of %s for %s (%" PRIu64 " %s)\n",
width, width,
- symbol_conf.show_total_period ? "Event count" : "Percent",
+ symbol_conf.show_total_period ? "Event count" :
+ symbol_conf.show_nr_samples ? "Samples" : "Percent",
d_filename, evsel_name,
- symbol_conf.show_total_period ? h->total_period : h->sum,
+ symbol_conf.show_total_period ? h->total_period : h->nr_samples,
symbol_conf.show_total_period ? "event count" : "samples");
printf("%-*.*s----\n",
@@ -1924,10 +1928,10 @@ void symbol__annotate_decay_histogram(struct symbol *sym, int evidx)
struct sym_hist *h = annotation__histogram(notes, evidx);
int len = symbol__size(sym), offset;
- h->sum = 0;
+ h->nr_samples = 0;
for (offset = 0; offset < len; ++offset) {
h->addr[offset].nr_samples = h->addr[offset].nr_samples * 7 / 8;
- h->sum += h->addr[offset].nr_samples;
+ h->nr_samples += h->addr[offset].nr_samples;
}
}
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index 6b2e645..e4d444a 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -86,7 +86,7 @@ struct sym_sample {
};
struct sym_hist {
- u64 sum;
+ u64 nr_samples;
u64 total_period;
struct sym_sample addr[0];
};
--
2.7.4