Skip to contentSkip to conversation

perf annotate: With --show-total-period, display total # of samples.

PATCH In review Martin Liška 4 participants 9 messages Jun 18, 2015 – Jun 22, 2015 perf events

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 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.

---
 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(-)
tools/perf/builtin-annotate.c +2 -0
index 4e08c2d..2c1bec3 100644
@@ -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();
tools/perf/ui/browsers/annotate.c +28 -16

89 lines not on this page · raw message

tools/perf/util/annotate.c +22 -6
index bf80430..8b94603 100644
@@ -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 {
tools/perf/util/annotate.h +2 -1
index cadbdc9..752d1bd 100644
@@ -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 {
Signature · 1 line
2.1.4
#1 lore raw
Quoted 17 lines · Martin Liška

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.

---
 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(-)
tools/perf/builtin-annotate.c +0 -0
index 4e08c2d..2c1bec3 100644

tools/perf/builtin-annotate.c int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused) · line 334

Quoted 6 lines
tools/perf/builtin-annotate.c +2 -0
@@ -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

#2 lore raw
Quoted 30 lines · Ingo Molnar
Quoted 17 lines · Martin Liška

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.

---
 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(-)
tools/perf/builtin-annotate.c +0 -0
index 4e08c2d..2c1bec3 100644

tools/perf/builtin-annotate.c int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused) · line 334

Quoted 6 lines
tools/perf/builtin-annotate.c +2 -0
@@ -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)

Quoted 4 lines

Thanks,

Ingo

Hello.

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

Thanks,
Martin

Quoted 1 line

From 98e1998fcd7481c7e1756e643d66eb85559849ac Mon Sep 17 00:00:00 2001

From: mliska <[email protected]>
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.

---
 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(-)
tools/perf/builtin-annotate.c +4 -1
index 4e08c2d..b0c442e 100644
@@ -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();
tools/perf/ui/browsers/annotate.c +42 -18

140 lines not on this page · raw message

tools/perf/util/annotate.c +22 -6
index bf80430..8b94603 100644
@@ -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 {
tools/perf/util/annotate.h +2 -1
index cadbdc9..752d1bd 100644
@@ -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 {
tools/perf/util/hist.h +2 -1
index 5ed8d9c..65f953f 100644
@@ -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,
Signature · 1 line
2.1.4
#3 lore raw

Em Thu, Jun 18, 2015 at 08:06:25PM +0200, Martin Liška escreveu:

Quoted 38 lines · Ingo Molnar
Quoted 8 lines · Martin Liška

tools/perf/builtin-annotate.c int cmd_annotate(int argc, const char **argv, const char *prefix __maybe_unused) · line 335

Quoted 4 lines

+++ b/tools/perf/builtin-annotate.c

tools/perf/builtin-annotate.c +2 -0
+	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.

Quoted 1 line

From 98e1998fcd7481c7e1756e643d66eb85559849ac Mon Sep 17 00:00:00 2001

From: mliska <[email protected]>
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.

---
 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(-)
tools/perf/builtin-annotate.c +0 -0
index 4e08c2d..b0c442e 100644

tools/perf/builtin-annotate.c find_next: · line 165

Quoted 6 lines
tools/perf/builtin-annotate.c +2 -1
@@ -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()?

Quoted 17 lines
tools/perf/builtin-annotate.c +2 -0
 			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();
tools/perf/ui/browsers/annotate.c +0 -0
index acb0e23..e7cd596 100644

tools/perf/ui/browsers/annotate.c line 16

Quoted 6 lines
tools/perf/ui/browsers/annotate.c +3 -0
@@ -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...

Quoted 7 lines
tools/perf/ui/browsers/annotate.c +2 -4
+};
+
 struct browser_disasm_line {
-	struct rb_node	rb_node;
-	u32		idx;
-	int		idx_asm;
-	int		jump_sources;

tools/perf/ui/browsers/annotate.c line 28

Quoted 10 lines
tools/perf/ui/browsers/annotate.c +5 -1
+	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.

Quoted 14 lines
tools/perf/ui/browsers/annotate.c +2 -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,

tools/perf/ui/browsers/annotate.c static void annotate_browser__write(struct ui_browser *browser, void *entry, int · line 115

Quoted 7 lines
tools/perf/ui/browsers/annotate.c +2 -2
@@ -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.

Quoted 207 lines
tools/perf/ui/browsers/annotate.c +28 -10
 	}
 
 	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
tools/perf/util/annotate.c +22 -6
index bf80430..8b94603 100644
@@ -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 {
tools/perf/util/annotate.h +0 -0
index cadbdc9..752d1bd 100644

tools/perf/util/annotate.h struct disasm_line *disasm__get_next_ip_line(struct list_head *head, struct disa · line 75

Quoted 5 lines
tools/perf/util/annotate.h +1 -1
@@ -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

tools/perf/util/annotate.h struct sym_hist { · line 82

Quoted 5 lines
tools/perf/util/annotate.h +0 -0
 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'

tools/perf/util/annotate.h struct sym_hist { · line 85

Quoted 3 lines
tools/perf/util/annotate.h +1 -0
 	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;
};
Quoted 8 lines
tools/perf/util/annotate.h +0 -0
 };
 
 struct source_line {
tools/perf/util/hist.h +0 -0
index 5ed8d9c..65f953f 100644

tools/perf/util/hist.h int map_symbol__tui_annotate(struct map_symbol *ms, struct perf_evsel *evsel, · line 310

Quoted 6 lines
tools/perf/util/hist.h +2 -1
@@ -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?

Quoted 6 lines
tools/perf/util/hist.h +0 -0
 int perf_evlist__tui_browse_hists(struct perf_evlist *evlist, const char *help,
 				  struct hist_browser_timer *hbt,
Signature · 1 line
2.1.4
#4 lore raw
Quoted 383 lines · Arnaldo Carvalho de Melo

Em Thu, Jun 18, 2015 at 08:06:25PM +0200, Martin Liška escreveu:

Quoted 38 lines · Ingo Molnar
Quoted 8 lines · Martin Liška

Review

Quoted 4 lines

+++ b/tools/perf/builtin-annotate.c

(quoted diff) +2 -0
+	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.

Quoted 1 line

From 98e1998fcd7481c7e1756e643d66eb85559849ac Mon Sep 17 00:00:00 2001

From: mliska <[email protected]>
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.

---
 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(-)
tools/perf/builtin-annotate.c +0 -0
index 4e08c2d..b0c442e 100644

tools/perf/builtin-annotate.c find_next: · line 165

Quoted 6 lines
tools/perf/builtin-annotate.c +2 -1
@@ -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()?

Quoted 17 lines
tools/perf/builtin-annotate.c +2 -0
 			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();
tools/perf/ui/browsers/annotate.c +0 -0
index acb0e23..e7cd596 100644

tools/perf/ui/browsers/annotate.c line 16

Quoted 6 lines
tools/perf/ui/browsers/annotate.c +3 -0
@@ -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...

Quoted 7 lines
tools/perf/ui/browsers/annotate.c +2 -4
+};
+
 struct browser_disasm_line {
-	struct rb_node	rb_node;
-	u32		idx;
-	int		idx_asm;
-	int		jump_sources;

tools/perf/ui/browsers/annotate.c line 28

Quoted 10 lines
tools/perf/ui/browsers/annotate.c +5 -1
+	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.

Quoted 14 lines
tools/perf/ui/browsers/annotate.c +2 -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,

tools/perf/ui/browsers/annotate.c static void annotate_browser__write(struct ui_browser *browser, void *entry, int · line 115

Quoted 7 lines
tools/perf/ui/browsers/annotate.c +2 -2
@@ -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.

Quoted 207 lines
tools/perf/ui/browsers/annotate.c +28 -10
 	}
 
 	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
tools/perf/util/annotate.c +22 -6
index bf80430..8b94603 100644
@@ -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 {
tools/perf/util/annotate.h +0 -0
index cadbdc9..752d1bd 100644

tools/perf/util/annotate.h struct disasm_line *disasm__get_next_ip_line(struct list_head *head, struct disa · line 75

Quoted 5 lines
tools/perf/util/annotate.h +1 -1
@@ -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

tools/perf/util/annotate.h struct sym_hist { · line 82

Quoted 5 lines
tools/perf/util/annotate.h +0 -0
 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'

tools/perf/util/annotate.h struct sym_hist { · line 85

Quoted 3 lines
tools/perf/util/annotate.h +1 -0
 	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;
};
Quoted 10 lines
tools/perf/util/annotate.h +0 -0
 };
 
 struct source_line {
tools/perf/util/hist.h +0 -0
index 5ed8d9c..65f953f 100644
@@ -306,7 +306,8 @@ int map_symbol__tui_annotate(struct map_symbol *ms, struct perf_evsel *evsel,
 			     struct hist_browser_timer *hbt);
 
Quoted 15 lines

tools/perf/util/hist.h int map_symbol__tui_annotate(struct map_symbol *ms, struct perf_evsel *evsel, · line 310

Quoted 4 lines
tools/perf/util/hist.h +2 -1
 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?

Quoted 6 lines
tools/perf/util/hist.h +0 -0
 int perf_evlist__tui_browse_hists(struct perf_evlist *evlist, const char *help,
 				  struct hist_browser_timer *hbt,
Signature · 1 line
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

Quoted 1 line

From 1c0d49462a82f1085de3675e20c28d8445cd493e Mon Sep 17 00:00:00 2001

From: mliska <[email protected]>
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.

---
 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(-)
tools/perf/builtin-annotate.c +2 -0
index 4e08c2d..2c1bec3 100644
@@ -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();
tools/perf/ui/browsers/annotate.c +43 -17

135 lines not on this page · raw message

tools/perf/util/annotate.c +23 -5
index bf80430..12914b6 100644
@@ -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 {
tools/perf/util/annotate.h +2 -1
index cadbdc9..c8c18ca 100644
@@ -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 {
Signature · 1 line
2.1.4
#5 lore raw

Em Fri, Jun 19, 2015 at 11:35:41AM +0200, Martin Liška escreveu:

Quoted 2 lines

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/

#6 lore raw
Quoted 10 lines · Arnaldo Carvalho de Melo

Em Fri, Jun 19, 2015 at 11:35:41AM +0200, Martin Liška escreveu:

Quoted 2 lines

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/

#7 lore raw

Em Sat, Jun 20, 2015 at 01:53:47PM +0200, Martin Liška escreveu:

Quoted 11 lines · Arnaldo Carvalho de Melo
Quoted 7 lines

Em Fri, Jun 19, 2015 at 11:35:41AM +0200, Martin Liška escreveu:

Quoted 2 lines

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 <[email protected]>" \
                 --to "Ingo Molnar <[email protected]>" \
                 --cc [email protected] --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/

#8 lore raw

[tip:perf/core] perf annotate: Display total number of samples with --show-total-period

In reply to Martin Liška
perf annotate: Display total number of samples with --show-total-period
Commit-ID:  0c4a5bcea4609948375173cdea8d73783110a75e
Gitweb:     http://git.kernel.org/tip/0c4a5bcea4609948375173cdea8d73783110a75e
Author:     Martin Liška <[email protected]>
AuthorDate: Fri, 19 Jun 2015 16:10:43 -0300
Committer:  Arnaldo Carvalho de Melo <[email protected]>
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.

---
 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(-)
tools/perf/builtin-annotate.c +2 -0
index 4e08c2d..2c1bec3 100644
@@ -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();
tools/perf/ui/browsers/annotate.c +43 -17

135 lines not on this page · raw message

tools/perf/util/annotate.c +23 -5
index bf80430..12914b6 100644
@@ -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 {
tools/perf/util/annotate.h +2 -1
index cadbdc9..c8c18ca 100644
@@ -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/