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(¬es->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 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
