* [PATCH 0/2] perf report/annotate: Support average IPC and IPC coverage for function @ 2018-11-26 9:40 Jin Yao 2018-11-26 9:40 ` [PATCH 1/2] perf annotate: Compute average IPC and IPC coverage per symbol Jin Yao 2018-11-26 9:40 ` [PATCH 2/2] perf report: Display " Jin Yao 0 siblings, 2 replies; 10+ messages in thread From: Jin Yao @ 2018-11-26 9:40 UTC (permalink / raw) To: acme, jolsa, peterz, mingo, alexander.shishkin Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao Add supporting of displaying the average IPC and IPC coverage percentage per function. For example, $ perf record -g -b ... $ perf report -s symbol,ipc Overhead Symbol IPC [IPC Coverage] 39.60% [.] __random 2.30 [ 54.8%] 18.02% [.] main 0.43 [ 54.3%] 14.21% [.] compute_flag 2.29 [100.0%] 14.16% [.] rand 0.36 [100.0%] 7.06% [.] __random_r 2.57 [ 70.5%] 6.85% [.] rand@plt 0.00 [ 0.0%] ... $ perf annotate --stdio2 Percent IPC Cycle (Average IPC: 2.30, IPC Coverage: 54.8%) Disassembly of section .text: 000000000003aac0 <random@@GLIBC_2.2.5>: 8.32 3.28 sub $0x18,%rsp 3.28 mov $0x1,%esi 3.28 xor %eax,%eax 3.28 cmpl $0x0,argp_program_version_hook@@GLIBC_2.2.5+0x1e0 11.57 3.28 1 ↓ je 20 lock cmpxchg %esi,__abort_msg@@GLIBC_PRIVATE+0x8a0 ↓ jne 29 ↓ jmp 43 11.57 1.10 20: cmpxchg %esi,__abort_msg@@GLIBC_PRIVATE+0x8a0 ... Jin Yao (2): perf annotate: Compute average IPC and IPC coverage per symbol perf report: Display average IPC and IPC coverage per symbol tools/perf/Documentation/perf-report.txt | 1 + tools/perf/builtin-report.c | 11 ++++++++ tools/perf/util/annotate.c | 41 +++++++++++++++++++++++++++--- tools/perf/util/annotate.h | 5 ++++ tools/perf/util/hist.h | 1 + tools/perf/util/sort.c | 43 ++++++++++++++++++++++++++++++++ tools/perf/util/sort.h | 1 + tools/perf/util/symbol.h | 1 + 8 files changed, 101 insertions(+), 3 deletions(-) -- 2.7.4 ^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/2] perf annotate: Compute average IPC and IPC coverage per symbol 2018-11-26 9:40 [PATCH 0/2] perf report/annotate: Support average IPC and IPC coverage for function Jin Yao @ 2018-11-26 9:40 ` Jin Yao 2018-11-26 9:40 ` [PATCH 2/2] perf report: Display " Jin Yao 1 sibling, 0 replies; 10+ messages in thread From: Jin Yao @ 2018-11-26 9:40 UTC (permalink / raw) To: acme, jolsa, peterz, mingo, alexander.shishkin Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao Add support to perf report annotate view or perf annotate --stdio2 to aggregate the IPC derived from timed LBRs per symbol. We compute the average IPC and the IPC coverage percentage. For example, $ perf annotate --stdio2 Percent IPC Cycle (Average IPC: 2.30, IPC Coverage: 54.8%) Disassembly of section .text: 000000000003aac0 <random@@GLIBC_2.2.5>: 8.32 3.28 sub $0x18,%rsp 3.28 mov $0x1,%esi 3.28 xor %eax,%eax 3.28 cmpl $0x0,argp_program_version_hook@@GLIBC_2.2.5+0x1e0 11.57 3.28 1 ↓ je 20 lock cmpxchg %esi,__abort_msg@@GLIBC_PRIVATE+0x8a0 ↓ jne 29 ↓ jmp 43 11.57 1.10 20: cmpxchg %esi,__abort_msg@@GLIBC_PRIVATE+0x8a0 0.00 1.10 1 ↓ je 43 29: lea __abort_msg@@GLIBC_PRIVATE+0x8a0,%rdi sub $0x80,%rsp → callq __lll_lock_wait_private add $0x80,%rsp 0.00 3.00 43: lea __ctype_b@GLIBC_2.2.5+0x38,%rdi 3.00 lea 0xc(%rsp),%rsi 8.49 3.00 1 → callq __random_r 7.91 1.94 cmpl $0x0,argp_program_version_hook@@GLIBC_2.2.5+0x1e0 0.00 1.94 1 ↓ je 68 lock decl __abort_msg@@GLIBC_PRIVATE+0x8a0 ↓ jne 70 ↓ jmp 8a 0.00 2.00 68: decl __abort_msg@@GLIBC_PRIVATE+0x8a0 21.56 2.00 1 ↓ je 8a 70: lea __abort_msg@@GLIBC_PRIVATE+0x8a0,%rdi sub $0x80,%rsp → callq __lll_unlock_wake_private add $0x80,%rsp 21.56 2.90 8a: movslq 0xc(%rsp),%rax 2.90 add $0x18,%rsp 9.03 2.90 1 ← retq It shows for this symbol the average IPC is 2.30 and the IPC coverage is 54.8%. Signed-off-by: Jin Yao <yao.jin@linux.intel.com> --- tools/perf/util/annotate.c | 41 ++++++++++++++++++++++++++++++++++++++--- tools/perf/util/annotate.h | 5 +++++ 2 files changed, 43 insertions(+), 3 deletions(-) diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c index 6936daf..4b2b1b0 100644 --- a/tools/perf/util/annotate.c +++ b/tools/perf/util/annotate.c @@ -1000,6 +1000,7 @@ static unsigned annotation__count_insn(struct annotation *notes, u64 start, u64 static void annotation__count_and_fill(struct annotation *notes, u64 start, u64 end, struct cyc_hist *ch) { unsigned n_insn; + unsigned int cover_insn = 0; u64 offset; n_insn = annotation__count_insn(notes, start, end); @@ -1013,21 +1014,34 @@ static void annotation__count_and_fill(struct annotation *notes, u64 start, u64 for (offset = start; offset <= end; offset++) { struct annotation_line *al = notes->offsets[offset]; - if (al) + if (al && al->ipc == 0.0) { al->ipc = ipc; + cover_insn++; + } + } + + if (cover_insn) { + notes->hit_cycles += ch->cycles; + notes->hit_insn += n_insn * ch->num; + notes->cover_insn += cover_insn; } } } void annotation__compute_ipc(struct annotation *notes, size_t size) { - u64 offset; + s64 offset; if (!notes->src || !notes->src->cycles_hist) return; + notes->total_insn = annotation__count_insn(notes, 0, size - 1); + notes->hit_cycles = 0; + notes->hit_insn = 0; + notes->cover_insn = 0; + pthread_mutex_lock(¬es->lock); - for (offset = 0; offset < size; ++offset) { + for (offset = size - 1; offset >= 0; --offset) { struct cyc_hist *ch; ch = ¬es->src->cycles_hist[offset]; @@ -2563,6 +2577,22 @@ static void disasm_line__write(struct disasm_line *dl, struct annotation *notes, disasm_line__scnprintf(dl, bf, size, !notes->options->use_offset); } +static void ipc_coverage_string(char *bf, int size, struct annotation *notes) +{ + double ipc = 0.0, coverage = 0.0; + + if (notes->hit_cycles) + ipc = notes->hit_insn / ((double)notes->hit_cycles); + + if (notes->total_insn) { + coverage = notes->cover_insn * 100.0 / + ((double)notes->total_insn); + } + + scnprintf(bf, size, "(Average IPC: %.2f, IPC Coverage: %.1f%%)", + ipc, coverage); +} + static void __annotation_line__write(struct annotation_line *al, struct annotation *notes, bool first_line, bool current_entry, bool change_color, int width, void *obj, unsigned int percent_type, @@ -2658,6 +2688,11 @@ static void __annotation_line__write(struct annotation_line *al, struct annotati ANNOTATION__MINMAX_CYCLES_WIDTH - 1, "Cycle(min/max)"); } + + if (show_title && !*al->line) { + ipc_coverage_string(bf, sizeof(bf), notes); + obj__printf(obj, "%*s", ANNOTATION__AVG_IPC_WIDTH, bf); + } } obj__printf(obj, " "); diff --git a/tools/perf/util/annotate.h b/tools/perf/util/annotate.h index 5399ba2..fb64637 100644 --- a/tools/perf/util/annotate.h +++ b/tools/perf/util/annotate.h @@ -64,6 +64,7 @@ bool ins__is_fused(struct arch *arch, const char *ins1, const char *ins2); #define ANNOTATION__IPC_WIDTH 6 #define ANNOTATION__CYCLES_WIDTH 6 #define ANNOTATION__MINMAX_CYCLES_WIDTH 19 +#define ANNOTATION__AVG_IPC_WIDTH 36 struct annotation_options { bool hide_src_code, @@ -262,6 +263,10 @@ struct annotation { pthread_mutex_t lock; u64 max_coverage; u64 start; + u64 hit_cycles; + u64 hit_insn; + unsigned int total_insn; + unsigned int cover_insn; struct annotation_options *options; struct annotation_line **offsets; int nr_events; -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* [PATCH 2/2] perf report: Display average IPC and IPC coverage per symbol 2018-11-26 9:40 [PATCH 0/2] perf report/annotate: Support average IPC and IPC coverage for function Jin Yao 2018-11-26 9:40 ` [PATCH 1/2] perf annotate: Compute average IPC and IPC coverage per symbol Jin Yao @ 2018-11-26 9:40 ` Jin Yao 2018-11-26 9:52 ` Jiri Olsa ` (2 more replies) 1 sibling, 3 replies; 10+ messages in thread From: Jin Yao @ 2018-11-26 9:40 UTC (permalink / raw) To: acme, jolsa, peterz, mingo, alexander.shishkin Cc: Linux-kernel, ak, kan.liang, yao.jin, Jin Yao Support displaying the average IPC and IPC coverage for symbol in perf report TUI browser. We create a new sort-key 'ipc' for that. For example, $ perf record -g -b ... $ perf report -s symbol,ipc or perf report -s ipc Overhead Symbol IPC [IPC Coverage] 39.60% [.] __random 2.30 [ 54.8%] 18.02% [.] main 0.43 [ 54.3%] 14.21% [.] compute_flag 2.29 [100.0%] 14.16% [.] rand 0.36 [100.0%] 7.06% [.] __random_r 2.57 [ 70.5%] 6.85% [.] rand@plt 0.00 [ 0.0%] Note that, stdio mode doesn't support this feature. Signed-off-by: Jin Yao <yao.jin@linux.intel.com> --- tools/perf/Documentation/perf-report.txt | 1 + tools/perf/builtin-report.c | 11 ++++++++ tools/perf/util/hist.h | 1 + tools/perf/util/sort.c | 43 ++++++++++++++++++++++++++++++++ tools/perf/util/sort.h | 1 + tools/perf/util/symbol.h | 1 + 6 files changed, 58 insertions(+) diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt index 474a494..e7b8974 100644 --- a/tools/perf/Documentation/perf-report.txt +++ b/tools/perf/Documentation/perf-report.txt @@ -122,6 +122,7 @@ OPTIONS - in_tx: branch in TSX transaction - abort: TSX transaction abort. - cycles: Cycles in basic block + - ipc: average ipc (instruction per cycle) of function And default sort keys are changed to comm, dso_from, symbol_from, dso_to and symbol_to, see '--branch-stack'. diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 257c9c1..9b75e11 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -954,6 +954,7 @@ int cmd_report(int argc, const char **argv) bool has_br_stack = false; int branch_mode = -1; bool branch_call_mode = false; + bool symbol_ipc = false; #define CALLCHAIN_DEFAULT_OPT "graph,0.5,caller,function,percent" const char report_callchain_help[] = "Display call graph (stack chain/backtrace):\n\n" CALLCHAIN_REPORT_HELP @@ -1284,6 +1285,13 @@ int cmd_report(int argc, const char **argv) else use_browser = 0; + if ((sort__mode == SORT_MODE__BRANCH) && sort_order && + strstr(sort_order, "ipc")) { + if (!strstr(sort_order, "symbol")) + sort_order = "symbol,ipc"; + symbol_ipc = true; + } + if (setup_sorting(session->evlist) < 0) { if (sort_order) parse_options_usage(report_usage, options, "s", 1); @@ -1331,6 +1339,9 @@ int cmd_report(int argc, const char **argv) symbol_conf.sort_by_name = true; } annotation_config__init(); + } else if (symbol_ipc) { + pr_err("Only TUI mode supports sort-key ipc\n"); + goto error; } if (symbol__init(&session->header.env) < 0) diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h index 3badd7f..664b5ed 100644 --- a/tools/perf/util/hist.h +++ b/tools/perf/util/hist.h @@ -62,6 +62,7 @@ enum hist_column { HISTC_TRACE, HISTC_SYM_SIZE, HISTC_DSO_SIZE, + HISTC_SYMBOL_IPC, HISTC_NR_COLS, /* Last entry */ }; diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c index f96c005..94f62c8 100644 --- a/tools/perf/util/sort.c +++ b/tools/perf/util/sort.c @@ -13,6 +13,7 @@ #include "strlist.h" #include <traceevent/event-parse.h> #include "mem-events.h" +#include "annotate.h" #include <linux/kernel.h> regex_t parent_regex; @@ -422,6 +423,47 @@ struct sort_entry sort_srcline_to = { .se_width_idx = HISTC_SRCLINE_TO, }; +static int hist_entry__sym_ipc_snprintf(struct hist_entry *he, char *bf, + size_t size, unsigned int width) +{ + + struct symbol *sym = he->ms.sym; + struct map *map = he->ms.map; + struct perf_evsel *evsel = hists_to_evsel(he->hists); + struct annotation *notes; + double ipc = 0.0, coverage = 0.0; + char tmp[64]; + + if (!sym) + return repsep_snprintf(bf, size, "%-*s", width, "-"); + + if (!sym->annotated && + symbol__annotate2(sym, map, evsel, &annotation__default_options, + NULL) < 0) { + return 0; + } + + sym->annotated = true; + notes = symbol__annotation(sym); + + if (notes->hit_cycles) + ipc = notes->hit_insn / ((double)notes->hit_cycles); + + if (notes->total_insn) + coverage = notes->cover_insn * 100.0 / + ((double)notes->total_insn); + + snprintf(tmp, sizeof(tmp), "%-5.2f [%5.1f%%]", ipc, coverage); + return repsep_snprintf(bf, size, "%-*s", width, tmp); +} + +struct sort_entry sort_sym_ipc = { + .se_header = "IPC [IPC Coverage]", + .se_cmp = sort__sym_cmp, + .se_snprintf = hist_entry__sym_ipc_snprintf, + .se_width_idx = HISTC_SYMBOL_IPC, +}; + /* --sort srcfile */ static char no_srcfile[1]; @@ -1591,6 +1633,7 @@ static struct sort_dimension bstack_sort_dimensions[] = { DIM(SORT_CYCLES, "cycles", sort_cycles), DIM(SORT_SRCLINE_FROM, "srcline_from", sort_srcline_from), DIM(SORT_SRCLINE_TO, "srcline_to", sort_srcline_to), + DIM(SORT_SYM_IPC, "ipc", sort_sym_ipc), }; #undef DIM diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h index a97cf8e..de70736 100644 --- a/tools/perf/util/sort.h +++ b/tools/perf/util/sort.h @@ -242,6 +242,7 @@ enum sort_type { SORT_CYCLES, SORT_SRCLINE_FROM, SORT_SRCLINE_TO, + SORT_SYM_IPC, /* memory mode specific sort keys */ __SORT_MEMORY_MODE, diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h index d026d21..94a567f 100644 --- a/tools/perf/util/symbol.h +++ b/tools/perf/util/symbol.h @@ -63,6 +63,7 @@ struct symbol { u8 ignore:1; u8 inlined:1; u8 arch_sym; + bool annotated; char name[0]; }; -- 2.7.4 ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] perf report: Display average IPC and IPC coverage per symbol 2018-11-26 9:40 ` [PATCH 2/2] perf report: Display " Jin Yao @ 2018-11-26 9:52 ` Jiri Olsa 2018-11-27 3:50 ` Jin, Yao 2018-11-26 9:53 ` Jiri Olsa 2018-11-26 9:55 ` Jiri Olsa 2 siblings, 1 reply; 10+ messages in thread From: Jiri Olsa @ 2018-11-26 9:52 UTC (permalink / raw) To: Jin Yao Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak, kan.liang, yao.jin On Mon, Nov 26, 2018 at 05:40:54PM +0800, Jin Yao wrote: > Support displaying the average IPC and IPC coverage for symbol > in perf report TUI browser. We create a new sort-key 'ipc' for > that. > > For example, > > $ perf record -g -b ... > $ perf report -s symbol,ipc or > perf report -s ipc > > Overhead Symbol IPC [IPC Coverage] > 39.60% [.] __random 2.30 [ 54.8%] > 18.02% [.] main 0.43 [ 54.3%] > 14.21% [.] compute_flag 2.29 [100.0%] > 14.16% [.] rand 0.36 [100.0%] > 7.06% [.] __random_r 2.57 [ 70.5%] > 6.85% [.] rand@plt 0.00 [ 0.0%] > > Note that, stdio mode doesn't support this feature. the patch below allowed this for stdio please merge it in, and feel free to change it as you see fit jirka --- diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c index 9b75e118f609..a6756dc13285 100644 --- a/tools/perf/builtin-report.c +++ b/tools/perf/builtin-report.c @@ -85,6 +85,7 @@ struct report { int socket_filter; DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS); struct branch_type_stat brtype_stat; + bool symbol_ipc; }; static int report__config(const char *var, const char *value, void *cb) @@ -129,7 +130,7 @@ static int hist_iter__report_callback(struct hist_entry_iter *iter, struct mem_info *mi; struct branch_info *bi; - if (!ui__has_annotation()) + if (!ui__has_annotation() && !rep->symbol_ipc) return 0; hist__account_cycles(sample->branch_stack, al, sample, @@ -174,7 +175,7 @@ static int hist_iter__branch_callback(struct hist_entry_iter *iter, struct perf_evsel *evsel = iter->evsel; int err; - if (!ui__has_annotation()) + if (!ui__has_annotation() && !rep->symbol_ipc) return 0; hist__account_cycles(sample->branch_stack, al, sample, @@ -954,7 +955,6 @@ int cmd_report(int argc, const char **argv) bool has_br_stack = false; int branch_mode = -1; bool branch_call_mode = false; - bool symbol_ipc = false; #define CALLCHAIN_DEFAULT_OPT "graph,0.5,caller,function,percent" const char report_callchain_help[] = "Display call graph (stack chain/backtrace):\n\n" CALLCHAIN_REPORT_HELP @@ -1289,7 +1289,7 @@ int cmd_report(int argc, const char **argv) strstr(sort_order, "ipc")) { if (!strstr(sort_order, "symbol")) sort_order = "symbol,ipc"; - symbol_ipc = true; + report.symbol_ipc = true; } if (setup_sorting(session->evlist) < 0) { @@ -1319,7 +1319,7 @@ int cmd_report(int argc, const char **argv) * so don't allocate extra space that won't be used in the stdio * implementation. */ - if (ui__has_annotation()) { + if (ui__has_annotation() || report.symbol_ipc) { ret = symbol__annotation_init(); if (ret < 0) goto error; @@ -1339,9 +1339,6 @@ int cmd_report(int argc, const char **argv) symbol_conf.sort_by_name = true; } annotation_config__init(); - } else if (symbol_ipc) { - pr_err("Only TUI mode supports sort-key ipc\n"); - goto error; } if (symbol__init(&session->header.env) < 0) ^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] perf report: Display average IPC and IPC coverage per symbol 2018-11-26 9:52 ` Jiri Olsa @ 2018-11-27 3:50 ` Jin, Yao 0 siblings, 0 replies; 10+ messages in thread From: Jin, Yao @ 2018-11-27 3:50 UTC (permalink / raw) To: Jiri Olsa Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak, kan.liang, yao.jin On 11/26/2018 5:52 PM, Jiri Olsa wrote: > On Mon, Nov 26, 2018 at 05:40:54PM +0800, Jin Yao wrote: >> Support displaying the average IPC and IPC coverage for symbol >> in perf report TUI browser. We create a new sort-key 'ipc' for >> that. >> >> For example, >> >> $ perf record -g -b ... >> $ perf report -s symbol,ipc or >> perf report -s ipc >> >> Overhead Symbol IPC [IPC Coverage] >> 39.60% [.] __random 2.30 [ 54.8%] >> 18.02% [.] main 0.43 [ 54.3%] >> 14.21% [.] compute_flag 2.29 [100.0%] >> 14.16% [.] rand 0.36 [100.0%] >> 7.06% [.] __random_r 2.57 [ 70.5%] >> 6.85% [.] rand@plt 0.00 [ 0.0%] >> >> Note that, stdio mode doesn't support this feature. > > the patch below allowed this for stdio > please merge it in, and feel free to change it as you see fit > > jirka > > Thanks so much! I have merged following code in v2. Thanks Jin Yao > --- > diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c > index 9b75e118f609..a6756dc13285 100644 > --- a/tools/perf/builtin-report.c > +++ b/tools/perf/builtin-report.c > @@ -85,6 +85,7 @@ struct report { > int socket_filter; > DECLARE_BITMAP(cpu_bitmap, MAX_NR_CPUS); > struct branch_type_stat brtype_stat; > + bool symbol_ipc; > }; > > static int report__config(const char *var, const char *value, void *cb) > @@ -129,7 +130,7 @@ static int hist_iter__report_callback(struct hist_entry_iter *iter, > struct mem_info *mi; > struct branch_info *bi; > > - if (!ui__has_annotation()) > + if (!ui__has_annotation() && !rep->symbol_ipc) > return 0; > > hist__account_cycles(sample->branch_stack, al, sample, > @@ -174,7 +175,7 @@ static int hist_iter__branch_callback(struct hist_entry_iter *iter, > struct perf_evsel *evsel = iter->evsel; > int err; > > - if (!ui__has_annotation()) > + if (!ui__has_annotation() && !rep->symbol_ipc) > return 0; > > hist__account_cycles(sample->branch_stack, al, sample, > @@ -954,7 +955,6 @@ int cmd_report(int argc, const char **argv) > bool has_br_stack = false; > int branch_mode = -1; > bool branch_call_mode = false; > - bool symbol_ipc = false; > #define CALLCHAIN_DEFAULT_OPT "graph,0.5,caller,function,percent" > const char report_callchain_help[] = "Display call graph (stack chain/backtrace):\n\n" > CALLCHAIN_REPORT_HELP > @@ -1289,7 +1289,7 @@ int cmd_report(int argc, const char **argv) > strstr(sort_order, "ipc")) { > if (!strstr(sort_order, "symbol")) > sort_order = "symbol,ipc"; > - symbol_ipc = true; > + report.symbol_ipc = true; > } > > if (setup_sorting(session->evlist) < 0) { > @@ -1319,7 +1319,7 @@ int cmd_report(int argc, const char **argv) > * so don't allocate extra space that won't be used in the stdio > * implementation. > */ > - if (ui__has_annotation()) { > + if (ui__has_annotation() || report.symbol_ipc) { > ret = symbol__annotation_init(); > if (ret < 0) > goto error; > @@ -1339,9 +1339,6 @@ int cmd_report(int argc, const char **argv) > symbol_conf.sort_by_name = true; > } > annotation_config__init(); > - } else if (symbol_ipc) { > - pr_err("Only TUI mode supports sort-key ipc\n"); > - goto error; > } > > if (symbol__init(&session->header.env) < 0) > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] perf report: Display average IPC and IPC coverage per symbol 2018-11-26 9:40 ` [PATCH 2/2] perf report: Display " Jin Yao 2018-11-26 9:52 ` Jiri Olsa @ 2018-11-26 9:53 ` Jiri Olsa 2018-11-27 3:52 ` Jin, Yao 2018-11-26 9:55 ` Jiri Olsa 2 siblings, 1 reply; 10+ messages in thread From: Jiri Olsa @ 2018-11-26 9:53 UTC (permalink / raw) To: Jin Yao Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak, kan.liang, yao.jin On Mon, Nov 26, 2018 at 05:40:54PM +0800, Jin Yao wrote: SNIP > diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c > index f96c005..94f62c8 100644 > --- a/tools/perf/util/sort.c > +++ b/tools/perf/util/sort.c > @@ -13,6 +13,7 @@ > #include "strlist.h" > #include <traceevent/event-parse.h> > #include "mem-events.h" > +#include "annotate.h" > #include <linux/kernel.h> > > regex_t parent_regex; > @@ -422,6 +423,47 @@ struct sort_entry sort_srcline_to = { > .se_width_idx = HISTC_SRCLINE_TO, > }; > > +static int hist_entry__sym_ipc_snprintf(struct hist_entry *he, char *bf, > + size_t size, unsigned int width) > +{ > + > + struct symbol *sym = he->ms.sym; > + struct map *map = he->ms.map; > + struct perf_evsel *evsel = hists_to_evsel(he->hists); > + struct annotation *notes; > + double ipc = 0.0, coverage = 0.0; > + char tmp[64]; > + > + if (!sym) > + return repsep_snprintf(bf, size, "%-*s", width, "-"); > + > + if (!sym->annotated && > + symbol__annotate2(sym, map, evsel, &annotation__default_options, > + NULL) < 0) { > + return 0; > + } > + > + sym->annotated = true; I don't like this being set in here.. please move it to symbol__annotate2 or symbol__annotate, not sure which one of these is the best fit > + notes = symbol__annotation(sym); > + > + if (notes->hit_cycles) > + ipc = notes->hit_insn / ((double)notes->hit_cycles); > + > + if (notes->total_insn) > + coverage = notes->cover_insn * 100.0 / > + ((double)notes->total_insn); missing { } for multiline code in 'if' condition thanks, jirka ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] perf report: Display average IPC and IPC coverage per symbol 2018-11-26 9:53 ` Jiri Olsa @ 2018-11-27 3:52 ` Jin, Yao 0 siblings, 0 replies; 10+ messages in thread From: Jin, Yao @ 2018-11-27 3:52 UTC (permalink / raw) To: Jiri Olsa Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak, kan.liang, yao.jin On 11/26/2018 5:53 PM, Jiri Olsa wrote: > On Mon, Nov 26, 2018 at 05:40:54PM +0800, Jin Yao wrote: > > SNIP > >> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c >> index f96c005..94f62c8 100644 >> --- a/tools/perf/util/sort.c >> +++ b/tools/perf/util/sort.c >> @@ -13,6 +13,7 @@ >> #include "strlist.h" >> #include <traceevent/event-parse.h> >> #include "mem-events.h" >> +#include "annotate.h" >> #include <linux/kernel.h> >> >> regex_t parent_regex; >> @@ -422,6 +423,47 @@ struct sort_entry sort_srcline_to = { >> .se_width_idx = HISTC_SRCLINE_TO, >> }; >> >> +static int hist_entry__sym_ipc_snprintf(struct hist_entry *he, char *bf, >> + size_t size, unsigned int width) >> +{ >> + >> + struct symbol *sym = he->ms.sym; >> + struct map *map = he->ms.map; >> + struct perf_evsel *evsel = hists_to_evsel(he->hists); >> + struct annotation *notes; >> + double ipc = 0.0, coverage = 0.0; >> + char tmp[64]; >> + >> + if (!sym) >> + return repsep_snprintf(bf, size, "%-*s", width, "-"); >> + >> + if (!sym->annotated && >> + symbol__annotate2(sym, map, evsel, &annotation__default_options, >> + NULL) < 0) { >> + return 0; >> + } >> + >> + sym->annotated = true; > > I don't like this being set in here.. please move it to > symbol__annotate2 or symbol__annotate, not sure which > one of these is the best fit > I have set this flag in symbol__annotate2 in v2. >> + notes = symbol__annotation(sym); >> + >> + if (notes->hit_cycles) >> + ipc = notes->hit_insn / ((double)notes->hit_cycles); >> + >> + if (notes->total_insn) >> + coverage = notes->cover_insn * 100.0 / >> + ((double)notes->total_insn); > > missing { } for multiline code in 'if' condition > Fixed, thanks! Thanks Jin Yao > thanks, > jirka > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] perf report: Display average IPC and IPC coverage per symbol 2018-11-26 9:40 ` [PATCH 2/2] perf report: Display " Jin Yao 2018-11-26 9:52 ` Jiri Olsa 2018-11-26 9:53 ` Jiri Olsa @ 2018-11-26 9:55 ` Jiri Olsa 2018-11-27 3:42 ` Jin, Yao 2 siblings, 1 reply; 10+ messages in thread From: Jiri Olsa @ 2018-11-26 9:55 UTC (permalink / raw) To: Jin Yao Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak, kan.liang, yao.jin On Mon, Nov 26, 2018 at 05:40:54PM +0800, Jin Yao wrote: SNIP > diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c > index f96c005..94f62c8 100644 > --- a/tools/perf/util/sort.c > +++ b/tools/perf/util/sort.c > @@ -13,6 +13,7 @@ > #include "strlist.h" > #include <traceevent/event-parse.h> > #include "mem-events.h" > +#include "annotate.h" > #include <linux/kernel.h> > > regex_t parent_regex; > @@ -422,6 +423,47 @@ struct sort_entry sort_srcline_to = { > .se_width_idx = HISTC_SRCLINE_TO, > }; > > +static int hist_entry__sym_ipc_snprintf(struct hist_entry *he, char *bf, > + size_t size, unsigned int width) > +{ > + > + struct symbol *sym = he->ms.sym; > + struct map *map = he->ms.map; > + struct perf_evsel *evsel = hists_to_evsel(he->hists); > + struct annotation *notes; > + double ipc = 0.0, coverage = 0.0; > + char tmp[64]; > + > + if (!sym) > + return repsep_snprintf(bf, size, "%-*s", width, "-"); > + > + if (!sym->annotated && > + symbol__annotate2(sym, map, evsel, &annotation__default_options, > + NULL) < 0) { > + return 0; > + } this seems to make the sorting extra long, would you please consider progress bar update for this? should be added somewhere around hists sorting code thanks, jirka ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] perf report: Display average IPC and IPC coverage per symbol 2018-11-26 9:55 ` Jiri Olsa @ 2018-11-27 3:42 ` Jin, Yao 2018-11-27 9:34 ` Jiri Olsa 0 siblings, 1 reply; 10+ messages in thread From: Jin, Yao @ 2018-11-27 3:42 UTC (permalink / raw) To: Jiri Olsa Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak, kan.liang, yao.jin On 11/26/2018 5:55 PM, Jiri Olsa wrote: > On Mon, Nov 26, 2018 at 05:40:54PM +0800, Jin Yao wrote: > > SNIP > >> diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c >> index f96c005..94f62c8 100644 >> --- a/tools/perf/util/sort.c >> +++ b/tools/perf/util/sort.c >> @@ -13,6 +13,7 @@ >> #include "strlist.h" >> #include <traceevent/event-parse.h> >> #include "mem-events.h" >> +#include "annotate.h" >> #include <linux/kernel.h> >> >> regex_t parent_regex; >> @@ -422,6 +423,47 @@ struct sort_entry sort_srcline_to = { >> .se_width_idx = HISTC_SRCLINE_TO, >> }; >> >> +static int hist_entry__sym_ipc_snprintf(struct hist_entry *he, char *bf, >> + size_t size, unsigned int width) >> +{ >> + >> + struct symbol *sym = he->ms.sym; >> + struct map *map = he->ms.map; >> + struct perf_evsel *evsel = hists_to_evsel(he->hists); >> + struct annotation *notes; >> + double ipc = 0.0, coverage = 0.0; >> + char tmp[64]; >> + >> + if (!sym) >> + return repsep_snprintf(bf, size, "%-*s", width, "-"); >> + >> + if (!sym->annotated && >> + symbol__annotate2(sym, map, evsel, &annotation__default_options, >> + NULL) < 0) { >> + return 0; >> + } > > this seems to make the sorting extra long, would you > please consider progress bar update for this? > > should be added somewhere around hists sorting code > Hi Jiri, Sorting doesn't take long time in my test but the session event processing takes some time. I just think maybe we need a new ops for stdio progress bar like what the tui_progress__ops does now. That should be benefit for all stdio usages. That may be another separate patch-set. Thanks Jin Yao > thanks, > jirka > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 2/2] perf report: Display average IPC and IPC coverage per symbol 2018-11-27 3:42 ` Jin, Yao @ 2018-11-27 9:34 ` Jiri Olsa 0 siblings, 0 replies; 10+ messages in thread From: Jiri Olsa @ 2018-11-27 9:34 UTC (permalink / raw) To: Jin, Yao Cc: acme, jolsa, peterz, mingo, alexander.shishkin, Linux-kernel, ak, kan.liang, yao.jin On Tue, Nov 27, 2018 at 11:42:12AM +0800, Jin, Yao wrote: > > > On 11/26/2018 5:55 PM, Jiri Olsa wrote: > > On Mon, Nov 26, 2018 at 05:40:54PM +0800, Jin Yao wrote: > > > > SNIP > > > > > diff --git a/tools/perf/util/sort.c b/tools/perf/util/sort.c > > > index f96c005..94f62c8 100644 > > > --- a/tools/perf/util/sort.c > > > +++ b/tools/perf/util/sort.c > > > @@ -13,6 +13,7 @@ > > > #include "strlist.h" > > > #include <traceevent/event-parse.h> > > > #include "mem-events.h" > > > +#include "annotate.h" > > > #include <linux/kernel.h> > > > regex_t parent_regex; > > > @@ -422,6 +423,47 @@ struct sort_entry sort_srcline_to = { > > > .se_width_idx = HISTC_SRCLINE_TO, > > > }; > > > +static int hist_entry__sym_ipc_snprintf(struct hist_entry *he, char *bf, > > > + size_t size, unsigned int width) > > > +{ > > > + > > > + struct symbol *sym = he->ms.sym; > > > + struct map *map = he->ms.map; > > > + struct perf_evsel *evsel = hists_to_evsel(he->hists); > > > + struct annotation *notes; > > > + double ipc = 0.0, coverage = 0.0; > > > + char tmp[64]; > > > + > > > + if (!sym) > > > + return repsep_snprintf(bf, size, "%-*s", width, "-"); > > > + > > > + if (!sym->annotated && > > > + symbol__annotate2(sym, map, evsel, &annotation__default_options, > > > + NULL) < 0) { > > > + return 0; > > > + } > > > > this seems to make the sorting extra long, would you > > please consider progress bar update for this? > > > > should be added somewhere around hists sorting code > > > > Hi Jiri, > > Sorting doesn't take long time in my test but the session event processing > takes some time. > > I just think maybe we need a new ops for stdio progress bar like what the > tui_progress__ops does now. That should be benefit for all stdio usages. > > That may be another separate patch-set. sure, thanks jirka ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2018-11-27 9:35 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2018-11-26 9:40 [PATCH 0/2] perf report/annotate: Support average IPC and IPC coverage for function Jin Yao 2018-11-26 9:40 ` [PATCH 1/2] perf annotate: Compute average IPC and IPC coverage per symbol Jin Yao 2018-11-26 9:40 ` [PATCH 2/2] perf report: Display " Jin Yao 2018-11-26 9:52 ` Jiri Olsa 2018-11-27 3:50 ` Jin, Yao 2018-11-26 9:53 ` Jiri Olsa 2018-11-27 3:52 ` Jin, Yao 2018-11-26 9:55 ` Jiri Olsa 2018-11-27 3:42 ` Jin, Yao 2018-11-27 9:34 ` Jiri Olsa
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.