Em Sat, Mar 18, 2017 at 10:49:28PM +0100, Milian Wolff escreveu: > Often it is interesting to know how costly a given source line is in > total. Previously, one had to build these sums manually based on all > addresses that pointed to the same source line. This patch introduces > srcline as a sort key, which will do the aggregation for us. > > Paired with the recent addition of showing inline frames, this makes > perf report much more useful for many C++ work loads. > > The following shows the new feature in action. First, let's show the > status quo output when we sort by address. The result contains many > hist entries that generate the same output:
Looks ok, one pet peeve below > ~~~~~~~~~~~~~~~~ > $ perf report --stdio --inline -g address > # Children Self Command Shared Object Symbol > # ........ ........ ............ ................... > ......................................... > # > 99.89% 35.34% cpp-inlining cpp-inlining [.] main > | > |--64.55%--main complex:655 > | > /home/milian/projects/kdab/rnd/hotspot/tests/test-clients/cpp-inlining/main.cpp:39 > (inline) > | /usr/include/c++/6.3.1/complex:664 (inline) > | | > | |--60.31%--hypot +20 > | | | > | | |--8.52%--__hypot_finite +273 > | | | > | | |--7.32%--__hypot_finite +411 > ... > --35.34%--_start +4194346 > __libc_start_main +241 > | > |--6.65%--main random.tcc:3326 > | > /home/milian/projects/kdab/rnd/hotspot/tests/test-clients/cpp-inlining/main.cpp:39 > (inline) > | /usr/include/c++/6.3.1/bits/random.h:1809 > (inline) > | /usr/include/c++/6.3.1/bits/random.h:1818 > (inline) > | /usr/include/c++/6.3.1/bits/random.h:185 > (inline) > | > |--2.70%--main random.tcc:3326 > | > /home/milian/projects/kdab/rnd/hotspot/tests/test-clients/cpp-inlining/main.cpp:39 > (inline) > | /usr/include/c++/6.3.1/bits/random.h:1809 > (inline) > | /usr/include/c++/6.3.1/bits/random.h:1818 > (inline) > | /usr/include/c++/6.3.1/bits/random.h:185 > (inline) > | > |--1.69%--main random.tcc:3326 > | > /home/milian/projects/kdab/rnd/hotspot/tests/test-clients/cpp-inlining/main.cpp:39 > (inline) > | /usr/include/c++/6.3.1/bits/random.h:1809 > (inline) > | /usr/include/c++/6.3.1/bits/random.h:1818 > (inline) > | /usr/include/c++/6.3.1/bits/random.h:185 > (inline) > ... > ~~~~~~~~~~~~~~~~ > > With this patch and `-g srcline` we instead get the following output: > > ~~~~~~~~~~~~~~~~ > $ perf report --stdio --inline -g srcline > # Children Self Command Shared Object Symbol > # ........ ........ ............ ................... > ......................................... > # > 99.89% 35.34% cpp-inlining cpp-inlining [.] main > | > |--64.55%--main complex:655 > | > /home/milian/projects/kdab/rnd/hotspot/tests/test-clients/cpp-inlining/main.cpp:39 > (inline) > | /usr/include/c++/6.3.1/complex:664 (inline) > | | > | |--64.02%--hypot > | | | > | | --59.81%--__hypot_finite > | | > | --0.53%--cabs > | > --35.34%--_start > __libc_start_main > | > |--12.48%--main random.tcc:3326 > | > /home/milian/projects/kdab/rnd/hotspot/tests/test-clients/cpp-inlining/main.cpp:39 > (inline) > | /usr/include/c++/6.3.1/bits/random.h:1809 > (inline) > | /usr/include/c++/6.3.1/bits/random.h:1818 > (inline) > | /usr/include/c++/6.3.1/bits/random.h:185 > (inline) > ... > ~~~~~~~~~~~~~~~~ > > Signed-off-by: Milian Wolff <[email protected]> > --- > tools/perf/Documentation/perf-report.txt | 1 + > tools/perf/ui/browsers/hists.c | 3 +- > tools/perf/ui/stdio/hist.c | 3 +- > tools/perf/util/annotate.c | 3 +- > tools/perf/util/callchain.c | 52 > +++++++++++++++++++++++++++++--- > tools/perf/util/callchain.h | 3 +- > tools/perf/util/map.c | 3 +- > tools/perf/util/sort.c | 16 ++++++---- > tools/perf/util/srcline.c | 11 +++++-- > tools/perf/util/util.h | 4 +-- > 10 files changed, 78 insertions(+), 21 deletions(-) > > diff --git a/tools/perf/Documentation/perf-report.txt > b/tools/perf/Documentation/perf-report.txt > index 248bba434b53..37a175914157 100644 > --- a/tools/perf/Documentation/perf-report.txt > +++ b/tools/perf/Documentation/perf-report.txt > @@ -235,6 +235,7 @@ OPTIONS > sort_key can be: > - function: compare on functions (default) > - address: compare on individual code addresses > + - srcline: compare on source filename and line number > > branch can be: > - branch: include last branch information in callgraph when available. > diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c > index 757222bc9dad..bca08be9fd78 100644 > --- a/tools/perf/ui/browsers/hists.c > +++ b/tools/perf/ui/browsers/hists.c > @@ -851,7 +851,8 @@ static int hist_browser__show_inline(struct hist_browser > *browser, > if (ui_browser__is_current_entry(&browser->b, row)) > color = HE_COLORSET_SELECTED; > > - if (callchain_param.key == CCKEY_ADDRESS) { > + if (callchain_param.key == CCKEY_ADDRESS || > + callchain_param.key == CCKEY_SRCLINE) { > if (ilist->filename != NULL) > scnprintf(buf, sizeof(buf), > "%s:%d (inline)", > diff --git a/tools/perf/ui/stdio/hist.c b/tools/perf/ui/stdio/hist.c > index 183470f25100..bf95b4a9a592 100644 > --- a/tools/perf/ui/stdio/hist.c > +++ b/tools/perf/ui/stdio/hist.c > @@ -53,7 +53,8 @@ static size_t inline__fprintf(struct map *map, u64 ip, int > left_margin, > ret += fprintf(fp, " "); > } > > - if (callchain_param.key == CCKEY_ADDRESS) { > + if (callchain_param.key == CCKEY_ADDRESS || > + callchain_param.key == CCKEY_SRCLINE) { > if (ilist->filename != NULL) > ret += fprintf(fp, "%s:%d (inline)", > ilist->filename, > diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c > index 273f21fa32b5..e7194ff88c4f 100644 > --- a/tools/perf/util/annotate.c > +++ b/tools/perf/util/annotate.c > @@ -1668,7 +1668,8 @@ static int symbol__get_source_line(struct symbol *sym, > struct map *map, > goto next; > > offset = start + i; > - src_line->path = get_srcline(map->dso, offset, NULL, false); > + src_line->path = get_srcline(map->dso, offset, NULL, > + false, true); > insert_source_line(&tmp_root, src_line); > > next: > diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c > index aba953421a03..d78776a20e80 100644 > --- a/tools/perf/util/callchain.c > +++ b/tools/perf/util/callchain.c > @@ -80,6 +80,10 @@ static int parse_callchain_sort_key(const char *value) > callchain_param.key = CCKEY_ADDRESS; > return 0; > } > + if (!strncmp(value, "srcline", strlen(value))) { > + callchain_param.key = CCKEY_SRCLINE; > + return 0; > + } > if (!strncmp(value, "branch", strlen(value))) { > callchain_param.branch_callstack = 1; > return 0; > @@ -510,14 +514,51 @@ enum match_result { > MATCH_GT, > }; > > +static enum match_result match_chain_srcline(struct callchain_cursor_node > *node, > + struct callchain_list *cnode) > +{ > + char *left = get_srcline(cnode->ms.map->dso, > + map__rip_2objdump(cnode->ms.map, cnode->ip), > + cnode->ms.sym, true, false); > + char *right = get_srcline(node->map->dso, > + map__rip_2objdump(node->map, node->ip), > + node->sym, true, false); > + enum match_result ret = MATCH_EQ; > + int cmp; > + > + if (left && right) > + cmp = strcmp(left, right); > + else if (!left && right) > + cmp = 1; > + else if (left && !right) > + cmp = -1; > + else if (cnode->ip == node->ip) > + cmp = 0; > + else > + cmp = (cnode->ip < node->ip) ? -1 : 1; > + > + if (cmp != 0) > + ret = cmp < 0 ? MATCH_LT : MATCH_GT; > + > + free_srcline(left); > + free_srcline(right); > + return ret; > +} > + > static enum match_result match_chain(struct callchain_cursor_node *node, > struct callchain_list *cnode) > { > struct symbol *sym = node->sym; > u64 left, right; > > - if (cnode->ms.sym && sym && > - callchain_param.key == CCKEY_FUNCTION) { > + if (callchain_param.key == CCKEY_SRCLINE) { > + enum match_result match = match_chain_srcline(node, cnode); > + > + if (match != MATCH_ERROR) > + return match; > + } > + > + if (cnode->ms.sym && sym && callchain_param.key == CCKEY_FUNCTION) { The above line is the same as it was in those first two removed, its just churn :-\ I.e. this part: - if (cnode->ms.sym && sym && - callchain_param.key == CCKEY_FUNCTION) { + if (cnode->ms.sym && sym && callchain_param.key == CCKEY_FUNCTION) { Please avoid doing that in the future. > left = cnode->ms.sym->start; > right = sym->start; > } else { > @@ -911,15 +952,16 @@ int fill_callchain_info(struct addr_location *al, > struct callchain_cursor_node * > char *callchain_list__sym_name(struct callchain_list *cl, > char *bf, size_t bfsize, bool show_dso) > { > + bool show_addr = callchain_param.key == CCKEY_ADDRESS; > + bool show_srcline = show_addr || callchain_param.key == CCKEY_SRCLINE; > int printed; > > if (cl->ms.sym) { > - if (callchain_param.key == CCKEY_ADDRESS && > - cl->ms.map && !cl->srcline) > + if (show_srcline && cl->ms.map && !cl->srcline) > cl->srcline = get_srcline(cl->ms.map->dso, > map__rip_2objdump(cl->ms.map, > cl->ip), > - cl->ms.sym, false); > + cl->ms.sym, false, show_addr); > if (cl->srcline) > printed = scnprintf(bf, bfsize, "%s %s", > cl->ms.sym->name, cl->srcline); > diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h > index 4f4b60f1558a..c56c23dbbf72 100644 > --- a/tools/perf/util/callchain.h > +++ b/tools/perf/util/callchain.h > @@ -77,7 +77,8 @@ typedef void (*sort_chain_func_t)(struct rb_root *, struct > callchain_root *, > > enum chain_key { > CCKEY_FUNCTION, > - CCKEY_ADDRESS > + CCKEY_ADDRESS, > + CCKEY_SRCLINE > }; > > enum chain_value { > diff --git a/tools/perf/util/map.c b/tools/perf/util/map.c > index 1d9ebcf9e38e..c1870ac365a3 100644 > --- a/tools/perf/util/map.c > +++ b/tools/perf/util/map.c > @@ -405,7 +405,8 @@ int map__fprintf_srcline(struct map *map, u64 addr, const > char *prefix, > > if (map && map->dso) { > srcline = get_srcline(map->dso, > - map__rip_2objdump(map, addr), NULL, true); > + map__rip_2objdump(map, addr), NULL, > + true, true); > if (srcline != SRCLINE_UNKNOWN) > ret = fprintf(fp, "%s%s", prefix, srcline); > free_srcline(srcline); > diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c > index 8b0d4e39f640..73f3ec1cf2a0 100644 > --- a/tools/perf/util/sort.c > +++ b/tools/perf/util/sort.c > @@ -323,7 +323,7 @@ char *hist_entry__get_srcline(struct hist_entry *he) > return SRCLINE_UNKNOWN; > > return get_srcline(map->dso, map__rip_2objdump(map, he->ip), > - he->ms.sym, true); > + he->ms.sym, true, true); > } > > static int64_t > @@ -366,7 +366,8 @@ sort__srcline_from_cmp(struct hist_entry *left, struct > hist_entry *right) > left->branch_info->srcline_from = get_srcline(map->dso, > map__rip_2objdump(map, > > left->branch_info->from.al_addr), > - > left->branch_info->from.sym, true); > + > left->branch_info->from.sym, > + true, true); > } > if (!right->branch_info->srcline_from) { > struct map *map = right->branch_info->from.map; > @@ -376,7 +377,8 @@ sort__srcline_from_cmp(struct hist_entry *left, struct > hist_entry *right) > right->branch_info->srcline_from = get_srcline(map->dso, > map__rip_2objdump(map, > > right->branch_info->from.al_addr), > - > right->branch_info->from.sym, true); > + > right->branch_info->from.sym, > + true, true); > } > return strcmp(right->branch_info->srcline_from, > left->branch_info->srcline_from); > } > @@ -407,7 +409,8 @@ sort__srcline_to_cmp(struct hist_entry *left, struct > hist_entry *right) > left->branch_info->srcline_to = get_srcline(map->dso, > map__rip_2objdump(map, > > left->branch_info->to.al_addr), > - > left->branch_info->from.sym, true); > + > left->branch_info->from.sym, > + true, true); > } > if (!right->branch_info->srcline_to) { > struct map *map = right->branch_info->to.map; > @@ -417,7 +420,8 @@ sort__srcline_to_cmp(struct hist_entry *left, struct > hist_entry *right) > right->branch_info->srcline_to = get_srcline(map->dso, > map__rip_2objdump(map, > > right->branch_info->to.al_addr), > - > right->branch_info->to.sym, true); > + right->branch_info->to.sym, > + true, true); > } > return strcmp(right->branch_info->srcline_to, > left->branch_info->srcline_to); > } > @@ -448,7 +452,7 @@ static char *hist_entry__get_srcfile(struct hist_entry *e) > return no_srcfile; > > sf = __get_srcline(map->dso, map__rip_2objdump(map, e->ip), > - e->ms.sym, false, true); > + e->ms.sym, false, true, true); > if (!strcmp(sf, SRCLINE_UNKNOWN)) > return no_srcfile; > p = strchr(sf, ':'); > diff --git a/tools/perf/util/srcline.c b/tools/perf/util/srcline.c > index f9d4b47d1fb5..6f8651104990 100644 > --- a/tools/perf/util/srcline.c > +++ b/tools/perf/util/srcline.c > @@ -427,7 +427,7 @@ static struct inline_node *addr2inlines(const char > *dso_name, u64 addr, > #define A2L_FAIL_LIMIT 123 > > char *__get_srcline(struct dso *dso, u64 addr, struct symbol *sym, > - bool show_sym, bool unwind_inlines) > + bool show_sym, bool show_addr, bool unwind_inlines) > { > char *file = NULL; > unsigned line = 0; > @@ -461,6 +461,11 @@ char *__get_srcline(struct dso *dso, u64 addr, struct > symbol *sym, > dso->has_srcline = 0; > dso__free_a2l(dso); > } > + > + if (!show_addr) > + return (show_sym && sym) ? > + strndup(sym->name, sym->namelen) : NULL; > + > if (sym) { > if (asprintf(&srcline, "%s+%" PRIu64, show_sym ? sym->name : "", > addr - sym->start) < 0) > @@ -477,9 +482,9 @@ void free_srcline(char *srcline) > } > > char *get_srcline(struct dso *dso, u64 addr, struct symbol *sym, > - bool show_sym) > + bool show_sym, bool show_addr) > { > - return __get_srcline(dso, addr, sym, show_sym, false); > + return __get_srcline(dso, addr, sym, show_sym, show_addr, false); > } > > struct inline_node *dso__parse_addr_inlines(struct dso *dso, u64 addr) > diff --git a/tools/perf/util/util.h b/tools/perf/util/util.h > index cc0700d6fef0..7cf5752b38fd 100644 > --- a/tools/perf/util/util.h > +++ b/tools/perf/util/util.h > @@ -287,9 +287,9 @@ struct symbol; > > extern bool srcline_full_filename; > char *get_srcline(struct dso *dso, u64 addr, struct symbol *sym, > - bool show_sym); > + bool show_sym, bool show_addr); > char *__get_srcline(struct dso *dso, u64 addr, struct symbol *sym, > - bool show_sym, bool unwind_inlines); > + bool show_sym, bool show_addr, bool unwind_inlines); > void free_srcline(char *srcline); > > int perf_event_paranoid(void); > -- > 2.12.0
