2017-07-11 22:14:59

by Taeung Song

Subject: [PATCH 1/4] perf annotate: Fix wrong --show-total-period option showing number of samples

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 two problem like below:

1) Wrong column i.e. 'Percent'
2) 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:

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

Cc: Namhyung Kim <[email protected]>
Cc: Milian Wolff <[email protected]>
Cc: Jiri Olsa <[email protected]>
Signed-off-by: Taeung Song <[email protected]>
---
tools/perf/builtin-annotate.c | 4 +-
tools/perf/builtin-report.c | 13 +++---
tools/perf/builtin-top.c | 6 ++-
tools/perf/ui/browsers/annotate.c | 4 +-
tools/perf/ui/gtk/annotate.c | 4 +-
tools/perf/util/annotate.c | 92 +++++++++++++++++++++++++++++----------
tools/perf/util/annotate.h | 12 +++--
7 files changed, 96 insertions(+), 39 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/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index b376637..73e5ae2 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -449,13 +449,13 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser,
next = disasm__get_next_ip_line(&notes->src->source, pos);

for (i = 0; i < browser->nr_events; i++) {
- u64 nr_samples;
+ u64 nr_samples, period;

bpos->samples[i].percent = disasm__calc_percent(notes,
evsel->idx + i,
pos->offset,
next ? next->offset : len,
- &path, &nr_samples);
+ &path, &nr_samples, &period);
bpos->samples[i].nr = nr_samples;

if (max_percent < bpos->samples[i].percent)
diff --git a/tools/perf/ui/gtk/annotate.c b/tools/perf/ui/gtk/annotate.c
index 87e3760..d736fd5 100644
--- a/tools/perf/ui/gtk/annotate.c
+++ b/tools/perf/ui/gtk/annotate.c
@@ -34,10 +34,10 @@ static int perf_gtk__get_percent(char *buf, size_t size, struct symbol *sym,
return 0;

symhist = annotation__histogram(symbol__annotation(sym), evidx);
- if (!symbol_conf.event_group && !symhist->addr[dl->offset])
+ if (!symbol_conf.event_group && !symhist->addr[dl->offset].nr_samples)
return 0;

- percent = 100.0 * symhist->addr[dl->offset] / symhist->sum;
+ percent = 100.0 * symhist->addr[dl->offset].nr_samples / symhist->sum;

markup = perf_gtk__get_percent_color(percent);
if (markup)
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index ef434b5..f7aeb5f 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -610,10 +610,10 @@ int symbol__alloc_hist(struct symbol *sym)
size_t sizeof_sym_hist;

/* Check for overflow when calculating sizeof_sym_hist */
- if (size > (SIZE_MAX - sizeof(struct sym_hist)) / sizeof(u64))
+ if (size > (SIZE_MAX - sizeof(struct sym_hist)) / sizeof(struct sym_sample))
return -1;

- sizeof_sym_hist = (sizeof(struct sym_hist) + size * sizeof(u64));
+ sizeof_sym_hist = (sizeof(struct sym_hist) + size * sizeof(struct sym_sample));

/* Check for overflow in zalloc argument */
if (sizeof_sym_hist > (SIZE_MAX - sizeof(*notes->src))
@@ -714,11 +714,11 @@ static int __symbol__inc_addr_samples(struct symbol *sym, struct map *map,
offset = addr - sym->start;
h = annotation__histogram(notes, evidx);
h->sum++;
- h->addr[offset]++;
+ h->addr[offset].nr_samples++;

pr_debug3("%#" PRIx64 " %s: period++ [addr: %#" PRIx64 ", %#" PRIx64
", evidx=%d] => %" PRIu64 "\n", sym->start, sym->name,
- addr, addr - sym->start, evidx, h->addr[offset]);
+ addr, addr - sym->start, evidx, h->addr[offset].nr_samples);
return 0;
}

@@ -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->sum++;
+ 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)
@@ -928,11 +952,13 @@ struct disasm_line *disasm__get_next_ip_line(struct list_head *head, struct disa
}

double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset,
- s64 end, const char **path, u64 *nr_samples)
+ s64 end, const char **path, u64 *nr_samples, u64 *period)
{
struct source_line *src_line = notes->src->lines;
double percent = 0.0;
+
*nr_samples = 0;
+ *period = 0;

if (src_line) {
size_t sizeof_src_line = sizeof(*src_line) +
@@ -952,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++];
+ while (offset < end) {
+ hits += h->addr[offset].nr_samples;
+ p += h->addr[offset++].period;
+ }

if (h->sum) {
*nr_samples = hits;
+ *period = p;
percent = 100.0 * hits / h->sum;
}
}
@@ -1057,10 +1087,11 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st

if (dl->offset != -1) {
const char *path = NULL;
- u64 nr_samples;
+ u64 nr_samples, period;
double percent, max_percent = 0.0;
double *ppercents = &percent;
u64 *psamples = &nr_samples;
+ u64 *pperiod = &period;
int i, nr_percent = 1;
const char *color;
struct annotation *notes = symbol__annotation(sym);
@@ -1075,9 +1106,9 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
nr_percent = evsel->nr_members;
ppercents = calloc(nr_percent, sizeof(double));
psamples = calloc(nr_percent, sizeof(u64));
- if (ppercents == NULL || psamples == NULL) {
+ pperiod = calloc(nr_percent, sizeof(u64));
+ if (ppercents == NULL || psamples == NULL || pperiod == NULL)
return -1;
- }
}

for (i = 0; i < nr_percent; i++) {
@@ -1085,10 +1116,11 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
notes->src->lines ? i : evsel->idx + i,
offset,
next ? next->offset : (s64) len,
- &path, &nr_samples);
+ &path, &nr_samples, &period);

ppercents[i] = percent;
psamples[i] = nr_samples;
+ pperiod[i] = period;
if (percent > max_percent)
max_percent = percent;
}
@@ -1127,11 +1159,12 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
for (i = 0; i < nr_percent; i++) {
percent = ppercents[i];
nr_samples = psamples[i];
+ period = pperiod[i];
color = get_percent_color(percent);

if (symbol_conf.show_total_period)
- color_fprintf(stdout, color, " %7" PRIu64,
- nr_samples);
+ color_fprintf(stdout, color, " %11" PRIu64,
+ period);
else
color_fprintf(stdout, color, " %7.2f", percent);
}
@@ -1150,6 +1183,9 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
if (psamples != &nr_samples)
free(psamples);

+ if (pperiod != &period)
+ free(pperiod);
+
} else if (max_lines && printed >= max_lines)
return 1;
else {
@@ -1161,6 +1197,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
@@ -1702,7 +1742,7 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map,
double percent = 0.0;

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

@@ -1773,9 +1813,9 @@ static void symbol__annotate_hits(struct symbol *sym, struct perf_evsel *evsel)
u64 len = symbol__size(sym), offset;

for (offset = 0; offset < len; ++offset)
- if (h->addr[offset] != 0)
+ if (h->addr[offset].nr_samples != 0)
printf("%*" PRIx64 ": %" PRIu64 "\n", BITS_PER_LONG / 2,
- sym->start + offset, h->addr[offset]);
+ sym->start + offset, h->addr[offset].nr_samples);
printf("%*s: %" PRIu64 "\n", BITS_PER_LONG / 2, "h->sum", h->sum);
}

@@ -1811,8 +1851,16 @@ 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->sum);
+ 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,
+ symbol_conf.show_total_period ? "Event count" : "Percent",
+ d_filename, evsel_name,
+ symbol_conf.show_total_period ? h->total_period : h->sum,
+ symbol_conf.show_total_period ? "event count" : "samples");

printf("%-*.*s----\n",
graph_dotted_len, graph_dotted_len, graph_dotted_line);
@@ -1878,8 +1926,8 @@ void symbol__annotate_decay_histogram(struct symbol *sym, int evidx)

h->sum = 0;
for (offset = 0; offset < len; ++offset) {
- h->addr[offset] = h->addr[offset] * 7 / 8;
- h->sum += h->addr[offset];
+ h->addr[offset].nr_samples = h->addr[offset].nr_samples * 7 / 8;
+ h->sum += h->addr[offset].nr_samples;
}
}

diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index bac698d..6b2e645 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -79,11 +79,16 @@ struct disasm_line *disasm__get_next_ip_line(struct list_head *head, struct disa
int disasm_line__scnprintf(struct disasm_line *dl, char *bf, size_t size, bool raw);
size_t disasm__fprintf(struct list_head *head, FILE *fp);
double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset,
- s64 end, const char **path, u64 *nr_samples);
+ s64 end, const char **path, u64 *nr_samples, u64 *period);
+struct sym_sample {
+ u64 nr_samples;
+ u64 period;
+};

struct sym_hist {
u64 sum;
- u64 addr[0];
+ u64 total_period;
+ struct sym_sample addr[0];
};

struct cyc_hist {
@@ -155,7 +160,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

2017-07-12 20:09:16

by Arnaldo Carvalho de Melo

Subject: Re: [PATCH 1/4] perf annotate: Fix wrong --show-total-period option showing number of samples

Em Wed, Jul 12, 2017 at 07:14:04AM +0900, Taeung Song escreveu:
> 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 two problem like below:
>
> 1) Wrong column i.e. 'Percent'
> 2) Show number of samples, not period
>
> So fix this option to show period instead of number of samples.

so you have multiple bugs here, please fix one per patch, i.e. if using
--show-total-period the header should not be "Percent".

Then, you need a patch to introduce that "struct sym_sample" and use it,
but please rename it to "struct sym_hist_entry".

You can do it and just update the sym_hist_entry->period field, before
the change to pass 'struct sample' around.

That disasm__calc_percent() function could receive a pointer to a struct
sym_hist_entry instead of two pointers.

Small, self contained patches that do one thing make reviewing easier,
by yourself and others.

Please give this a try, if you didn't understand something here I can do
it for you, as this needs doing to fix real bugs.

Thanks,

- Arnaldo

> Before:
>
> Percent | Source code & Disassembly of old for cycles:ppp (102 samples)
> -----------------------------------------------------------------------------
> :
> :
> :
> : Disassembly of section .text:
> :
> : 0000000000400816 <knapsack>:
> : knapsack():
> 1 : 400816: push %rbp
>
> After:
>
> Event count | Source code & Disassembly of old for cycles:ppp (84813704 event count)
> --------------------------------------------------------------------------------------
> :
> :
> :
> : Disassembly of section .text:
> :
> : 0000000000400816 <knapsack>:
> : knapsack():
> 743737 : 400816: push %rbp
>
> Cc: Namhyung Kim <[email protected]>
> Cc: Milian Wolff <[email protected]>
> Cc: Jiri Olsa <[email protected]>
> Signed-off-by: Taeung Song <[email protected]>
> ---
> tools/perf/builtin-annotate.c | 4 +-
> tools/perf/builtin-report.c | 13 +++---
> tools/perf/builtin-top.c | 6 ++-
> tools/perf/ui/browsers/annotate.c | 4 +-
> tools/perf/ui/gtk/annotate.c | 4 +-
> tools/perf/util/annotate.c | 92 +++++++++++++++++++++++++++++----------
> tools/perf/util/annotate.h | 12 +++--
> 7 files changed, 96 insertions(+), 39 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;
> -

Please do not remove this line.

> 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);

why break this into two lines?

>
> } 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/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index b376637..73e5ae2 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -449,13 +449,13 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser,
> next = disasm__get_next_ip_line(&notes->src->source, pos);
>
> for (i = 0; i < browser->nr_events; i++) {
> - u64 nr_samples;
> + u64 nr_samples, period;
>
> bpos->samples[i].percent = disasm__calc_percent(notes,
> evsel->idx + i,
> pos->offset,
> next ? next->offset : len,
> - &path, &nr_samples);
> + &path, &nr_samples, &period);
> bpos->samples[i].nr = nr_samples;
>
> if (max_percent < bpos->samples[i].percent)
> diff --git a/tools/perf/ui/gtk/annotate.c b/tools/perf/ui/gtk/annotate.c
> index 87e3760..d736fd5 100644
> --- a/tools/perf/ui/gtk/annotate.c
> +++ b/tools/perf/ui/gtk/annotate.c
> @@ -34,10 +34,10 @@ static int perf_gtk__get_percent(char *buf, size_t size, struct symbol *sym,
> return 0;
>
> symhist = annotation__histogram(symbol__annotation(sym), evidx);
> - if (!symbol_conf.event_group && !symhist->addr[dl->offset])
> + if (!symbol_conf.event_group && !symhist->addr[dl->offset].nr_samples)
> return 0;
>
> - percent = 100.0 * symhist->addr[dl->offset] / symhist->sum;
> + percent = 100.0 * symhist->addr[dl->offset].nr_samples / symhist->sum;
>
> markup = perf_gtk__get_percent_color(percent);
> if (markup)
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index ef434b5..f7aeb5f 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -610,10 +610,10 @@ int symbol__alloc_hist(struct symbol *sym)
> size_t sizeof_sym_hist;
>
> /* Check for overflow when calculating sizeof_sym_hist */
> - if (size > (SIZE_MAX - sizeof(struct sym_hist)) / sizeof(u64))
> + if (size > (SIZE_MAX - sizeof(struct sym_hist)) / sizeof(struct sym_sample))
> return -1;
>
> - sizeof_sym_hist = (sizeof(struct sym_hist) + size * sizeof(u64));
> + sizeof_sym_hist = (sizeof(struct sym_hist) + size * sizeof(struct sym_sample));
>
> /* Check for overflow in zalloc argument */
> if (sizeof_sym_hist > (SIZE_MAX - sizeof(*notes->src))
> @@ -714,11 +714,11 @@ static int __symbol__inc_addr_samples(struct symbol *sym, struct map *map,
> offset = addr - sym->start;
> h = annotation__histogram(notes, evidx);
> h->sum++;
> - h->addr[offset]++;
> + h->addr[offset].nr_samples++;
>
> pr_debug3("%#" PRIx64 " %s: period++ [addr: %#" PRIx64 ", %#" PRIx64
> ", evidx=%d] => %" PRIu64 "\n", sym->start, sym->name,
> - addr, addr - sym->start, evidx, h->addr[offset]);
> + addr, addr - sym->start, evidx, h->addr[offset].nr_samples);
> return 0;
> }
>
> @@ -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->sum++;
> + 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)
> @@ -928,11 +952,13 @@ struct disasm_line *disasm__get_next_ip_line(struct list_head *head, struct disa
> }
>
> double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset,
> - s64 end, const char **path, u64 *nr_samples)
> + s64 end, const char **path, u64 *nr_samples, u64 *period)
> {
> struct source_line *src_line = notes->src->lines;
> double percent = 0.0;
> +
> *nr_samples = 0;
> + *period = 0;
>
> if (src_line) {
> size_t sizeof_src_line = sizeof(*src_line) +
> @@ -952,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++];
> + while (offset < end) {
> + hits += h->addr[offset].nr_samples;
> + p += h->addr[offset++].period;
> + }
>
> if (h->sum) {
> *nr_samples = hits;
> + *period = p;
> percent = 100.0 * hits / h->sum;
> }
> }
> @@ -1057,10 +1087,11 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
>
> if (dl->offset != -1) {
> const char *path = NULL;
> - u64 nr_samples;
> + u64 nr_samples, period;
> double percent, max_percent = 0.0;
> double *ppercents = &percent;
> u64 *psamples = &nr_samples;
> + u64 *pperiod = &period;
> int i, nr_percent = 1;
> const char *color;
> struct annotation *notes = symbol__annotation(sym);
> @@ -1075,9 +1106,9 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
> nr_percent = evsel->nr_members;
> ppercents = calloc(nr_percent, sizeof(double));
> psamples = calloc(nr_percent, sizeof(u64));
> - if (ppercents == NULL || psamples == NULL) {
> + pperiod = calloc(nr_percent, sizeof(u64));
> + if (ppercents == NULL || psamples == NULL || pperiod == NULL)
> return -1;
> - }
> }
>
> for (i = 0; i < nr_percent; i++) {
> @@ -1085,10 +1116,11 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
> notes->src->lines ? i : evsel->idx + i,
> offset,
> next ? next->offset : (s64) len,
> - &path, &nr_samples);
> + &path, &nr_samples, &period);
>
> ppercents[i] = percent;
> psamples[i] = nr_samples;
> + pperiod[i] = period;
> if (percent > max_percent)
> max_percent = percent;
> }
> @@ -1127,11 +1159,12 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
> for (i = 0; i < nr_percent; i++) {
> percent = ppercents[i];
> nr_samples = psamples[i];
> + period = pperiod[i];
> color = get_percent_color(percent);
>
> if (symbol_conf.show_total_period)
> - color_fprintf(stdout, color, " %7" PRIu64,
> - nr_samples);
> + color_fprintf(stdout, color, " %11" PRIu64,
> + period);
> else
> color_fprintf(stdout, color, " %7.2f", percent);
> }
> @@ -1150,6 +1183,9 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
> if (psamples != &nr_samples)
> free(psamples);
>
> + if (pperiod != &period)
> + free(pperiod);
> +
> } else if (max_lines && printed >= max_lines)
> return 1;
> else {
> @@ -1161,6 +1197,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
> @@ -1702,7 +1742,7 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map,
> double percent = 0.0;
>
> h = annotation__histogram(notes, evidx + k);
> - nr_samples = h->addr[i];
> + nr_samples = h->addr[i].nr_samples;
> if (h->sum)
> percent = 100.0 * nr_samples / h->sum;
>
> @@ -1773,9 +1813,9 @@ static void symbol__annotate_hits(struct symbol *sym, struct perf_evsel *evsel)
> u64 len = symbol__size(sym), offset;
>
> for (offset = 0; offset < len; ++offset)
> - if (h->addr[offset] != 0)
> + if (h->addr[offset].nr_samples != 0)
> printf("%*" PRIx64 ": %" PRIu64 "\n", BITS_PER_LONG / 2,
> - sym->start + offset, h->addr[offset]);
> + sym->start + offset, h->addr[offset].nr_samples);
> printf("%*s: %" PRIu64 "\n", BITS_PER_LONG / 2, "h->sum", h->sum);
> }
>
> @@ -1811,8 +1851,16 @@ 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->sum);
> + 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,
> + symbol_conf.show_total_period ? "Event count" : "Percent",
> + d_filename, evsel_name,
> + symbol_conf.show_total_period ? h->total_period : h->sum,
> + symbol_conf.show_total_period ? "event count" : "samples");
>
> printf("%-*.*s----\n",
> graph_dotted_len, graph_dotted_len, graph_dotted_line);
> @@ -1878,8 +1926,8 @@ void symbol__annotate_decay_histogram(struct symbol *sym, int evidx)
>
> h->sum = 0;
> for (offset = 0; offset < len; ++offset) {
> - h->addr[offset] = h->addr[offset] * 7 / 8;
> - h->sum += h->addr[offset];
> + h->addr[offset].nr_samples = h->addr[offset].nr_samples * 7 / 8;
> + h->sum += h->addr[offset].nr_samples;
> }
> }
>
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index bac698d..6b2e645 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -79,11 +79,16 @@ struct disasm_line *disasm__get_next_ip_line(struct list_head *head, struct disa
> int disasm_line__scnprintf(struct disasm_line *dl, char *bf, size_t size, bool raw);
> size_t disasm__fprintf(struct list_head *head, FILE *fp);
> double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset,
> - s64 end, const char **path, u64 *nr_samples);
> + s64 end, const char **path, u64 *nr_samples, u64 *period);
> +struct sym_sample {
> + u64 nr_samples;
> + u64 period;
> +};
>
> struct sym_hist {
> u64 sum;
> - u64 addr[0];
> + u64 total_period;
> + struct sym_sample addr[0];
> };
>
> struct cyc_hist {
> @@ -155,7 +160,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

2017-07-13 07:23:13

by Taeung Song

Subject: Re: [PATCH 1/4] perf annotate: Fix wrong --show-total-period option showing number of samples

Hi Arnaldo :)

On 07/13/2017 05:09 AM, Arnaldo Carvalho de Melo wrote:
> Em Wed, Jul 12, 2017 at 07:14:04AM +0900, Taeung Song escreveu:
>> 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 two problem like below:
>>
>> 1) Wrong column i.e. 'Percent'
>> 2) Show number of samples, not period
>>
>> So fix this option to show period instead of number of samples.
>
> so you have multiple bugs here, please fix one per patch, i.e. if using
> --show-total-period the header should not be "Percent".

Okey, I'll separate this patch.

>
> Then, you need a patch to introduce that "struct sym_sample" and use it,
> but please rename it to "struct sym_hist_entry".
>

I got it.

> You can do it and just update the sym_hist_entry->period field, before
> the change to pass 'struct sample' around.
>

Understood.

> That disasm__calc_percent() function could receive a pointer to a struct
> sym_hist_entry instead of two pointers.
>

Okey.

> Small, self contained patches that do one thing make reviewing easier,
> by yourself and others.
>

Yep.

> Please give this a try, if you didn't understand something here I can do
> it for you, as this needs doing to fix real bugs.
>
> Thanks,
>
> - Arnaldo
>

I understood what you said! I'll send v2 in accordance with your comment.

Thanks,
Taeung

>> Before:
>>
>> Percent | Source code & Disassembly of old for cycles:ppp (102 samples)
>> -----------------------------------------------------------------------------
>> :
>> :
>> :
>> : Disassembly of section .text:
>> :
>> : 0000000000400816 <knapsack>:
>> : knapsack():
>> 1 : 400816: push %rbp
>>
>> After:
>>
>> Event count | Source code & Disassembly of old for cycles:ppp (84813704 event count)
>> --------------------------------------------------------------------------------------
>> :
>> :
>> :
>> : Disassembly of section .text:
>> :
>> : 0000000000400816 <knapsack>:
>> : knapsack():
>> 743737 : 400816: push %rbp
>>
>> Cc: Namhyung Kim <[email protected]>
>> Cc: Milian Wolff <[email protected]>
>> Cc: Jiri Olsa <[email protected]>
>> Signed-off-by: Taeung Song <[email protected]>
>> ---
>> tools/perf/builtin-annotate.c | 4 +-
>> tools/perf/builtin-report.c | 13 +++---
>> tools/perf/builtin-top.c | 6 ++-
>> tools/perf/ui/browsers/annotate.c | 4 +-
>> tools/perf/ui/gtk/annotate.c | 4 +-
>> tools/perf/util/annotate.c | 92 +++++++++++++++++++++++++++++----------
>> tools/perf/util/annotate.h | 12 +++--
>> 7 files changed, 96 insertions(+), 39 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;
>> -
>
> Please do not remove this line.
>
>> 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);
>
> why break this into two lines?
>
>>
>> } 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/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
>> index b376637..73e5ae2 100644
>> --- a/tools/perf/ui/browsers/annotate.c
>> +++ b/tools/perf/ui/browsers/annotate.c
>> @@ -449,13 +449,13 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser,
>> next = disasm__get_next_ip_line(&notes->src->source, pos);
>>
>> for (i = 0; i < browser->nr_events; i++) {
>> - u64 nr_samples;
>> + u64 nr_samples, period;
>>
>> bpos->samples[i].percent = disasm__calc_percent(notes,
>> evsel->idx + i,
>> pos->offset,
>> next ? next->offset : len,
>> - &path, &nr_samples);
>> + &path, &nr_samples, &period);
>> bpos->samples[i].nr = nr_samples;
>>
>> if (max_percent < bpos->samples[i].percent)
>> diff --git a/tools/perf/ui/gtk/annotate.c b/tools/perf/ui/gtk/annotate.c
>> index 87e3760..d736fd5 100644
>> --- a/tools/perf/ui/gtk/annotate.c
>> +++ b/tools/perf/ui/gtk/annotate.c
>> @@ -34,10 +34,10 @@ static int perf_gtk__get_percent(char *buf, size_t size, struct symbol *sym,
>> return 0;
>>
>> symhist = annotation__histogram(symbol__annotation(sym), evidx);
>> - if (!symbol_conf.event_group && !symhist->addr[dl->offset])
>> + if (!symbol_conf.event_group && !symhist->addr[dl->offset].nr_samples)
>> return 0;
>>
>> - percent = 100.0 * symhist->addr[dl->offset] / symhist->sum;
>> + percent = 100.0 * symhist->addr[dl->offset].nr_samples / symhist->sum;
>>
>> markup = perf_gtk__get_percent_color(percent);
>> if (markup)
>> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
>> index ef434b5..f7aeb5f 100644
>> --- a/tools/perf/util/annotate.c
>> +++ b/tools/perf/util/annotate.c
>> @@ -610,10 +610,10 @@ int symbol__alloc_hist(struct symbol *sym)
>> size_t sizeof_sym_hist;
>>
>> /* Check for overflow when calculating sizeof_sym_hist */
>> - if (size > (SIZE_MAX - sizeof(struct sym_hist)) / sizeof(u64))
>> + if (size > (SIZE_MAX - sizeof(struct sym_hist)) / sizeof(struct sym_sample))
>> return -1;
>>
>> - sizeof_sym_hist = (sizeof(struct sym_hist) + size * sizeof(u64));
>> + sizeof_sym_hist = (sizeof(struct sym_hist) + size * sizeof(struct sym_sample));
>>
>> /* Check for overflow in zalloc argument */
>> if (sizeof_sym_hist > (SIZE_MAX - sizeof(*notes->src))
>> @@ -714,11 +714,11 @@ static int __symbol__inc_addr_samples(struct symbol *sym, struct map *map,
>> offset = addr - sym->start;
>> h = annotation__histogram(notes, evidx);
>> h->sum++;
>> - h->addr[offset]++;
>> + h->addr[offset].nr_samples++;
>>
>> pr_debug3("%#" PRIx64 " %s: period++ [addr: %#" PRIx64 ", %#" PRIx64
>> ", evidx=%d] => %" PRIu64 "\n", sym->start, sym->name,
>> - addr, addr - sym->start, evidx, h->addr[offset]);
>> + addr, addr - sym->start, evidx, h->addr[offset].nr_samples);
>> return 0;
>> }
>>
>> @@ -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->sum++;
>> + 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)
>> @@ -928,11 +952,13 @@ struct disasm_line *disasm__get_next_ip_line(struct list_head *head, struct disa
>> }
>>
>> double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset,
>> - s64 end, const char **path, u64 *nr_samples)
>> + s64 end, const char **path, u64 *nr_samples, u64 *period)
>> {
>> struct source_line *src_line = notes->src->lines;
>> double percent = 0.0;
>> +
>> *nr_samples = 0;
>> + *period = 0;
>>
>> if (src_line) {
>> size_t sizeof_src_line = sizeof(*src_line) +
>> @@ -952,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++];
>> + while (offset < end) {
>> + hits += h->addr[offset].nr_samples;
>> + p += h->addr[offset++].period;
>> + }
>>
>> if (h->sum) {
>> *nr_samples = hits;
>> + *period = p;
>> percent = 100.0 * hits / h->sum;
>> }
>> }
>> @@ -1057,10 +1087,11 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
>>
>> if (dl->offset != -1) {
>> const char *path = NULL;
>> - u64 nr_samples;
>> + u64 nr_samples, period;
>> double percent, max_percent = 0.0;
>> double *ppercents = &percent;
>> u64 *psamples = &nr_samples;
>> + u64 *pperiod = &period;
>> int i, nr_percent = 1;
>> const char *color;
>> struct annotation *notes = symbol__annotation(sym);
>> @@ -1075,9 +1106,9 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
>> nr_percent = evsel->nr_members;
>> ppercents = calloc(nr_percent, sizeof(double));
>> psamples = calloc(nr_percent, sizeof(u64));
>> - if (ppercents == NULL || psamples == NULL) {
>> + pperiod = calloc(nr_percent, sizeof(u64));
>> + if (ppercents == NULL || psamples == NULL || pperiod == NULL)
>> return -1;
>> - }
>> }
>>
>> for (i = 0; i < nr_percent; i++) {
>> @@ -1085,10 +1116,11 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
>> notes->src->lines ? i : evsel->idx + i,
>> offset,
>> next ? next->offset : (s64) len,
>> - &path, &nr_samples);
>> + &path, &nr_samples, &period);
>>
>> ppercents[i] = percent;
>> psamples[i] = nr_samples;
>> + pperiod[i] = period;
>> if (percent > max_percent)
>> max_percent = percent;
>> }
>> @@ -1127,11 +1159,12 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
>> for (i = 0; i < nr_percent; i++) {
>> percent = ppercents[i];
>> nr_samples = psamples[i];
>> + period = pperiod[i];
>> color = get_percent_color(percent);
>>
>> if (symbol_conf.show_total_period)
>> - color_fprintf(stdout, color, " %7" PRIu64,
>> - nr_samples);
>> + color_fprintf(stdout, color, " %11" PRIu64,
>> + period);
>> else
>> color_fprintf(stdout, color, " %7.2f", percent);
>> }
>> @@ -1150,6 +1183,9 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
>> if (psamples != &nr_samples)
>> free(psamples);
>>
>> + if (pperiod != &period)
>> + free(pperiod);
>> +
>> } else if (max_lines && printed >= max_lines)
>> return 1;
>> else {
>> @@ -1161,6 +1197,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
>> @@ -1702,7 +1742,7 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map,
>> double percent = 0.0;
>>
>> h = annotation__histogram(notes, evidx + k);
>> - nr_samples = h->addr[i];
>> + nr_samples = h->addr[i].nr_samples;
>> if (h->sum)
>> percent = 100.0 * nr_samples / h->sum;
>>
>> @@ -1773,9 +1813,9 @@ static void symbol__annotate_hits(struct symbol *sym, struct perf_evsel *evsel)
>> u64 len = symbol__size(sym), offset;
>>
>> for (offset = 0; offset < len; ++offset)
>> - if (h->addr[offset] != 0)
>> + if (h->addr[offset].nr_samples != 0)
>> printf("%*" PRIx64 ": %" PRIu64 "\n", BITS_PER_LONG / 2,
>> - sym->start + offset, h->addr[offset]);
>> + sym->start + offset, h->addr[offset].nr_samples);
>> printf("%*s: %" PRIu64 "\n", BITS_PER_LONG / 2, "h->sum", h->sum);
>> }
>>
>> @@ -1811,8 +1851,16 @@ 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->sum);
>> + 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,
>> + symbol_conf.show_total_period ? "Event count" : "Percent",
>> + d_filename, evsel_name,
>> + symbol_conf.show_total_period ? h->total_period : h->sum,
>> + symbol_conf.show_total_period ? "event count" : "samples");
>>
>> printf("%-*.*s----\n",
>> graph_dotted_len, graph_dotted_len, graph_dotted_line);
>> @@ -1878,8 +1926,8 @@ void symbol__annotate_decay_histogram(struct symbol *sym, int evidx)
>>
>> h->sum = 0;
>> for (offset = 0; offset < len; ++offset) {
>> - h->addr[offset] = h->addr[offset] * 7 / 8;
>> - h->sum += h->addr[offset];
>> + h->addr[offset].nr_samples = h->addr[offset].nr_samples * 7 / 8;
>> + h->sum += h->addr[offset].nr_samples;
>> }
>> }
>>
>> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
>> index bac698d..6b2e645 100644
>> --- a/tools/perf/util/annotate.h
>> +++ b/tools/perf/util/annotate.h
>> @@ -79,11 +79,16 @@ struct disasm_line *disasm__get_next_ip_line(struct list_head *head, struct disa
>> int disasm_line__scnprintf(struct disasm_line *dl, char *bf, size_t size, bool raw);
>> size_t disasm__fprintf(struct list_head *head, FILE *fp);
>> double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset,
>> - s64 end, const char **path, u64 *nr_samples);
>> + s64 end, const char **path, u64 *nr_samples, u64 *period);
>> +struct sym_sample {
>> + u64 nr_samples;
>> + u64 period;
>> +};
>>
>> struct sym_hist {
>> u64 sum;
>> - u64 addr[0];
>> + u64 total_period;
>> + struct sym_sample addr[0];
>> };
>>
>> struct cyc_hist {
>> @@ -155,7 +160,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