Re: [PATCH 2/2] perf report: Display average IPC and IPC coverage per symbol

[Jin, Yao]

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)