All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] perf annotate: With --show-total-period, display total # of samples.
@ 2015-06-18 16:10 Martin Liška
  2015-06-18 16:26 ` Ingo Molnar
  0 siblings, 1 reply; 9+ messages in thread
From: Martin Liška @ 2015-06-18 16:10 UTC (permalink / raw)
  To: linux-kernel
  Cc: Arnaldo Carvalho de Melo, Ingo Molnar, Peter Zijlstra,
	Paul Mackerras, Andi Kleen

To compare two records on an instruction base, with --show-total-period
option provided, display total number of samples that belong to a line
in assembly language.

Signed-off-by: Martin Liska <mliska@suse.cz>
---
 tools/perf/builtin-annotate.c     |  2 ++
 tools/perf/ui/browsers/annotate.c | 44 +++++++++++++++++++++++++--------------
 tools/perf/util/annotate.c        | 28 +++++++++++++++++++------
 tools/perf/util/annotate.h        |  3 ++-
 4 files changed, 54 insertions(+), 23 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 4e08c2d..2c1bec3 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -329,6 +329,8 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused)
 		   "objdump binary to use for disassembly and annotations"),
 	OPT_BOOLEAN(0, "group", &symbol_conf.event_group,
 		    "Show event group information together"),
+	OPT_BOOLEAN(0, "show-total-period", &symbol_conf.show_total_period,
+		    "Show a column with the sum of periods"),
 	OPT_END()
 	};
 	int ret = hists__init();
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index acb0e23..89dd816 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -11,16 +11,21 @@
 #include "../../util/evsel.h"
 #include <pthread.h>
 
+struct disasm_tuple {
+	double		percent;
+	double		samples;
+};
+
 struct browser_disasm_line {
-	struct rb_node	rb_node;
-	u32		idx;
-	int		idx_asm;
-	int		jump_sources;
+	struct rb_node		rb_node;
+	u32			idx;
+	int			idx_asm;
+	int			jump_sources;
 	/*
 	 * actual length of this array is saved on the nr_events field
 	 * of the struct annotate_browser
 	 */
-	double		percent[1];
+	struct disasm_tuple    tuples[1];
 };
 
 static struct annotate_browser_opt {
@@ -105,15 +110,19 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
 	char bf[256];
 
 	for (i = 0; i < ab->nr_events; i++) {
-		if (bdl->percent[i] > percent_max)
-			percent_max = bdl->percent[i];
+		if (bdl->tuples[i].percent > percent_max)
+			percent_max = bdl->tuples[i].percent;
 	}
 
 	if (dl->offset != -1 && percent_max != 0.0) {
 		for (i = 0; i < ab->nr_events; i++) {
-			ui_browser__set_percent_color(browser, bdl->percent[i],
+			ui_browser__set_percent_color(browser,
+						      bdl->tuples[i].percent,
 						      current_entry);
-			slsmg_printf("%6.2f ", bdl->percent[i]);
+			if (symbol_conf.show_total_period)
+				slsmg_printf("%6.0f ", bdl->tuples[i].samples);
+			else
+				slsmg_printf("%6.2f ", bdl->tuples[i].percent);
 		}
 	} else {
 		ui_browser__set_percent_color(browser, 0, current_entry);
@@ -273,9 +282,9 @@ static int disasm__cmp(struct browser_disasm_line *a,
 	int i;
 
 	for (i = 0; i < nr_pcnt; i++) {
-		if (a->percent[i] == b->percent[i])
+		if (a->tuples[i].percent == b->tuples[i].percent)
 			continue;
-		return a->percent[i] < b->percent[i];
+		return a->tuples[i].percent < b->tuples[i].percent;
 	}
 	return 0;
 }
@@ -366,14 +375,17 @@ 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++) {
-			bpos->percent[i] = disasm__calc_percent(notes,
+			double samples;
+
+			bpos->tuples[i].percent = disasm__calc_percent(notes,
 						evsel->idx + i,
 						pos->offset,
 						next ? next->offset : len,
-					        &path);
+						&path, &samples);
+			bpos->tuples[i].samples = samples;
 
-			if (max_percent < bpos->percent[i])
-				max_percent = bpos->percent[i];
+			if (max_percent < bpos->tuples[i].percent)
+				max_percent = bpos->tuples[i].percent;
 		}
 
 		if (max_percent < 0.01) {
@@ -929,7 +941,7 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map,
 
 	if (perf_evsel__is_group_event(evsel)) {
 		nr_pcnt = evsel->nr_members;
-		sizeof_bdl += sizeof(double) * (nr_pcnt - 1);
+		sizeof_bdl += sizeof(struct disasm_tuple) * (nr_pcnt - 1);
 	}
 
 	if (symbol__annotate(sym, map, sizeof_bdl) < 0) {
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index bf80430..8b94603 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -654,10 +654,11 @@ 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)
+			    s64 end, const char **path, double *samples)
 {
 	struct source_line *src_line = notes->src->lines;
 	double percent = 0.0;
+	*samples = 0.0;
 
 	if (src_line) {
 		size_t sizeof_src_line = sizeof(*src_line) +
@@ -671,6 +672,7 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset,
 				*path = src_line->path;
 
 			percent += src_line->p[evidx].percent;
+			*samples += src_line->p[evidx].samples;
 			offset++;
 		}
 	} else {
@@ -680,8 +682,10 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset,
 		while (offset < end)
 			hits += h->addr[offset++];
 
-		if (h->sum)
+		if (h->sum) {
+			*samples = hits;
 			percent = 100.0 * hits / h->sum;
+		}
 	}
 
 	return percent;
@@ -696,8 +700,9 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
 
 	if (dl->offset != -1) {
 		const char *path = NULL;
-		double percent, max_percent = 0.0;
+		double percent, samples, max_percent = 0.0;
 		double *ppercents = &percent;
+		double *psamples = &samples;
 		int i, nr_percent = 1;
 		const char *color;
 		struct annotation *notes = symbol__annotation(sym);
@@ -710,8 +715,10 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
 		if (perf_evsel__is_group_event(evsel)) {
 			nr_percent = evsel->nr_members;
 			ppercents = calloc(nr_percent, sizeof(double));
-			if (ppercents == NULL)
+			psamples = calloc(nr_percent, sizeof(double));
+			if (ppercents == NULL || psamples == NULL) {
 				return -1;
+			}
 		}
 
 		for (i = 0; i < nr_percent; i++) {
@@ -719,9 +726,10 @@ 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);
+					&path, &samples);
 
 			ppercents[i] = percent;
+			psamples[i] = samples;
 			if (percent > max_percent)
 				max_percent = percent;
 		}
@@ -759,8 +767,13 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
 
 		for (i = 0; i < nr_percent; i++) {
 			percent = ppercents[i];
+			samples = psamples[i];
 			color = get_percent_color(percent);
-			color_fprintf(stdout, color, " %7.2f", percent);
+
+			if (symbol_conf.show_total_period)
+				color_fprintf(stdout, color, " %7.0f", samples);
+			else
+				color_fprintf(stdout, color, " %7.2f", percent);
 		}
 
 		printf(" :	");
@@ -770,6 +783,9 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
 		if (ppercents != &percent)
 			free(ppercents);
 
+		if (psamples != &samples)
+			free(psamples);
+
 	} else if (max_lines && printed >= max_lines)
 		return 1;
 	else {
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index cadbdc9..752d1bd 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -72,7 +72,7 @@ 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);
+			    s64 end, const char **path, double *samples);
 
 struct sym_hist {
 	u64		sum;
@@ -82,6 +82,7 @@ struct sym_hist {
 struct source_line_percent {
 	double		percent;
 	double		percent_sum;
+	double          samples;
 };
 
 struct source_line {
-- 
2.1.4


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] perf annotate: With --show-total-period, display total # of samples.
  2015-06-18 16:10 [PATCH] perf annotate: With --show-total-period, display total # of samples Martin Liška
@ 2015-06-18 16:26 ` Ingo Molnar
  2015-06-18 18:06   ` Martin Liška
  0 siblings, 1 reply; 9+ messages in thread
From: Ingo Molnar @ 2015-06-18 16:26 UTC (permalink / raw)
  To: Martin Liška
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Ingo Molnar,
	Peter Zijlstra, Paul Mackerras, Andi Kleen


* Martin Liška <mliska@suse.cz> wrote:

> To compare two records on an instruction base, with --show-total-period
> option provided, display total number of samples that belong to a line
> in assembly language.
> 
> Signed-off-by: Martin Liska <mliska@suse.cz>
> ---
>  tools/perf/builtin-annotate.c     |  2 ++
>  tools/perf/ui/browsers/annotate.c | 44 +++++++++++++++++++++++++--------------
>  tools/perf/util/annotate.c        | 28 +++++++++++++++++++------
>  tools/perf/util/annotate.h        |  3 ++-
>  4 files changed, 54 insertions(+), 23 deletions(-)
> 
> diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
> index 4e08c2d..2c1bec3 100644
> --- a/tools/perf/builtin-annotate.c
> +++ b/tools/perf/builtin-annotate.c
> @@ -329,6 +329,8 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused)
>  		   "objdump binary to use for disassembly and annotations"),
>  	OPT_BOOLEAN(0, "group", &symbol_conf.event_group,
>  		    "Show event group information together"),
> +	OPT_BOOLEAN(0, "show-total-period", &symbol_conf.show_total_period,
> +		    "Show a column with the sum of periods"),
>  	OPT_END()

What is the toggle for this in the 'perf report' TUI? (which displays annotated 
output too)

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] perf annotate: With --show-total-period, display total # of samples.
  2015-06-18 16:26 ` Ingo Molnar
@ 2015-06-18 18:06   ` Martin Liška
  2015-06-18 19:15     ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 9+ messages in thread
From: Martin Liška @ 2015-06-18 18:06 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Arnaldo Carvalho de Melo, Ingo Molnar,
	Peter Zijlstra, Paul Mackerras, Andi Kleen

[-- Attachment #1: Type: text/plain, Size: 1449 bytes --]

On 06/18/2015 06:26 PM, Ingo Molnar wrote:
> 
> * Martin Liška <mliska@suse.cz> wrote:
> 
>> To compare two records on an instruction base, with --show-total-period
>> option provided, display total number of samples that belong to a line
>> in assembly language.
>>
>> Signed-off-by: Martin Liska <mliska@suse.cz>
>> ---
>>  tools/perf/builtin-annotate.c     |  2 ++
>>  tools/perf/ui/browsers/annotate.c | 44 +++++++++++++++++++++++++--------------
>>  tools/perf/util/annotate.c        | 28 +++++++++++++++++++------
>>  tools/perf/util/annotate.h        |  3 ++-
>>  4 files changed, 54 insertions(+), 23 deletions(-)
>>
>> diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
>> index 4e08c2d..2c1bec3 100644
>> --- a/tools/perf/builtin-annotate.c
>> +++ b/tools/perf/builtin-annotate.c
>> @@ -329,6 +329,8 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused)
>>  		   "objdump binary to use for disassembly and annotations"),
>>  	OPT_BOOLEAN(0, "group", &symbol_conf.event_group,
>>  		    "Show event group information together"),
>> +	OPT_BOOLEAN(0, "show-total-period", &symbol_conf.show_total_period,
>> +		    "Show a column with the sum of periods"),
>>  	OPT_END()
> 
> What is the toggle for this in the 'perf report' TUI? (which displays annotated 
> output too)
> 
> Thanks,
> 
> 	Ingo
> 

Hello.

Thanks for note, I fixed that by introducing a new 't' hot key.

Thanks,
Martin

[-- Attachment #2: 0001-perf-annotate-With-show-total-period-display-total-o-v2.patch --]
[-- Type: text/x-patch, Size: 11014 bytes --]

>From 98e1998fcd7481c7e1756e643d66eb85559849ac Mon Sep 17 00:00:00 2001
From: mliska <mliska@suse.cz>
Date: Wed, 27 May 2015 10:54:42 +0200
Subject: [PATCH] perf annotate: With --show-total-period, display total # of
 samples.

To compare two records on an instruction base, with --show-total-period
option provided, display total number of samples that belong to a line
in assembly language.

New hot key 't' is introduced for 'perf annotate' TUI.

Signed-off-by: Martin Liska <mliska@suse.cz>
---
 tools/perf/builtin-annotate.c     |  5 +++-
 tools/perf/ui/browsers/annotate.c | 60 +++++++++++++++++++++++++++------------
 tools/perf/util/annotate.c        | 28 ++++++++++++++----
 tools/perf/util/annotate.h        |  3 +-
 tools/perf/util/hist.h            |  3 +-
 5 files changed, 72 insertions(+), 27 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 4e08c2d..b0c442e 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -161,7 +161,8 @@ find_next:
 			/* skip missing symbols */
 			nd = rb_next(nd);
 		} else if (use_browser == 1) {
-			key = hist_entry__tui_annotate(he, evsel, NULL);
+			key = hist_entry__tui_annotate(he, evsel, NULL,
+				symbol_conf.show_total_period);
 			switch (key) {
 			case -1:
 				if (!ann->skip_missing)
@@ -329,6 +330,8 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused)
 		   "objdump binary to use for disassembly and annotations"),
 	OPT_BOOLEAN(0, "group", &symbol_conf.event_group,
 		    "Show event group information together"),
+	OPT_BOOLEAN(0, "show-total-period", &symbol_conf.show_total_period,
+		    "Show a column with the sum of periods"),
 	OPT_END()
 	};
 	int ret = hists__init();
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index acb0e23..e7cd596 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -11,16 +11,21 @@
 #include "../../util/evsel.h"
 #include <pthread.h>
 
+struct disasm_tuple {
+	double		percent;
+	double		samples;
+};
+
 struct browser_disasm_line {
-	struct rb_node	rb_node;
-	u32		idx;
-	int		idx_asm;
-	int		jump_sources;
+	struct rb_node		rb_node;
+	u32			idx;
+	int			idx_asm;
+	int			jump_sources;
 	/*
 	 * actual length of this array is saved on the nr_events field
 	 * of the struct annotate_browser
 	 */
-	double		percent[1];
+	struct disasm_tuple    tuples[1];
 };
 
 static struct annotate_browser_opt {
@@ -28,7 +33,8 @@ static struct annotate_browser_opt {
 	     use_offset,
 	     jump_arrows,
 	     show_linenr,
-	     show_nr_jumps;
+	     show_nr_jumps,
+	     show_total_period;
 } annotate_browser__opts = {
 	.use_offset	= true,
 	.jump_arrows	= true,
@@ -105,15 +111,19 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
 	char bf[256];
 
 	for (i = 0; i < ab->nr_events; i++) {
-		if (bdl->percent[i] > percent_max)
-			percent_max = bdl->percent[i];
+		if (bdl->tuples[i].percent > percent_max)
+			percent_max = bdl->tuples[i].percent;
 	}
 
 	if (dl->offset != -1 && percent_max != 0.0) {
 		for (i = 0; i < ab->nr_events; i++) {
-			ui_browser__set_percent_color(browser, bdl->percent[i],
+			ui_browser__set_percent_color(browser,
+						      bdl->tuples[i].percent,
 						      current_entry);
-			slsmg_printf("%6.2f ", bdl->percent[i]);
+			if (annotate_browser__opts.show_total_period)
+				slsmg_printf("%6.0f ", bdl->tuples[i].samples);
+			else
+				slsmg_printf("%6.2f ", bdl->tuples[i].percent);
 		}
 	} else {
 		ui_browser__set_percent_color(browser, 0, current_entry);
@@ -273,9 +283,9 @@ static int disasm__cmp(struct browser_disasm_line *a,
 	int i;
 
 	for (i = 0; i < nr_pcnt; i++) {
-		if (a->percent[i] == b->percent[i])
+		if (a->tuples[i].percent == b->tuples[i].percent)
 			continue;
-		return a->percent[i] < b->percent[i];
+		return a->tuples[i].percent < b->tuples[i].percent;
 	}
 	return 0;
 }
@@ -366,14 +376,17 @@ 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++) {
-			bpos->percent[i] = disasm__calc_percent(notes,
+			double samples;
+
+			bpos->tuples[i].percent = disasm__calc_percent(notes,
 						evsel->idx + i,
 						pos->offset,
 						next ? next->offset : len,
-					        &path);
+						&path, &samples);
+			bpos->tuples[i].samples = samples;
 
-			if (max_percent < bpos->percent[i])
-				max_percent = bpos->percent[i];
+			if (max_percent < bpos->tuples[i].percent)
+				max_percent = bpos->tuples[i].percent;
 		}
 
 		if (max_percent < 0.01) {
@@ -737,6 +750,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
 		"n             Search next string\n"
 		"o             Toggle disassembler output/simplified view\n"
 		"s             Toggle source code view\n"
+		"t             Toggle total period view\n"
 		"/             Search string\n"
 		"k             Toggle line numbers\n"
 		"r             Run available scripts\n"
@@ -812,6 +826,11 @@ show_sup_ins:
 				ui_helpline__puts("Actions are only available for 'callq', 'retq' & jump instructions.");
 			}
 			continue;
+		case 't':
+			annotate_browser__opts.show_total_period =
+			  !annotate_browser__opts.show_total_period;
+			annotate_browser__update_addr_width(browser);
+			continue;
 		case K_LEFT:
 		case K_ESC:
 		case 'q':
@@ -836,12 +855,16 @@ int map_symbol__tui_annotate(struct map_symbol *ms, struct perf_evsel *evsel,
 }
 
 int hist_entry__tui_annotate(struct hist_entry *he, struct perf_evsel *evsel,
-			     struct hist_browser_timer *hbt)
+			     struct hist_browser_timer *hbt,
+			     bool show_total_period)
 {
 	/* reset abort key so that it can get Ctrl-C as a key */
 	SLang_reset_tty();
 	SLang_init_tty(0, 0, 0);
 
+	/* Set default value for show_total_period.  */
+	annotate_browser__opts.show_total_period = show_total_period;
+
 	return map_symbol__tui_annotate(&he->ms, evsel, hbt);
 }
 
@@ -929,7 +952,7 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map,
 
 	if (perf_evsel__is_group_event(evsel)) {
 		nr_pcnt = evsel->nr_members;
-		sizeof_bdl += sizeof(double) * (nr_pcnt - 1);
+		sizeof_bdl += sizeof(struct disasm_tuple) * (nr_pcnt - 1);
 	}
 
 	if (symbol__annotate(sym, map, sizeof_bdl) < 0) {
@@ -1006,6 +1029,7 @@ static struct annotate_config {
 	ANNOTATE_CFG(show_linenr),
 	ANNOTATE_CFG(show_nr_jumps),
 	ANNOTATE_CFG(use_offset),
+	ANNOTATE_CFG(show_total_period),
 };
 
 #undef ANNOTATE_CFG
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index bf80430..8b94603 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -654,10 +654,11 @@ 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)
+			    s64 end, const char **path, double *samples)
 {
 	struct source_line *src_line = notes->src->lines;
 	double percent = 0.0;
+	*samples = 0.0;
 
 	if (src_line) {
 		size_t sizeof_src_line = sizeof(*src_line) +
@@ -671,6 +672,7 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset,
 				*path = src_line->path;
 
 			percent += src_line->p[evidx].percent;
+			*samples += src_line->p[evidx].samples;
 			offset++;
 		}
 	} else {
@@ -680,8 +682,10 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset,
 		while (offset < end)
 			hits += h->addr[offset++];
 
-		if (h->sum)
+		if (h->sum) {
+			*samples = hits;
 			percent = 100.0 * hits / h->sum;
+		}
 	}
 
 	return percent;
@@ -696,8 +700,9 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
 
 	if (dl->offset != -1) {
 		const char *path = NULL;
-		double percent, max_percent = 0.0;
+		double percent, samples, max_percent = 0.0;
 		double *ppercents = &percent;
+		double *psamples = &samples;
 		int i, nr_percent = 1;
 		const char *color;
 		struct annotation *notes = symbol__annotation(sym);
@@ -710,8 +715,10 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
 		if (perf_evsel__is_group_event(evsel)) {
 			nr_percent = evsel->nr_members;
 			ppercents = calloc(nr_percent, sizeof(double));
-			if (ppercents == NULL)
+			psamples = calloc(nr_percent, sizeof(double));
+			if (ppercents == NULL || psamples == NULL) {
 				return -1;
+			}
 		}
 
 		for (i = 0; i < nr_percent; i++) {
@@ -719,9 +726,10 @@ 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);
+					&path, &samples);
 
 			ppercents[i] = percent;
+			psamples[i] = samples;
 			if (percent > max_percent)
 				max_percent = percent;
 		}
@@ -759,8 +767,13 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
 
 		for (i = 0; i < nr_percent; i++) {
 			percent = ppercents[i];
+			samples = psamples[i];
 			color = get_percent_color(percent);
-			color_fprintf(stdout, color, " %7.2f", percent);
+
+			if (symbol_conf.show_total_period)
+				color_fprintf(stdout, color, " %7.0f", samples);
+			else
+				color_fprintf(stdout, color, " %7.2f", percent);
 		}
 
 		printf(" :	");
@@ -770,6 +783,9 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
 		if (ppercents != &percent)
 			free(ppercents);
 
+		if (psamples != &samples)
+			free(psamples);
+
 	} else if (max_lines && printed >= max_lines)
 		return 1;
 	else {
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index cadbdc9..752d1bd 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -72,7 +72,7 @@ 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);
+			    s64 end, const char **path, double *samples);
 
 struct sym_hist {
 	u64		sum;
@@ -82,6 +82,7 @@ struct sym_hist {
 struct source_line_percent {
 	double		percent;
 	double		percent_sum;
+	double          samples;
 };
 
 struct source_line {
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 5ed8d9c..65f953f 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -306,7 +306,8 @@ int map_symbol__tui_annotate(struct map_symbol *ms, struct perf_evsel *evsel,
 			     struct hist_browser_timer *hbt);
 
 int hist_entry__tui_annotate(struct hist_entry *he, struct perf_evsel *evsel,
-			     struct hist_browser_timer *hbt);
+			     struct hist_browser_timer *hbt,
+			     bool show_total_period);
 
 int perf_evlist__tui_browse_hists(struct perf_evlist *evlist, const char *help,
 				  struct hist_browser_timer *hbt,
-- 
2.1.4


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] perf annotate: With --show-total-period, display total # of samples.
  2015-06-18 18:06   ` Martin Liška
@ 2015-06-18 19:15     ` Arnaldo Carvalho de Melo
  2015-06-19  9:35       ` Martin Liška
  0 siblings, 1 reply; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-06-18 19:15 UTC (permalink / raw)
  To: Martin Liška
  Cc: Ingo Molnar, linux-kernel, Ingo Molnar, Peter Zijlstra,
	Paul Mackerras, Andi Kleen

Em Thu, Jun 18, 2015 at 08:06:25PM +0200, Martin Liška escreveu:
> On 06/18/2015 06:26 PM, Ingo Molnar wrote:
> > * Martin Liška <mliska@suse.cz> wrote:
> >> +++ b/tools/perf/builtin-annotate.c
> >> +	OPT_BOOLEAN(0, "show-total-period", &symbol_conf.show_total_period,
> >> +		    "Show a column with the sum of periods"),
> >>  	OPT_END()

> > What is the toggle for this in the 'perf report' TUI? (which displays annotated 
> > output too)

> Thanks for note, I fixed that by introducing a new 't' hot key.

> >From 98e1998fcd7481c7e1756e643d66eb85559849ac Mon Sep 17 00:00:00 2001
> From: mliska <mliska@suse.cz>
> Date: Wed, 27 May 2015 10:54:42 +0200
> Subject: [PATCH] perf annotate: With --show-total-period, display total # of
>  samples.
> 
> To compare two records on an instruction base, with --show-total-period
> option provided, display total number of samples that belong to a line
> in assembly language.
> 
> New hot key 't' is introduced for 'perf annotate' TUI.

> Signed-off-by: Martin Liska <mliska@suse.cz>
> ---
>  tools/perf/builtin-annotate.c     |  5 +++-
>  tools/perf/ui/browsers/annotate.c | 60 +++++++++++++++++++++++++++------------
>  tools/perf/util/annotate.c        | 28 ++++++++++++++----
>  tools/perf/util/annotate.h        |  3 +-
>  tools/perf/util/hist.h            |  3 +-
>  5 files changed, 72 insertions(+), 27 deletions(-)
> 
> diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
> index 4e08c2d..b0c442e 100644
> --- a/tools/perf/builtin-annotate.c
> +++ b/tools/perf/builtin-annotate.c
> @@ -161,7 +161,8 @@ find_next:
>  			/* skip missing symbols */
>  			nd = rb_next(nd);
>  		} else if (use_browser == 1) {
> -			key = hist_entry__tui_annotate(he, evsel, NULL);
> +			key = hist_entry__tui_annotate(he, evsel, NULL,
> +				symbol_conf.show_total_period);

No need to pass this global variable around, since we already have it
like that, why not simply read it in hist_entry__tui_annotate()?

>  			switch (key) {
>  			case -1:
>  				if (!ann->skip_missing)
> @@ -329,6 +330,8 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused)
>  		   "objdump binary to use for disassembly and annotations"),
>  	OPT_BOOLEAN(0, "group", &symbol_conf.event_group,
>  		    "Show event group information together"),
> +	OPT_BOOLEAN(0, "show-total-period", &symbol_conf.show_total_period,
> +		    "Show a column with the sum of periods"),
>  	OPT_END()
>  	};
>  	int ret = hists__init();
> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
> index acb0e23..e7cd596 100644
> --- a/tools/perf/ui/browsers/annotate.c
> +++ b/tools/perf/ui/browsers/annotate.c
> @@ -11,16 +11,21 @@
>  #include "../../util/evsel.h"
>  #include <pthread.h>
>  
> +struct disasm_tuple {
> +	double		percent;
> +	double		samples;

Why use double for 'samples'? We use u64 elsewhere for this.

And 'disasm_tuple' is way too vague, how about:

struct disasm_line_samples {
	double		percent;
	u64		nr;
};

and then, below...

> +};
> +
>  struct browser_disasm_line {
> -	struct rb_node	rb_node;
> -	u32		idx;
> -	int		idx_asm;
> -	int		jump_sources;
> +	struct rb_node		rb_node;
> +	u32			idx;
> +	int			idx_asm;
> +	int			jump_sources;
>  	/*
>  	 * actual length of this array is saved on the nr_events field
>  	 * of the struct annotate_browser
>  	 */
> -	double		percent[1];
> +	struct disasm_tuple    tuples[1];

here have:

	struct disasm_line_samples samples;o

Then use bdl->samples[i].nr and bdl->samples[i].percent.

>  };
>  
>  static struct annotate_browser_opt {
> @@ -28,7 +33,8 @@ static struct annotate_browser_opt {
>  	     use_offset,
>  	     jump_arrows,
>  	     show_linenr,
> -	     show_nr_jumps;
> +	     show_nr_jumps,
> +	     show_total_period;
>  } annotate_browser__opts = {
>  	.use_offset	= true,
>  	.jump_arrows	= true,
> @@ -105,15 +111,19 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
>  	char bf[256];
>  
>  	for (i = 0; i < ab->nr_events; i++) {
> -		if (bdl->percent[i] > percent_max)
> -			percent_max = bdl->percent[i];
> +		if (bdl->tuples[i].percent > percent_max)
> +			percent_max = bdl->tuples[i].percent;

When reading this:
`
		if (bdl->samples[i].percent > percent_max)
			percent_max = bdl->samples[i].percent;

It gets clearer what this is about.

>  	}
>  
>  	if (dl->offset != -1 && percent_max != 0.0) {
>  		for (i = 0; i < ab->nr_events; i++) {
> -			ui_browser__set_percent_color(browser, bdl->percent[i],
> +			ui_browser__set_percent_color(browser,
> +						      bdl->tuples[i].percent,
>  						      current_entry);
> -			slsmg_printf("%6.2f ", bdl->percent[i]);
> +			if (annotate_browser__opts.show_total_period)
> +				slsmg_printf("%6.0f ", bdl->tuples[i].samples);
> +			else
> +				slsmg_printf("%6.2f ", bdl->tuples[i].percent);
>  		}
>  	} else {
>  		ui_browser__set_percent_color(browser, 0, current_entry);
> @@ -273,9 +283,9 @@ static int disasm__cmp(struct browser_disasm_line *a,
>  	int i;
>  
>  	for (i = 0; i < nr_pcnt; i++) {
> -		if (a->percent[i] == b->percent[i])
> +		if (a->tuples[i].percent == b->tuples[i].percent)
>  			continue;
> -		return a->percent[i] < b->percent[i];
> +		return a->tuples[i].percent < b->tuples[i].percent;
>  	}
>  	return 0;
>  }
> @@ -366,14 +376,17 @@ 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++) {
> -			bpos->percent[i] = disasm__calc_percent(notes,
> +			double samples;
> +
> +			bpos->tuples[i].percent = disasm__calc_percent(notes,
>  						evsel->idx + i,
>  						pos->offset,
>  						next ? next->offset : len,
> -					        &path);
> +						&path, &samples);
> +			bpos->tuples[i].samples = samples;
>  
> -			if (max_percent < bpos->percent[i])
> -				max_percent = bpos->percent[i];
> +			if (max_percent < bpos->tuples[i].percent)
> +				max_percent = bpos->tuples[i].percent;
>  		}
>  
>  		if (max_percent < 0.01) {
> @@ -737,6 +750,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
>  		"n             Search next string\n"
>  		"o             Toggle disassembler output/simplified view\n"
>  		"s             Toggle source code view\n"
> +		"t             Toggle total period view\n"
>  		"/             Search string\n"
>  		"k             Toggle line numbers\n"
>  		"r             Run available scripts\n"
> @@ -812,6 +826,11 @@ show_sup_ins:
>  				ui_helpline__puts("Actions are only available for 'callq', 'retq' & jump instructions.");
>  			}
>  			continue;
> +		case 't':
> +			annotate_browser__opts.show_total_period =
> +			  !annotate_browser__opts.show_total_period;
> +			annotate_browser__update_addr_width(browser);
> +			continue;
>  		case K_LEFT:
>  		case K_ESC:
>  		case 'q':
> @@ -836,12 +855,16 @@ int map_symbol__tui_annotate(struct map_symbol *ms, struct perf_evsel *evsel,
>  }
>  
>  int hist_entry__tui_annotate(struct hist_entry *he, struct perf_evsel *evsel,
> -			     struct hist_browser_timer *hbt)
> +			     struct hist_browser_timer *hbt,
> +			     bool show_total_period)
>  {
>  	/* reset abort key so that it can get Ctrl-C as a key */
>  	SLang_reset_tty();
>  	SLang_init_tty(0, 0, 0);
>  
> +	/* Set default value for show_total_period.  */
> +	annotate_browser__opts.show_total_period = show_total_period;
> +
>  	return map_symbol__tui_annotate(&he->ms, evsel, hbt);
>  }
>  
> @@ -929,7 +952,7 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map,
>  
>  	if (perf_evsel__is_group_event(evsel)) {
>  		nr_pcnt = evsel->nr_members;
> -		sizeof_bdl += sizeof(double) * (nr_pcnt - 1);
> +		sizeof_bdl += sizeof(struct disasm_tuple) * (nr_pcnt - 1);
>  	}
>  
>  	if (symbol__annotate(sym, map, sizeof_bdl) < 0) {
> @@ -1006,6 +1029,7 @@ static struct annotate_config {
>  	ANNOTATE_CFG(show_linenr),
>  	ANNOTATE_CFG(show_nr_jumps),
>  	ANNOTATE_CFG(use_offset),
> +	ANNOTATE_CFG(show_total_period),
>  };
>  
>  #undef ANNOTATE_CFG
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index bf80430..8b94603 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -654,10 +654,11 @@ 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)
> +			    s64 end, const char **path, double *samples)
>  {
>  	struct source_line *src_line = notes->src->lines;
>  	double percent = 0.0;
> +	*samples = 0.0;
>  
>  	if (src_line) {
>  		size_t sizeof_src_line = sizeof(*src_line) +
> @@ -671,6 +672,7 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset,
>  				*path = src_line->path;
>  
>  			percent += src_line->p[evidx].percent;
> +			*samples += src_line->p[evidx].samples;
>  			offset++;
>  		}
>  	} else {
> @@ -680,8 +682,10 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset,
>  		while (offset < end)
>  			hits += h->addr[offset++];
>  
> -		if (h->sum)
> +		if (h->sum) {
> +			*samples = hits;
>  			percent = 100.0 * hits / h->sum;
> +		}
>  	}
>  
>  	return percent;
> @@ -696,8 +700,9 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
>  
>  	if (dl->offset != -1) {
>  		const char *path = NULL;
> -		double percent, max_percent = 0.0;
> +		double percent, samples, max_percent = 0.0;
>  		double *ppercents = &percent;
> +		double *psamples = &samples;
>  		int i, nr_percent = 1;
>  		const char *color;
>  		struct annotation *notes = symbol__annotation(sym);
> @@ -710,8 +715,10 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
>  		if (perf_evsel__is_group_event(evsel)) {
>  			nr_percent = evsel->nr_members;
>  			ppercents = calloc(nr_percent, sizeof(double));
> -			if (ppercents == NULL)
> +			psamples = calloc(nr_percent, sizeof(double));
> +			if (ppercents == NULL || psamples == NULL) {
>  				return -1;
> +			}
>  		}
>  
>  		for (i = 0; i < nr_percent; i++) {
> @@ -719,9 +726,10 @@ 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);
> +					&path, &samples);
>  
>  			ppercents[i] = percent;
> +			psamples[i] = samples;
>  			if (percent > max_percent)
>  				max_percent = percent;
>  		}
> @@ -759,8 +767,13 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
>  
>  		for (i = 0; i < nr_percent; i++) {
>  			percent = ppercents[i];
> +			samples = psamples[i];
>  			color = get_percent_color(percent);
> -			color_fprintf(stdout, color, " %7.2f", percent);
> +
> +			if (symbol_conf.show_total_period)
> +				color_fprintf(stdout, color, " %7.0f", samples);
> +			else
> +				color_fprintf(stdout, color, " %7.2f", percent);
>  		}
>  
>  		printf(" :	");
> @@ -770,6 +783,9 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
>  		if (ppercents != &percent)
>  			free(ppercents);
>  
> +		if (psamples != &samples)
> +			free(psamples);
> +
>  	} else if (max_lines && printed >= max_lines)
>  		return 1;
>  	else {
> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
> index cadbdc9..752d1bd 100644
> --- a/tools/perf/util/annotate.h
> +++ b/tools/perf/util/annotate.h
> @@ -72,7 +72,7 @@ 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);
> +			    s64 end, const char **path, double *samples);

Change samples to u64

>  
>  struct sym_hist {
>  	u64		sum;
> @@ -82,6 +82,7 @@ struct sym_hist {
>  struct source_line_percent {

Please rename source_line_percent to source_line_samples, for
consistency with 'struct disasm_line_samples'

>  	double		percent;
>  	double		percent_sum;
> +	double          samples;

Also change to u64 and rename it to 'nr'.

struct source_line_samples {
	double	percent;
	double	percent_sum;
	u64	nr;
};

>  };
>  
>  struct source_line {
> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
> index 5ed8d9c..65f953f 100644
> --- a/tools/perf/util/hist.h
> +++ b/tools/perf/util/hist.h
> @@ -306,7 +306,8 @@ int map_symbol__tui_annotate(struct map_symbol *ms, struct perf_evsel *evsel,
>  			     struct hist_browser_timer *hbt);
>  
>  int hist_entry__tui_annotate(struct hist_entry *he, struct perf_evsel *evsel,
> -			     struct hist_browser_timer *hbt);
> +			     struct hist_browser_timer *hbt,
> +			     bool show_total_period);

Why not use symbol_conf.show_total_period, since it already is there for
this?

>  
>  int perf_evlist__tui_browse_hists(struct perf_evlist *evlist, const char *help,
>  				  struct hist_browser_timer *hbt,
> -- 
> 2.1.4
> 


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] perf annotate: With --show-total-period, display total # of samples.
  2015-06-18 19:15     ` Arnaldo Carvalho de Melo
@ 2015-06-19  9:35       ` Martin Liška
  2015-06-19 19:10         ` Arnaldo Carvalho de Melo
  2015-06-19 23:15         ` [tip:perf/core] perf annotate: Display total number of samples with --show-total-period tip-bot for Martin Liška
  0 siblings, 2 replies; 9+ messages in thread
From: Martin Liška @ 2015-06-19  9:35 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, linux-kernel, Ingo Molnar, Peter Zijlstra,
	Paul Mackerras, Andi Kleen

[-- Attachment #1: Type: text/plain, Size: 13750 bytes --]

On 06/18/2015 09:15 PM, Arnaldo Carvalho de Melo wrote:
> Em Thu, Jun 18, 2015 at 08:06:25PM +0200, Martin Liška escreveu:
>> On 06/18/2015 06:26 PM, Ingo Molnar wrote:
>>> * Martin Liška <mliska@suse.cz> wrote:
>>>> +++ b/tools/perf/builtin-annotate.c
>>>> +	OPT_BOOLEAN(0, "show-total-period", &symbol_conf.show_total_period,
>>>> +		    "Show a column with the sum of periods"),
>>>>  	OPT_END()
> 
>>> What is the toggle for this in the 'perf report' TUI? (which displays annotated 
>>> output too)
> 
>> Thanks for note, I fixed that by introducing a new 't' hot key.
> 
>> >From 98e1998fcd7481c7e1756e643d66eb85559849ac Mon Sep 17 00:00:00 2001
>> From: mliska <mliska@suse.cz>
>> Date: Wed, 27 May 2015 10:54:42 +0200
>> Subject: [PATCH] perf annotate: With --show-total-period, display total # of
>>  samples.
>>
>> To compare two records on an instruction base, with --show-total-period
>> option provided, display total number of samples that belong to a line
>> in assembly language.
>>
>> New hot key 't' is introduced for 'perf annotate' TUI.
> 
>> Signed-off-by: Martin Liska <mliska@suse.cz>
>> ---
>>  tools/perf/builtin-annotate.c     |  5 +++-
>>  tools/perf/ui/browsers/annotate.c | 60 +++++++++++++++++++++++++++------------
>>  tools/perf/util/annotate.c        | 28 ++++++++++++++----
>>  tools/perf/util/annotate.h        |  3 +-
>>  tools/perf/util/hist.h            |  3 +-
>>  5 files changed, 72 insertions(+), 27 deletions(-)
>>
>> diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
>> index 4e08c2d..b0c442e 100644
>> --- a/tools/perf/builtin-annotate.c
>> +++ b/tools/perf/builtin-annotate.c
>> @@ -161,7 +161,8 @@ find_next:
>>  			/* skip missing symbols */
>>  			nd = rb_next(nd);
>>  		} else if (use_browser == 1) {
>> -			key = hist_entry__tui_annotate(he, evsel, NULL);
>> +			key = hist_entry__tui_annotate(he, evsel, NULL,
>> +				symbol_conf.show_total_period);
> 
> No need to pass this global variable around, since we already have it
> like that, why not simply read it in hist_entry__tui_annotate()?
> 
>>  			switch (key) {
>>  			case -1:
>>  				if (!ann->skip_missing)
>> @@ -329,6 +330,8 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused)
>>  		   "objdump binary to use for disassembly and annotations"),
>>  	OPT_BOOLEAN(0, "group", &symbol_conf.event_group,
>>  		    "Show event group information together"),
>> +	OPT_BOOLEAN(0, "show-total-period", &symbol_conf.show_total_period,
>> +		    "Show a column with the sum of periods"),
>>  	OPT_END()
>>  	};
>>  	int ret = hists__init();
>> diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
>> index acb0e23..e7cd596 100644
>> --- a/tools/perf/ui/browsers/annotate.c
>> +++ b/tools/perf/ui/browsers/annotate.c
>> @@ -11,16 +11,21 @@
>>  #include "../../util/evsel.h"
>>  #include <pthread.h>
>>  
>> +struct disasm_tuple {
>> +	double		percent;
>> +	double		samples;
> 
> Why use double for 'samples'? We use u64 elsewhere for this.
> 
> And 'disasm_tuple' is way too vague, how about:
> 
> struct disasm_line_samples {
> 	double		percent;
> 	u64		nr;
> };
> 
> and then, below...
> 
>> +};
>> +
>>  struct browser_disasm_line {
>> -	struct rb_node	rb_node;
>> -	u32		idx;
>> -	int		idx_asm;
>> -	int		jump_sources;
>> +	struct rb_node		rb_node;
>> +	u32			idx;
>> +	int			idx_asm;
>> +	int			jump_sources;
>>  	/*
>>  	 * actual length of this array is saved on the nr_events field
>>  	 * of the struct annotate_browser
>>  	 */
>> -	double		percent[1];
>> +	struct disasm_tuple    tuples[1];
> 
> here have:
> 
> 	struct disasm_line_samples samples;o
> 
> Then use bdl->samples[i].nr and bdl->samples[i].percent.
> 
>>  };
>>  
>>  static struct annotate_browser_opt {
>> @@ -28,7 +33,8 @@ static struct annotate_browser_opt {
>>  	     use_offset,
>>  	     jump_arrows,
>>  	     show_linenr,
>> -	     show_nr_jumps;
>> +	     show_nr_jumps,
>> +	     show_total_period;
>>  } annotate_browser__opts = {
>>  	.use_offset	= true,
>>  	.jump_arrows	= true,
>> @@ -105,15 +111,19 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
>>  	char bf[256];
>>  
>>  	for (i = 0; i < ab->nr_events; i++) {
>> -		if (bdl->percent[i] > percent_max)
>> -			percent_max = bdl->percent[i];
>> +		if (bdl->tuples[i].percent > percent_max)
>> +			percent_max = bdl->tuples[i].percent;
> 
> When reading this:
> `
> 		if (bdl->samples[i].percent > percent_max)
> 			percent_max = bdl->samples[i].percent;
> 
> It gets clearer what this is about.
> 
>>  	}
>>  
>>  	if (dl->offset != -1 && percent_max != 0.0) {
>>  		for (i = 0; i < ab->nr_events; i++) {
>> -			ui_browser__set_percent_color(browser, bdl->percent[i],
>> +			ui_browser__set_percent_color(browser,
>> +						      bdl->tuples[i].percent,
>>  						      current_entry);
>> -			slsmg_printf("%6.2f ", bdl->percent[i]);
>> +			if (annotate_browser__opts.show_total_period)
>> +				slsmg_printf("%6.0f ", bdl->tuples[i].samples);
>> +			else
>> +				slsmg_printf("%6.2f ", bdl->tuples[i].percent);
>>  		}
>>  	} else {
>>  		ui_browser__set_percent_color(browser, 0, current_entry);
>> @@ -273,9 +283,9 @@ static int disasm__cmp(struct browser_disasm_line *a,
>>  	int i;
>>  
>>  	for (i = 0; i < nr_pcnt; i++) {
>> -		if (a->percent[i] == b->percent[i])
>> +		if (a->tuples[i].percent == b->tuples[i].percent)
>>  			continue;
>> -		return a->percent[i] < b->percent[i];
>> +		return a->tuples[i].percent < b->tuples[i].percent;
>>  	}
>>  	return 0;
>>  }
>> @@ -366,14 +376,17 @@ 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++) {
>> -			bpos->percent[i] = disasm__calc_percent(notes,
>> +			double samples;
>> +
>> +			bpos->tuples[i].percent = disasm__calc_percent(notes,
>>  						evsel->idx + i,
>>  						pos->offset,
>>  						next ? next->offset : len,
>> -					        &path);
>> +						&path, &samples);
>> +			bpos->tuples[i].samples = samples;
>>  
>> -			if (max_percent < bpos->percent[i])
>> -				max_percent = bpos->percent[i];
>> +			if (max_percent < bpos->tuples[i].percent)
>> +				max_percent = bpos->tuples[i].percent;
>>  		}
>>  
>>  		if (max_percent < 0.01) {
>> @@ -737,6 +750,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
>>  		"n             Search next string\n"
>>  		"o             Toggle disassembler output/simplified view\n"
>>  		"s             Toggle source code view\n"
>> +		"t             Toggle total period view\n"
>>  		"/             Search string\n"
>>  		"k             Toggle line numbers\n"
>>  		"r             Run available scripts\n"
>> @@ -812,6 +826,11 @@ show_sup_ins:
>>  				ui_helpline__puts("Actions are only available for 'callq', 'retq' & jump instructions.");
>>  			}
>>  			continue;
>> +		case 't':
>> +			annotate_browser__opts.show_total_period =
>> +			  !annotate_browser__opts.show_total_period;
>> +			annotate_browser__update_addr_width(browser);
>> +			continue;
>>  		case K_LEFT:
>>  		case K_ESC:
>>  		case 'q':
>> @@ -836,12 +855,16 @@ int map_symbol__tui_annotate(struct map_symbol *ms, struct perf_evsel *evsel,
>>  }
>>  
>>  int hist_entry__tui_annotate(struct hist_entry *he, struct perf_evsel *evsel,
>> -			     struct hist_browser_timer *hbt)
>> +			     struct hist_browser_timer *hbt,
>> +			     bool show_total_period)
>>  {
>>  	/* reset abort key so that it can get Ctrl-C as a key */
>>  	SLang_reset_tty();
>>  	SLang_init_tty(0, 0, 0);
>>  
>> +	/* Set default value for show_total_period.  */
>> +	annotate_browser__opts.show_total_period = show_total_period;
>> +
>>  	return map_symbol__tui_annotate(&he->ms, evsel, hbt);
>>  }
>>  
>> @@ -929,7 +952,7 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map,
>>  
>>  	if (perf_evsel__is_group_event(evsel)) {
>>  		nr_pcnt = evsel->nr_members;
>> -		sizeof_bdl += sizeof(double) * (nr_pcnt - 1);
>> +		sizeof_bdl += sizeof(struct disasm_tuple) * (nr_pcnt - 1);
>>  	}
>>  
>>  	if (symbol__annotate(sym, map, sizeof_bdl) < 0) {
>> @@ -1006,6 +1029,7 @@ static struct annotate_config {
>>  	ANNOTATE_CFG(show_linenr),
>>  	ANNOTATE_CFG(show_nr_jumps),
>>  	ANNOTATE_CFG(use_offset),
>> +	ANNOTATE_CFG(show_total_period),
>>  };
>>  
>>  #undef ANNOTATE_CFG
>> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
>> index bf80430..8b94603 100644
>> --- a/tools/perf/util/annotate.c
>> +++ b/tools/perf/util/annotate.c
>> @@ -654,10 +654,11 @@ 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)
>> +			    s64 end, const char **path, double *samples)
>>  {
>>  	struct source_line *src_line = notes->src->lines;
>>  	double percent = 0.0;
>> +	*samples = 0.0;
>>  
>>  	if (src_line) {
>>  		size_t sizeof_src_line = sizeof(*src_line) +
>> @@ -671,6 +672,7 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset,
>>  				*path = src_line->path;
>>  
>>  			percent += src_line->p[evidx].percent;
>> +			*samples += src_line->p[evidx].samples;
>>  			offset++;
>>  		}
>>  	} else {
>> @@ -680,8 +682,10 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset,
>>  		while (offset < end)
>>  			hits += h->addr[offset++];
>>  
>> -		if (h->sum)
>> +		if (h->sum) {
>> +			*samples = hits;
>>  			percent = 100.0 * hits / h->sum;
>> +		}
>>  	}
>>  
>>  	return percent;
>> @@ -696,8 +700,9 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
>>  
>>  	if (dl->offset != -1) {
>>  		const char *path = NULL;
>> -		double percent, max_percent = 0.0;
>> +		double percent, samples, max_percent = 0.0;
>>  		double *ppercents = &percent;
>> +		double *psamples = &samples;
>>  		int i, nr_percent = 1;
>>  		const char *color;
>>  		struct annotation *notes = symbol__annotation(sym);
>> @@ -710,8 +715,10 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
>>  		if (perf_evsel__is_group_event(evsel)) {
>>  			nr_percent = evsel->nr_members;
>>  			ppercents = calloc(nr_percent, sizeof(double));
>> -			if (ppercents == NULL)
>> +			psamples = calloc(nr_percent, sizeof(double));
>> +			if (ppercents == NULL || psamples == NULL) {
>>  				return -1;
>> +			}
>>  		}
>>  
>>  		for (i = 0; i < nr_percent; i++) {
>> @@ -719,9 +726,10 @@ 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);
>> +					&path, &samples);
>>  
>>  			ppercents[i] = percent;
>> +			psamples[i] = samples;
>>  			if (percent > max_percent)
>>  				max_percent = percent;
>>  		}
>> @@ -759,8 +767,13 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
>>  
>>  		for (i = 0; i < nr_percent; i++) {
>>  			percent = ppercents[i];
>> +			samples = psamples[i];
>>  			color = get_percent_color(percent);
>> -			color_fprintf(stdout, color, " %7.2f", percent);
>> +
>> +			if (symbol_conf.show_total_period)
>> +				color_fprintf(stdout, color, " %7.0f", samples);
>> +			else
>> +				color_fprintf(stdout, color, " %7.2f", percent);
>>  		}
>>  
>>  		printf(" :	");
>> @@ -770,6 +783,9 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
>>  		if (ppercents != &percent)
>>  			free(ppercents);
>>  
>> +		if (psamples != &samples)
>> +			free(psamples);
>> +
>>  	} else if (max_lines && printed >= max_lines)
>>  		return 1;
>>  	else {
>> diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
>> index cadbdc9..752d1bd 100644
>> --- a/tools/perf/util/annotate.h
>> +++ b/tools/perf/util/annotate.h
>> @@ -72,7 +72,7 @@ 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);
>> +			    s64 end, const char **path, double *samples);
> 
> Change samples to u64
> 
>>  
>>  struct sym_hist {
>>  	u64		sum;
>> @@ -82,6 +82,7 @@ struct sym_hist {
>>  struct source_line_percent {
> 
> Please rename source_line_percent to source_line_samples, for
> consistency with 'struct disasm_line_samples'
> 
>>  	double		percent;
>>  	double		percent_sum;
>> +	double          samples;
> 
> Also change to u64 and rename it to 'nr'.
> 
> struct source_line_samples {
> 	double	percent;
> 	double	percent_sum;
> 	u64	nr;
> };
> 
>>  };
>>  
>>  struct source_line {
>> diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
>> index 5ed8d9c..65f953f 100644
>> --- a/tools/perf/util/hist.h
>> +++ b/tools/perf/util/hist.h
>> @@ -306,7 +306,8 @@ int map_symbol__tui_annotate(struct map_symbol *ms, struct perf_evsel *evsel,
>>  			     struct hist_browser_timer *hbt);
>>  
>>  int hist_entry__tui_annotate(struct hist_entry *he, struct perf_evsel *evsel,
>> -			     struct hist_browser_timer *hbt);
>> +			     struct hist_browser_timer *hbt,
>> +			     bool show_total_period);
> 
> Why not use symbol_conf.show_total_period, since it already is there for
> this?
> 
>>  
>>  int perf_evlist__tui_browse_hists(struct perf_evlist *evlist, const char *help,
>>  				  struct hist_browser_timer *hbt,
>> -- 
>> 2.1.4
>>
> 

Hello.

Thank you for the review, all remarks were reasonable.
Feel free to comment next version (v3) of the patch.

Thanks,
Martin

[-- Attachment #2: 0001-perf-annotate-With-show-total-period-display-total-o-v3.patch --]
[-- Type: text/x-patch, Size: 9839 bytes --]

>From 1c0d49462a82f1085de3675e20c28d8445cd493e Mon Sep 17 00:00:00 2001
From: mliska <mliska@suse.cz>
Date: Wed, 27 May 2015 10:54:42 +0200
Subject: [PATCH] perf annotate: With --show-total-period, display total # of
 samples.

To compare two records on an instruction base, with --show-total-period
option provided, display total number of samples that belong to a line
in assembly language.

New hot key 't' is introduced for 'perf annotate' TUI.

Signed-off-by: Martin Liska <mliska@suse.cz>
---
 tools/perf/builtin-annotate.c     |  2 ++
 tools/perf/ui/browsers/annotate.c | 60 ++++++++++++++++++++++++++++-----------
 tools/perf/util/annotate.c        | 28 ++++++++++++++----
 tools/perf/util/annotate.h        |  3 +-
 4 files changed, 70 insertions(+), 23 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 4e08c2d..2c1bec3 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -329,6 +329,8 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused)
 		   "objdump binary to use for disassembly and annotations"),
 	OPT_BOOLEAN(0, "group", &symbol_conf.event_group,
 		    "Show event group information together"),
+	OPT_BOOLEAN(0, "show-total-period", &symbol_conf.show_total_period,
+		    "Show a column with the sum of periods"),
 	OPT_END()
 	};
 	int ret = hists__init();
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index acb0e23..e80df4e 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -11,16 +11,21 @@
 #include "../../util/evsel.h"
 #include <pthread.h>
 
+struct disasm_line_samples {
+	double		percent;
+	u64		nr;
+};
+
 struct browser_disasm_line {
-	struct rb_node	rb_node;
-	u32		idx;
-	int		idx_asm;
-	int		jump_sources;
+	struct rb_node			rb_node;
+	u32				idx;
+	int				idx_asm;
+	int				jump_sources;
 	/*
 	 * actual length of this array is saved on the nr_events field
 	 * of the struct annotate_browser
 	 */
-	double		percent[1];
+	struct disasm_line_samples	tuples[1];
 };
 
 static struct annotate_browser_opt {
@@ -28,7 +33,8 @@ static struct annotate_browser_opt {
 	     use_offset,
 	     jump_arrows,
 	     show_linenr,
-	     show_nr_jumps;
+	     show_nr_jumps,
+	     show_total_period;
 } annotate_browser__opts = {
 	.use_offset	= true,
 	.jump_arrows	= true,
@@ -105,15 +111,20 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
 	char bf[256];
 
 	for (i = 0; i < ab->nr_events; i++) {
-		if (bdl->percent[i] > percent_max)
-			percent_max = bdl->percent[i];
+		if (bdl->tuples[i].percent > percent_max)
+			percent_max = bdl->tuples[i].percent;
 	}
 
 	if (dl->offset != -1 && percent_max != 0.0) {
 		for (i = 0; i < ab->nr_events; i++) {
-			ui_browser__set_percent_color(browser, bdl->percent[i],
+			ui_browser__set_percent_color(browser,
+						      bdl->tuples[i].percent,
 						      current_entry);
-			slsmg_printf("%6.2f ", bdl->percent[i]);
+			if (annotate_browser__opts.show_total_period)
+				slsmg_printf("%6" PRIu64 " ",
+					     bdl->tuples[i].nr);
+			else
+				slsmg_printf("%6.2f ", bdl->tuples[i].percent);
 		}
 	} else {
 		ui_browser__set_percent_color(browser, 0, current_entry);
@@ -273,9 +284,9 @@ static int disasm__cmp(struct browser_disasm_line *a,
 	int i;
 
 	for (i = 0; i < nr_pcnt; i++) {
-		if (a->percent[i] == b->percent[i])
+		if (a->tuples[i].percent == b->tuples[i].percent)
 			continue;
-		return a->percent[i] < b->percent[i];
+		return a->tuples[i].percent < b->tuples[i].percent;
 	}
 	return 0;
 }
@@ -366,14 +377,17 @@ 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++) {
-			bpos->percent[i] = disasm__calc_percent(notes,
+			u64 nr_samples;
+
+			bpos->tuples[i].percent = disasm__calc_percent(notes,
 						evsel->idx + i,
 						pos->offset,
 						next ? next->offset : len,
-					        &path);
+						&path, &nr_samples);
+			bpos->tuples[i].nr = nr_samples;
 
-			if (max_percent < bpos->percent[i])
-				max_percent = bpos->percent[i];
+			if (max_percent < bpos->tuples[i].percent)
+				max_percent = bpos->tuples[i].percent;
 		}
 
 		if (max_percent < 0.01) {
@@ -737,6 +751,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
 		"n             Search next string\n"
 		"o             Toggle disassembler output/simplified view\n"
 		"s             Toggle source code view\n"
+		"t             Toggle total period view\n"
 		"/             Search string\n"
 		"k             Toggle line numbers\n"
 		"r             Run available scripts\n"
@@ -812,6 +827,11 @@ show_sup_ins:
 				ui_helpline__puts("Actions are only available for 'callq', 'retq' & jump instructions.");
 			}
 			continue;
+		case 't':
+			annotate_browser__opts.show_total_period =
+			  !annotate_browser__opts.show_total_period;
+			annotate_browser__update_addr_width(browser);
+			continue;
 		case K_LEFT:
 		case K_ESC:
 		case 'q':
@@ -832,6 +852,10 @@ out:
 int map_symbol__tui_annotate(struct map_symbol *ms, struct perf_evsel *evsel,
 			     struct hist_browser_timer *hbt)
 {
+	/* Set default value for show_total_period.  */
+	annotate_browser__opts.show_total_period =
+	  symbol_conf.show_total_period;
+
 	return symbol__tui_annotate(ms->sym, ms->map, evsel, hbt);
 }
 
@@ -929,7 +953,8 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map,
 
 	if (perf_evsel__is_group_event(evsel)) {
 		nr_pcnt = evsel->nr_members;
-		sizeof_bdl += sizeof(double) * (nr_pcnt - 1);
+		sizeof_bdl += sizeof(struct disasm_line_samples)
+		  * (nr_pcnt - 1);
 	}
 
 	if (symbol__annotate(sym, map, sizeof_bdl) < 0) {
@@ -1006,6 +1031,7 @@ static struct annotate_config {
 	ANNOTATE_CFG(show_linenr),
 	ANNOTATE_CFG(show_nr_jumps),
 	ANNOTATE_CFG(use_offset),
+	ANNOTATE_CFG(show_total_period),
 };
 
 #undef ANNOTATE_CFG
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index bf80430..12914b6 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -654,10 +654,11 @@ 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)
+			    s64 end, const char **path, u64 *nr_samples)
 {
 	struct source_line *src_line = notes->src->lines;
 	double percent = 0.0;
+	*nr_samples = 0;
 
 	if (src_line) {
 		size_t sizeof_src_line = sizeof(*src_line) +
@@ -671,6 +672,7 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset,
 				*path = src_line->path;
 
 			percent += src_line->p[evidx].percent;
+			*nr_samples += src_line->p[evidx].samples;
 			offset++;
 		}
 	} else {
@@ -680,8 +682,10 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset,
 		while (offset < end)
 			hits += h->addr[offset++];
 
-		if (h->sum)
+		if (h->sum) {
+			*nr_samples = hits;
 			percent = 100.0 * hits / h->sum;
+		}
 	}
 
 	return percent;
@@ -696,8 +700,10 @@ 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;
 		double percent, max_percent = 0.0;
 		double *ppercents = &percent;
+		u64 *psamples = &nr_samples;
 		int i, nr_percent = 1;
 		const char *color;
 		struct annotation *notes = symbol__annotation(sym);
@@ -710,8 +716,10 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
 		if (perf_evsel__is_group_event(evsel)) {
 			nr_percent = evsel->nr_members;
 			ppercents = calloc(nr_percent, sizeof(double));
-			if (ppercents == NULL)
+			psamples = calloc(nr_percent, sizeof(u64));
+			if (ppercents == NULL || psamples == NULL) {
 				return -1;
+			}
 		}
 
 		for (i = 0; i < nr_percent; i++) {
@@ -719,9 +727,10 @@ 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);
+					&path, &nr_samples);
 
 			ppercents[i] = percent;
+			psamples[i] = nr_samples;
 			if (percent > max_percent)
 				max_percent = percent;
 		}
@@ -759,8 +768,14 @@ 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];
 			color = get_percent_color(percent);
-			color_fprintf(stdout, color, " %7.2f", percent);
+
+			if (symbol_conf.show_total_period)
+				color_fprintf(stdout, color, " %7" PRIu64,
+					      nr_samples);
+			else
+				color_fprintf(stdout, color, " %7.2f", percent);
 		}
 
 		printf(" :	");
@@ -770,6 +785,9 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
 		if (ppercents != &percent)
 			free(ppercents);
 
+		if (psamples != &nr_samples)
+			free(psamples);
+
 	} else if (max_lines && printed >= max_lines)
 		return 1;
 	else {
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index cadbdc9..c8c18ca 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -72,7 +72,7 @@ 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);
+			    s64 end, const char **path, u64 *nr_samples);
 
 struct sym_hist {
 	u64		sum;
@@ -82,6 +82,7 @@ struct sym_hist {
 struct source_line_percent {
 	double		percent;
 	double		percent_sum;
+	double          samples;
 };
 
 struct source_line {
-- 
2.1.4


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] perf annotate: With --show-total-period, display total # of samples.
  2015-06-19  9:35       ` Martin Liška
@ 2015-06-19 19:10         ` Arnaldo Carvalho de Melo
  2015-06-20 11:53           ` Martin Liška
  2015-06-19 23:15         ` [tip:perf/core] perf annotate: Display total number of samples with --show-total-period tip-bot for Martin Liška
  1 sibling, 1 reply; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-06-19 19:10 UTC (permalink / raw)
  To: Martin Liška
  Cc: Ingo Molnar, linux-kernel, Ingo Molnar, Peter Zijlstra,
	Paul Mackerras, Andi Kleen

Em Fri, Jun 19, 2015 at 11:35:41AM +0200, Martin Liška escreveu:
> Thank you for the review, all remarks were reasonable.
> Feel free to comment next version (v3) of the patch.

Thanks, there was just one missing s/tuples/samples/g, I did it and
tried running git-am on the attached patch, it failed, so I applied it
manually.

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [tip:perf/core] perf annotate: Display total number of samples with --show-total-period
  2015-06-19  9:35       ` Martin Liška
  2015-06-19 19:10         ` Arnaldo Carvalho de Melo
@ 2015-06-19 23:15         ` tip-bot for Martin Liška
  1 sibling, 0 replies; 9+ messages in thread
From: tip-bot for Martin Liška @ 2015-06-19 23:15 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: linux-kernel, hpa, acme, a.p.zijlstra, mliska, mingo, tglx, andi

Commit-ID:  0c4a5bcea4609948375173cdea8d73783110a75e
Gitweb:     http://git.kernel.org/tip/0c4a5bcea4609948375173cdea8d73783110a75e
Author:     Martin Liška <mliska@suse.cz>
AuthorDate: Fri, 19 Jun 2015 16:10:43 -0300
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Fri, 19 Jun 2015 16:39:18 -0300

perf annotate: Display total number of samples with --show-total-period

To compare two records on an instruction base, with --show-total-period
option provided, display total number of samples that belong to a line
in assembly language.

New hot key 't' is introduced for 'perf annotate' TUI.

Signed-off-by: Martin Liska <mliska@suse.cz>
Cc: Andi Kleen <andi@firstfloor.org>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Link: http://lkml.kernel.org/r/5583E26D.1040407@suse.cz
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-annotate.c     |  2 ++
 tools/perf/ui/browsers/annotate.c | 60 ++++++++++++++++++++++++++++-----------
 tools/perf/util/annotate.c        | 28 ++++++++++++++----
 tools/perf/util/annotate.h        |  3 +-
 4 files changed, 70 insertions(+), 23 deletions(-)

diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c
index 4e08c2d..2c1bec3 100644
--- a/tools/perf/builtin-annotate.c
+++ b/tools/perf/builtin-annotate.c
@@ -329,6 +329,8 @@ int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused)
 		   "objdump binary to use for disassembly and annotations"),
 	OPT_BOOLEAN(0, "group", &symbol_conf.event_group,
 		    "Show event group information together"),
+	OPT_BOOLEAN(0, "show-total-period", &symbol_conf.show_total_period,
+		    "Show a column with the sum of periods"),
 	OPT_END()
 	};
 	int ret = hists__init();
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index acb0e23..5995a8b 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -11,16 +11,21 @@
 #include "../../util/evsel.h"
 #include <pthread.h>
 
+struct disasm_line_samples {
+	double		percent;
+	u64		nr;
+};
+
 struct browser_disasm_line {
-	struct rb_node	rb_node;
-	u32		idx;
-	int		idx_asm;
-	int		jump_sources;
+	struct rb_node			rb_node;
+	u32				idx;
+	int				idx_asm;
+	int				jump_sources;
 	/*
 	 * actual length of this array is saved on the nr_events field
 	 * of the struct annotate_browser
 	 */
-	double		percent[1];
+	struct disasm_line_samples	samples[1];
 };
 
 static struct annotate_browser_opt {
@@ -28,7 +33,8 @@ static struct annotate_browser_opt {
 	     use_offset,
 	     jump_arrows,
 	     show_linenr,
-	     show_nr_jumps;
+	     show_nr_jumps,
+	     show_total_period;
 } annotate_browser__opts = {
 	.use_offset	= true,
 	.jump_arrows	= true,
@@ -105,15 +111,20 @@ static void annotate_browser__write(struct ui_browser *browser, void *entry, int
 	char bf[256];
 
 	for (i = 0; i < ab->nr_events; i++) {
-		if (bdl->percent[i] > percent_max)
-			percent_max = bdl->percent[i];
+		if (bdl->samples[i].percent > percent_max)
+			percent_max = bdl->samples[i].percent;
 	}
 
 	if (dl->offset != -1 && percent_max != 0.0) {
 		for (i = 0; i < ab->nr_events; i++) {
-			ui_browser__set_percent_color(browser, bdl->percent[i],
+			ui_browser__set_percent_color(browser,
+						      bdl->samples[i].percent,
 						      current_entry);
-			slsmg_printf("%6.2f ", bdl->percent[i]);
+			if (annotate_browser__opts.show_total_period)
+				slsmg_printf("%6" PRIu64 " ",
+					     bdl->samples[i].nr);
+			else
+				slsmg_printf("%6.2f ", bdl->samples[i].percent);
 		}
 	} else {
 		ui_browser__set_percent_color(browser, 0, current_entry);
@@ -273,9 +284,9 @@ static int disasm__cmp(struct browser_disasm_line *a,
 	int i;
 
 	for (i = 0; i < nr_pcnt; i++) {
-		if (a->percent[i] == b->percent[i])
+		if (a->samples[i].percent == b->samples[i].percent)
 			continue;
-		return a->percent[i] < b->percent[i];
+		return a->samples[i].percent < b->samples[i].percent;
 	}
 	return 0;
 }
@@ -366,14 +377,17 @@ 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++) {
-			bpos->percent[i] = disasm__calc_percent(notes,
+			u64 nr_samples;
+
+			bpos->samples[i].percent = disasm__calc_percent(notes,
 						evsel->idx + i,
 						pos->offset,
 						next ? next->offset : len,
-					        &path);
+						&path, &nr_samples);
+			bpos->samples[i].nr = nr_samples;
 
-			if (max_percent < bpos->percent[i])
-				max_percent = bpos->percent[i];
+			if (max_percent < bpos->samples[i].percent)
+				max_percent = bpos->samples[i].percent;
 		}
 
 		if (max_percent < 0.01) {
@@ -737,6 +751,7 @@ static int annotate_browser__run(struct annotate_browser *browser,
 		"n             Search next string\n"
 		"o             Toggle disassembler output/simplified view\n"
 		"s             Toggle source code view\n"
+		"t             Toggle total period view\n"
 		"/             Search string\n"
 		"k             Toggle line numbers\n"
 		"r             Run available scripts\n"
@@ -812,6 +827,11 @@ show_sup_ins:
 				ui_helpline__puts("Actions are only available for 'callq', 'retq' & jump instructions.");
 			}
 			continue;
+		case 't':
+			annotate_browser__opts.show_total_period =
+			  !annotate_browser__opts.show_total_period;
+			annotate_browser__update_addr_width(browser);
+			continue;
 		case K_LEFT:
 		case K_ESC:
 		case 'q':
@@ -832,6 +852,10 @@ out:
 int map_symbol__tui_annotate(struct map_symbol *ms, struct perf_evsel *evsel,
 			     struct hist_browser_timer *hbt)
 {
+	/* Set default value for show_total_period.  */
+	annotate_browser__opts.show_total_period =
+	  symbol_conf.show_total_period;
+
 	return symbol__tui_annotate(ms->sym, ms->map, evsel, hbt);
 }
 
@@ -929,7 +953,8 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map,
 
 	if (perf_evsel__is_group_event(evsel)) {
 		nr_pcnt = evsel->nr_members;
-		sizeof_bdl += sizeof(double) * (nr_pcnt - 1);
+		sizeof_bdl += sizeof(struct disasm_line_samples) *
+		  (nr_pcnt - 1);
 	}
 
 	if (symbol__annotate(sym, map, sizeof_bdl) < 0) {
@@ -1006,6 +1031,7 @@ static struct annotate_config {
 	ANNOTATE_CFG(show_linenr),
 	ANNOTATE_CFG(show_nr_jumps),
 	ANNOTATE_CFG(use_offset),
+	ANNOTATE_CFG(show_total_period),
 };
 
 #undef ANNOTATE_CFG
diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
index bf80430..12914b6 100644
--- a/tools/perf/util/annotate.c
+++ b/tools/perf/util/annotate.c
@@ -654,10 +654,11 @@ 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)
+			    s64 end, const char **path, u64 *nr_samples)
 {
 	struct source_line *src_line = notes->src->lines;
 	double percent = 0.0;
+	*nr_samples = 0;
 
 	if (src_line) {
 		size_t sizeof_src_line = sizeof(*src_line) +
@@ -671,6 +672,7 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset,
 				*path = src_line->path;
 
 			percent += src_line->p[evidx].percent;
+			*nr_samples += src_line->p[evidx].samples;
 			offset++;
 		}
 	} else {
@@ -680,8 +682,10 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset,
 		while (offset < end)
 			hits += h->addr[offset++];
 
-		if (h->sum)
+		if (h->sum) {
+			*nr_samples = hits;
 			percent = 100.0 * hits / h->sum;
+		}
 	}
 
 	return percent;
@@ -696,8 +700,10 @@ 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;
 		double percent, max_percent = 0.0;
 		double *ppercents = &percent;
+		u64 *psamples = &nr_samples;
 		int i, nr_percent = 1;
 		const char *color;
 		struct annotation *notes = symbol__annotation(sym);
@@ -710,8 +716,10 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
 		if (perf_evsel__is_group_event(evsel)) {
 			nr_percent = evsel->nr_members;
 			ppercents = calloc(nr_percent, sizeof(double));
-			if (ppercents == NULL)
+			psamples = calloc(nr_percent, sizeof(u64));
+			if (ppercents == NULL || psamples == NULL) {
 				return -1;
+			}
 		}
 
 		for (i = 0; i < nr_percent; i++) {
@@ -719,9 +727,10 @@ 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);
+					&path, &nr_samples);
 
 			ppercents[i] = percent;
+			psamples[i] = nr_samples;
 			if (percent > max_percent)
 				max_percent = percent;
 		}
@@ -759,8 +768,14 @@ 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];
 			color = get_percent_color(percent);
-			color_fprintf(stdout, color, " %7.2f", percent);
+
+			if (symbol_conf.show_total_period)
+				color_fprintf(stdout, color, " %7" PRIu64,
+					      nr_samples);
+			else
+				color_fprintf(stdout, color, " %7.2f", percent);
 		}
 
 		printf(" :	");
@@ -770,6 +785,9 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st
 		if (ppercents != &percent)
 			free(ppercents);
 
+		if (psamples != &nr_samples)
+			free(psamples);
+
 	} else if (max_lines && printed >= max_lines)
 		return 1;
 	else {
diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h
index cadbdc9..c8c18ca 100644
--- a/tools/perf/util/annotate.h
+++ b/tools/perf/util/annotate.h
@@ -72,7 +72,7 @@ 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);
+			    s64 end, const char **path, u64 *nr_samples);
 
 struct sym_hist {
 	u64		sum;
@@ -82,6 +82,7 @@ struct sym_hist {
 struct source_line_percent {
 	double		percent;
 	double		percent_sum;
+	double          samples;
 };
 
 struct source_line {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] perf annotate: With --show-total-period, display total # of samples.
  2015-06-19 19:10         ` Arnaldo Carvalho de Melo
@ 2015-06-20 11:53           ` Martin Liška
  2015-06-22 14:38             ` Arnaldo Carvalho de Melo
  0 siblings, 1 reply; 9+ messages in thread
From: Martin Liška @ 2015-06-20 11:53 UTC (permalink / raw)
  To: Arnaldo Carvalho de Melo
  Cc: Ingo Molnar, linux-kernel, Ingo Molnar, Peter Zijlstra,
	Paul Mackerras, Andi Kleen

On 06/19/2015 09:10 PM, Arnaldo Carvalho de Melo wrote:
> Em Fri, Jun 19, 2015 at 11:35:41AM +0200, Martin Liška escreveu:
>> Thank you for the review, all remarks were reasonable.
>> Feel free to comment next version (v3) of the patch.
> 
> Thanks, there was just one missing s/tuples/samples/g, I did it and
> tried running git-am on the attached patch, it failed, so I applied it
> manually.
> 
> - Arnaldo
> 

Hello Arnaldo.

Thank you for help, as I followed the documentation related to Thunderbird,
all spaces and tabs should gone. I still suspect that's culprit of the failure?

Martin
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] perf annotate: With --show-total-period, display total # of samples.
  2015-06-20 11:53           ` Martin Liška
@ 2015-06-22 14:38             ` Arnaldo Carvalho de Melo
  0 siblings, 0 replies; 9+ messages in thread
From: Arnaldo Carvalho de Melo @ 2015-06-22 14:38 UTC (permalink / raw)
  To: Martin Liška
  Cc: Ingo Molnar, linux-kernel, Ingo Molnar, Peter Zijlstra,
	Paul Mackerras, Andi Kleen

Em Sat, Jun 20, 2015 at 01:53:47PM +0200, Martin Liška escreveu:
> On 06/19/2015 09:10 PM, Arnaldo Carvalho de Melo wrote:
> > Em Fri, Jun 19, 2015 at 11:35:41AM +0200, Martin Liška escreveu:
> >> Thank you for the review, all remarks were reasonable.
> >> Feel free to comment next version (v3) of the patch.

> > Thanks, there was just one missing s/tuples/samples/g, I did it and
> > tried running git-am on the attached patch, it failed, so I applied it
> > manually.

> Thank you for help, as I followed the documentation related to Thunderbird,
> all spaces and tabs should gone. I still suspect that's culprit of the failure?

Sometimes it is tricky to figure out exactly what went wrong, so I
rather applied it by hand and just dropped a note, but my workflow, when
submitting patches is:

  $ git format-patch -n --cover-letter tip/perf/core..
  # edit 0000-cover-letter.txt
  $ smi 0*

Where smi is:

  $ cat ~/bin/smi
  git send-email --from "Arnaldo Carvalho de Melo <acme@kernel.org>" \
                 --to "Ingo Molnar <mingo@kernel.org>" \
                 --cc linux-kernel@vger.kernel.org --no-chain-reply-to $*
  $ 

Which I guess is what most contributors do, at least the ones I manage
to use 'git am' on their patches, etc.

I.e. no MUA involved, but of course a MUA can be used, just make sure
that you can use git am on the message you first send to yourself, i.e.
that you try replicating the upstream workflow when trying to apply your
patches :-)

- Arnaldo
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
Please read the FAQ at  http://www.tux.org/lkml/

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2015-06-22 14:38 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-06-18 16:10 [PATCH] perf annotate: With --show-total-period, display total # of samples Martin Liška
2015-06-18 16:26 ` Ingo Molnar
2015-06-18 18:06   ` Martin Liška
2015-06-18 19:15     ` Arnaldo Carvalho de Melo
2015-06-19  9:35       ` Martin Liška
2015-06-19 19:10         ` Arnaldo Carvalho de Melo
2015-06-20 11:53           ` Martin Liška
2015-06-22 14:38             ` Arnaldo Carvalho de Melo
2015-06-19 23:15         ` [tip:perf/core] perf annotate: Display total number of samples with --show-total-period tip-bot for Martin Liška

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.