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

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

[Taeung Song]

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 <namhyung@kernel.org>
Cc: Milian Wolff <milian.wolff@kdab.com>
Cc: Jiri Olsa <jolsa@redhat.com>
Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
---
 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

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

[Arnaldo Carvalho de Melo]

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 <namhyung@kernel.org>
> Cc: Milian Wolff <milian.wolff@kdab.com>
> Cc: Jiri Olsa <jolsa@redhat.com>
> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
> ---
>  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

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

[Taeung Song]

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 <namhyung@kernel.org>
>> Cc: Milian Wolff <milian.wolff@kdab.com>
>> Cc: Jiri Olsa <jolsa@redhat.com>
>> Signed-off-by: Taeung Song <treeze.taeung@gmail.com>
>> ---
>>   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