* [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: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: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-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: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-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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).