2015-06-18 16:10:31

by Martin Liška

[permalink] [raw]
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.

Signed-off-by: Martin Liska <[email protected]>
---
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


2015-06-18 16:26:34

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] perf annotate: With --show-total-period, display total # of samples.


* Martin Liška <[email protected]> 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 <[email protected]>
> ---
> 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

2015-06-18 18:06:35

by Martin Liška

[permalink] [raw]
Subject: Re: [PATCH] perf annotate: With --show-total-period, display total # of samples.

On 06/18/2015 06:26 PM, Ingo Molnar wrote:
>
> * Martin Liška <[email protected]> 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 <[email protected]>
>> ---
>> 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


Attachments:
0001-perf-annotate-With-show-total-period-display-total-o-v2.patch (10.76 kB)

2015-06-18 19:15:50

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf annotate: With --show-total-period, display total # of samples.

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

> Signed-off-by: Martin Liska <[email protected]>
> ---
> 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
>

2015-06-19 09:35:54

by Martin Liška

[permalink] [raw]
Subject: Re: [PATCH] perf annotate: With --show-total-period, display total # of samples.

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 <[email protected]> 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 <[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.
>
>> Signed-off-by: Martin Liska <[email protected]>
>> ---
>> 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


Attachments:
0001-perf-annotate-With-show-total-period-display-total-o-v3.patch (9.61 kB)

2015-06-19 19:10:49

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf annotate: With --show-total-period, display total # of samples.

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

Subject: [tip:perf/core] 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.

Signed-off-by: Martin Liska <[email protected]>
Cc: Andi Kleen <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
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 {

2015-06-20 11:54:17

by Martin Liška

[permalink] [raw]
Subject: Re: [PATCH] perf annotate: With --show-total-period, display total # of samples.

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

2015-06-22 14:38:36

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf annotate: With --show-total-period, display total # of samples.

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 <[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