Re: [PATCH 2/2] perf report: Display average IPC and IPC coverage per symbol
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
> > > #include "mem-events.h"
> > > +#include "annotate.h"
> > > #include
> > > 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, __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
Re: [PATCH 2/2] perf report: Display average IPC and IPC coverage per symbol
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
> > > #include "mem-events.h"
> > > +#include "annotate.h"
> > > #include
> > > 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, __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
Re: [PATCH 2/2] perf report: Display average IPC and IPC coverage per symbol
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
#include "mem-events.h"
+#include "annotate.h"
#include
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, __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
Re: [PATCH 2/2] perf report: Display average IPC and IPC coverage per symbol
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
#include "mem-events.h"
+#include "annotate.h"
#include
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, __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
Re: [PATCH 2/2] perf report: Display average IPC and IPC coverage per symbol
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;
+ boolsymbol_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(>header.env) < 0)
Re: [PATCH 2/2] perf report: Display average IPC and IPC coverage per symbol
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;
+ boolsymbol_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(>header.env) < 0)
Re: [PATCH 2/2] perf report: Display average IPC and IPC coverage per symbol
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
#include "mem-events.h"
+#include "annotate.h"
#include
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, __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
Re: [PATCH 2/2] perf report: Display average IPC and IPC coverage per symbol
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
#include "mem-events.h"
+#include "annotate.h"
#include
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, __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
Re: [PATCH 2/2] perf report: Display average IPC and IPC coverage per symbol
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
> #include "mem-events.h"
> +#include "annotate.h"
> #include
>
> 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, __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
Re: [PATCH 2/2] perf report: Display average IPC and IPC coverage per symbol
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
> #include "mem-events.h"
> +#include "annotate.h"
> #include
>
> 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, __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
Re: [PATCH 2/2] perf report: Display average IPC and IPC coverage per symbol
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
> #include "mem-events.h"
> +#include "annotate.h"
> #include
>
> 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, __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
Re: [PATCH 2/2] perf report: Display average IPC and IPC coverage per symbol
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
> #include "mem-events.h"
> +#include "annotate.h"
> #include
>
> 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, __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
Re: [PATCH 2/2] perf report: Display average IPC and IPC coverage per symbol
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;
+ boolsymbol_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(>header.env) < 0)
Re: [PATCH 2/2] perf report: Display average IPC and IPC coverage per symbol
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;
+ boolsymbol_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(>header.env) < 0)
