Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S934203AbdGKWO7 (ORCPT ); Tue, 11 Jul 2017 18:14:59 -0400 Received: from mail-pf0-f194.google.com ([209.85.192.194]:35025 "EHLO mail-pf0-f194.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1756081AbdGKWOU (ORCPT ); Tue, 11 Jul 2017 18:14:20 -0400 From: Taeung Song To: Arnaldo Carvalho de Melo Cc: linux-kernel@vger.kernel.org, Namhyung Kim , Milian Wolff , Jiri Olsa Subject: [PATCH 1/4] perf annotate: Fix wrong --show-total-period option showing number of samples Date: Wed, 12 Jul 2017 07:14:04 +0900 Message-Id: <1499811244-9261-1-git-send-email-treeze.taeung@gmail.com> X-Mailer: git-send-email 2.7.4 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 15736 Lines: 447 Currently the --show-total-period option of perf-annotate is different from perf-report's. For example, perf-report ordinarily shows period and number of samples. # Overhead Samples Period Command Shared Object Symbol # ........ ............ ............ ....... .............. ............ # 9.83% 102 84813704 test test [.] knapsack But --show-total-period of perf-annotate has two problem like below: 1) Wrong column i.e. 'Percent' 2) Show number of samples, not period So fix this option to show period instead of number of samples. Before: Percent | Source code & Disassembly of old for cycles:ppp (102 samples) ----------------------------------------------------------------------------- : : : : Disassembly of section .text: : : 0000000000400816 : : knapsack(): 1 : 400816: push %rbp After: Event count | Source code & Disassembly of old for cycles:ppp (84813704 event count) -------------------------------------------------------------------------------------- : : : : Disassembly of section .text: : : 0000000000400816 : : knapsack(): 743737 : 400816: push %rbp Cc: Namhyung Kim Cc: Milian Wolff Cc: Jiri Olsa Signed-off-by: Taeung Song --- tools/perf/builtin-annotate.c | 4 +- tools/perf/builtin-report.c | 13 +++--- tools/perf/builtin-top.c | 6 ++- tools/perf/ui/browsers/annotate.c | 4 +- tools/perf/ui/gtk/annotate.c | 4 +- tools/perf/util/annotate.c | 92 +++++++++++++++++++++++++++++---------- tools/perf/util/annotate.h | 12 +++-- 7 files changed, 96 insertions(+), 39 deletions(-) diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c index 7a5dc7e..f314661 100644 --- a/tools/perf/builtin-annotate.c +++ b/tools/perf/builtin-annotate.c @@ -177,14 +177,12 @@ static int perf_evsel__add_sample(struct perf_evsel *evsel, */ process_branch_stack(sample->branch_stack, al, sample); - sample->period = 1; sample->weight = 1; - he = hists__add_entry(hists, al, NULL, NULL, NULL, sample, true); if (he == NULL) return -ENOMEM; - ret = hist_entry__inc_addr_samples(he, evsel->idx, al->addr); + ret = hist_entry__inc_addr_samples(he, sample, evsel->idx, al->addr); hists__inc_nr_samples(hists, true); return ret; } diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 79a33eb..5f1894c 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -113,13 +113,14 @@ static int hist_iter__report_callback(struct hist_entry_iter *iter, struct report *rep = arg; struct hist_entry *he = iter->he; struct perf_evsel *evsel = iter->evsel; + struct perf_sample *sample = iter->sample; struct mem_info *mi; struct branch_info *bi; if (!ui__has_annotation()) return 0; - hist__account_cycles(iter->sample->branch_stack, al, iter->sample, + hist__account_cycles(sample->branch_stack, al, sample, rep->nonany_branch_mode); if (sort__mode == SORT_MODE__BRANCH) { @@ -136,14 +137,16 @@ static int hist_iter__report_callback(struct hist_entry_iter *iter, if (err) goto out; - err = hist_entry__inc_addr_samples(he, evsel->idx, al->addr); + err = hist_entry__inc_addr_samples(he, sample, + evsel->idx, al->addr); } else if (symbol_conf.cumulate_callchain) { if (single) - err = hist_entry__inc_addr_samples(he, evsel->idx, - al->addr); + err = hist_entry__inc_addr_samples(he, sample, + evsel->idx, al->addr); } else { - err = hist_entry__inc_addr_samples(he, evsel->idx, al->addr); + err = hist_entry__inc_addr_samples(he, sample, + evsel->idx, al->addr); } out: diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c index 022486d..09885a4 100644 --- a/tools/perf/builtin-top.c +++ b/tools/perf/builtin-top.c @@ -183,6 +183,7 @@ static void ui__warn_map_erange(struct map *map, struct symbol *sym, u64 ip) static void perf_top__record_precise_ip(struct perf_top *top, struct hist_entry *he, + struct perf_sample *sample, int counter, u64 ip) { struct annotation *notes; @@ -199,7 +200,7 @@ static void perf_top__record_precise_ip(struct perf_top *top, if (pthread_mutex_trylock(¬es->lock)) return; - err = hist_entry__inc_addr_samples(he, counter, ip); + err = hist_entry__inc_addr_samples(he, sample, counter, ip); pthread_mutex_unlock(¬es->lock); @@ -671,7 +672,8 @@ static int hist_iter__top_callback(struct hist_entry_iter *iter, struct perf_evsel *evsel = iter->evsel; if (perf_hpp_list.sym && single) - perf_top__record_precise_ip(top, he, evsel->idx, al->addr); + perf_top__record_precise_ip(top, he, iter->sample, + evsel->idx, al->addr); hist__account_cycles(iter->sample->branch_stack, al, iter->sample, !(top->record_opts.branch_stack & PERF_SAMPLE_BRANCH_ANY)); diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c index b376637..73e5ae2 100644 --- a/tools/perf/ui/browsers/annotate.c +++ b/tools/perf/ui/browsers/annotate.c @@ -449,13 +449,13 @@ static void annotate_browser__calc_percent(struct annotate_browser *browser, next = disasm__get_next_ip_line(¬es->src->source, pos); for (i = 0; i < browser->nr_events; i++) { - u64 nr_samples; + u64 nr_samples, period; bpos->samples[i].percent = disasm__calc_percent(notes, evsel->idx + i, pos->offset, next ? next->offset : len, - &path, &nr_samples); + &path, &nr_samples, &period); bpos->samples[i].nr = nr_samples; if (max_percent < bpos->samples[i].percent) diff --git a/tools/perf/ui/gtk/annotate.c b/tools/perf/ui/gtk/annotate.c index 87e3760..d736fd5 100644 --- a/tools/perf/ui/gtk/annotate.c +++ b/tools/perf/ui/gtk/annotate.c @@ -34,10 +34,10 @@ static int perf_gtk__get_percent(char *buf, size_t size, struct symbol *sym, return 0; symhist = annotation__histogram(symbol__annotation(sym), evidx); - if (!symbol_conf.event_group && !symhist->addr[dl->offset]) + if (!symbol_conf.event_group && !symhist->addr[dl->offset].nr_samples) return 0; - percent = 100.0 * symhist->addr[dl->offset] / symhist->sum; + percent = 100.0 * symhist->addr[dl->offset].nr_samples / symhist->sum; markup = perf_gtk__get_percent_color(percent); if (markup) diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index ef434b5..f7aeb5f 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -610,10 +610,10 @@ int symbol__alloc_hist(struct symbol *sym) size_t sizeof_sym_hist; /* Check for overflow when calculating sizeof_sym_hist */ - if (size > (SIZE_MAX - sizeof(struct sym_hist)) / sizeof(u64)) + if (size > (SIZE_MAX - sizeof(struct sym_hist)) / sizeof(struct sym_sample)) return -1; - sizeof_sym_hist = (sizeof(struct sym_hist) + size * sizeof(u64)); + sizeof_sym_hist = (sizeof(struct sym_hist) + size * sizeof(struct sym_sample)); /* Check for overflow in zalloc argument */ if (sizeof_sym_hist > (SIZE_MAX - sizeof(*notes->src)) @@ -714,11 +714,11 @@ static int __symbol__inc_addr_samples(struct symbol *sym, struct map *map, offset = addr - sym->start; h = annotation__histogram(notes, evidx); h->sum++; - h->addr[offset]++; + h->addr[offset].nr_samples++; pr_debug3("%#" PRIx64 " %s: period++ [addr: %#" PRIx64 ", %#" PRIx64 ", evidx=%d] => %" PRIu64 "\n", sym->start, sym->name, - addr, addr - sym->start, evidx, h->addr[offset]); + addr, addr - sym->start, evidx, h->addr[offset].nr_samples); return 0; } @@ -816,9 +816,33 @@ int addr_map_symbol__inc_samples(struct addr_map_symbol *ams, int evidx) return symbol__inc_addr_samples(ams->sym, ams->map, evidx, ams->al_addr); } -int hist_entry__inc_addr_samples(struct hist_entry *he, int evidx, u64 ip) +int hist_entry__inc_addr_samples(struct hist_entry *he, struct perf_sample *sample, + int evidx, u64 addr) { - return symbol__inc_addr_samples(he->ms.sym, he->ms.map, evidx, ip); + struct symbol *sym = he->ms.sym; + struct annotation *notes; + struct sym_hist *h; + s64 offset; + + if (sym == NULL) + return 0; + + notes = symbol__get_annotation(sym, false); + if (notes == NULL) + return -ENOMEM; + + if ((addr < sym->start || addr >= sym->end) && + (addr != sym->end || sym->start != sym->end)) + return -ERANGE; + + offset = addr - sym->start; + h = annotation__histogram(notes, evidx); + h->sum++; + h->addr[offset].nr_samples++; + h->total_period += sample->period; + h->addr[offset].period += sample->period; + + return 0; } static void disasm_line__init_ins(struct disasm_line *dl, struct arch *arch, struct map *map) @@ -928,11 +952,13 @@ struct disasm_line *disasm__get_next_ip_line(struct list_head *head, struct disa } double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset, - s64 end, const char **path, u64 *nr_samples) + s64 end, const char **path, u64 *nr_samples, u64 *period) { struct source_line *src_line = notes->src->lines; double percent = 0.0; + *nr_samples = 0; + *period = 0; if (src_line) { size_t sizeof_src_line = sizeof(*src_line) + @@ -952,12 +978,16 @@ double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset, } else { struct sym_hist *h = annotation__histogram(notes, evidx); unsigned int hits = 0; + unsigned int p = 0; - while (offset < end) - hits += h->addr[offset++]; + while (offset < end) { + hits += h->addr[offset].nr_samples; + p += h->addr[offset++].period; + } if (h->sum) { *nr_samples = hits; + *period = p; percent = 100.0 * hits / h->sum; } } @@ -1057,10 +1087,11 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st if (dl->offset != -1) { const char *path = NULL; - u64 nr_samples; + u64 nr_samples, period; double percent, max_percent = 0.0; double *ppercents = &percent; u64 *psamples = &nr_samples; + u64 *pperiod = . int i, nr_percent = 1; const char *color; struct annotation *notes = symbol__annotation(sym); @@ -1075,9 +1106,9 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st nr_percent = evsel->nr_members; ppercents = calloc(nr_percent, sizeof(double)); psamples = calloc(nr_percent, sizeof(u64)); - if (ppercents == NULL || psamples == NULL) { + pperiod = calloc(nr_percent, sizeof(u64)); + if (ppercents == NULL || psamples == NULL || pperiod == NULL) return -1; - } } for (i = 0; i < nr_percent; i++) { @@ -1085,10 +1116,11 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st notes->src->lines ? i : evsel->idx + i, offset, next ? next->offset : (s64) len, - &path, &nr_samples); + &path, &nr_samples, &period); ppercents[i] = percent; psamples[i] = nr_samples; + pperiod[i] = period; if (percent > max_percent) max_percent = percent; } @@ -1127,11 +1159,12 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st for (i = 0; i < nr_percent; i++) { percent = ppercents[i]; nr_samples = psamples[i]; + period = pperiod[i]; color = get_percent_color(percent); if (symbol_conf.show_total_period) - color_fprintf(stdout, color, " %7" PRIu64, - nr_samples); + color_fprintf(stdout, color, " %11" PRIu64, + period); else color_fprintf(stdout, color, " %7.2f", percent); } @@ -1150,6 +1183,9 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st if (psamples != &nr_samples) free(psamples); + if (pperiod != &period) + free(pperiod); + } else if (max_lines && printed >= max_lines) return 1; else { @@ -1161,6 +1197,10 @@ static int disasm_line__print(struct disasm_line *dl, struct symbol *sym, u64 st if (perf_evsel__is_group_event(evsel)) width *= evsel->nr_members; + if (symbol_conf.show_total_period) + width += perf_evsel__is_group_event(evsel) ? + 4 * evsel->nr_members : 4; + if (!*dl->line) printf(" %*s:\n", width, " "); else @@ -1702,7 +1742,7 @@ static int symbol__get_source_line(struct symbol *sym, struct map *map, double percent = 0.0; h = annotation__histogram(notes, evidx + k); - nr_samples = h->addr[i]; + nr_samples = h->addr[i].nr_samples; if (h->sum) percent = 100.0 * nr_samples / h->sum; @@ -1773,9 +1813,9 @@ static void symbol__annotate_hits(struct symbol *sym, struct perf_evsel *evsel) u64 len = symbol__size(sym), offset; for (offset = 0; offset < len; ++offset) - if (h->addr[offset] != 0) + if (h->addr[offset].nr_samples != 0) printf("%*" PRIx64 ": %" PRIu64 "\n", BITS_PER_LONG / 2, - sym->start + offset, h->addr[offset]); + sym->start + offset, h->addr[offset].nr_samples); printf("%*s: %" PRIu64 "\n", BITS_PER_LONG / 2, "h->sum", h->sum); } @@ -1811,8 +1851,16 @@ int symbol__annotate_printf(struct symbol *sym, struct map *map, if (perf_evsel__is_group_event(evsel)) width *= evsel->nr_members; - graph_dotted_len = printf(" %-*.*s| Source code & Disassembly of %s for %s (%" PRIu64 " samples)\n", - width, width, "Percent", d_filename, evsel_name, h->sum); + if (symbol_conf.show_total_period) + width += perf_evsel__is_group_event(evsel) ? + 4 * evsel->nr_members : 4; + + graph_dotted_len = printf(" %-*.*s| Source code & Disassembly of %s for %s (%" PRIu64 " %s)\n", + width, width, + symbol_conf.show_total_period ? "Event count" : "Percent", + d_filename, evsel_name, + symbol_conf.show_total_period ? h->total_period : h->sum, + symbol_conf.show_total_period ? "event count" : "samples"); printf("%-*.*s----\n", graph_dotted_len, graph_dotted_len, graph_dotted_line); @@ -1878,8 +1926,8 @@ void symbol__annotate_decay_histogram(struct symbol *sym, int evidx) h->sum = 0; for (offset = 0; offset < len; ++offset) { - h->addr[offset] = h->addr[offset] * 7 / 8; - h->sum += h->addr[offset]; + h->addr[offset].nr_samples = h->addr[offset].nr_samples * 7 / 8; + h->sum += h->addr[offset].nr_samples; } } diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h index bac698d..6b2e645 100644 --- a/tools/perf/util/annotate.h +++ b/tools/perf/util/annotate.h @@ -79,11 +79,16 @@ struct disasm_line *disasm__get_next_ip_line(struct list_head *head, struct disa int disasm_line__scnprintf(struct disasm_line *dl, char *bf, size_t size, bool raw); size_t disasm__fprintf(struct list_head *head, FILE *fp); double disasm__calc_percent(struct annotation *notes, int evidx, s64 offset, - s64 end, const char **path, u64 *nr_samples); + s64 end, const char **path, u64 *nr_samples, u64 *period); +struct sym_sample { + u64 nr_samples; + u64 period; +}; struct sym_hist { u64 sum; - u64 addr[0]; + u64 total_period; + struct sym_sample addr[0]; }; struct cyc_hist { @@ -155,7 +160,8 @@ int addr_map_symbol__account_cycles(struct addr_map_symbol *ams, struct addr_map_symbol *start, unsigned cycles); -int hist_entry__inc_addr_samples(struct hist_entry *he, int evidx, u64 addr); +int hist_entry__inc_addr_samples(struct hist_entry *he, struct perf_sample *sample, + int evidx, u64 addr); int symbol__alloc_hist(struct symbol *sym); void symbol__annotate_zero_histograms(struct symbol *sym); -- 2.7.4