Re: [PATCH v2] perf report: Make --branch-history work without callgraphs(-g) option in perf record

["Jin, Yao" <yao.jin@linux.intel.com>]

Hi Arnaldo,

Is this patch OK for merging? It's more than 2 months no more comments.

Thanks
Jin Yao


On 5/8/2017 6:43 PM, Jin Yao wrote:
> perf record -b -g <command>
> perf report --branch-history
>
> This merges the LBRs with the callgraphs.
>
> However it would be nice if it also works without callgraphs (-g)
> set in perf record, so that only the LBRs are displayed.
> But currently perf report errors in this case. For example,
>
> perf record -b <command>
> perf report --branch-history
>
> Error:
> Selected -g or --branch-history but no callchain data. Did
> you call 'perf record' without -g?
>
> This patch displays the LBRs only even if callgraphs(-g) is not
> enabled in perf record.
>
> Change log:
> ----------
> v2: According to Milian Wolff's comment, change the obsolete error
> message. Now the error message is:
>
>                   ┌─Error:─────────────────────────────────────┐
>                   │Selected -g or --branch-history.            │
>                   │But no callchain or branch data.            │
>                   │Did you call 'perf record' without -g or -b?│
>                   │                                            │
>                   │                                            │
>                   │Press any key...                            │
>                   └────────────────────────────────────────────┘
>
> When passing the last parameter to hists__fprintf,
> changes "|" to "||".
>
> hsts__fprintf(hists, !quiet, 0, 0, rep->min_percent, stdout,
>                symbol_conf.use_callchain ||
>                symbol_conf.show_branchflag_count);
>
> Signed-off-by: Jin Yao <yao.jin@linux.intel.com>
> ---
>   tools/perf/builtin-report.c | 12 +++++++-----
>   tools/perf/util/callchain.c |  7 ++++---
>   tools/perf/util/hist.c      |  2 ++
>   tools/perf/util/machine.c   | 13 ++++++++++++-
>   4 files changed, 25 insertions(+), 9 deletions(-)
>
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 22478ff..4ceb2d2 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -259,10 +259,11 @@ static int report__setup_sample_type(struct report *rep)
>   				    "'perf record' without -g?\n");
>   			return -EINVAL;
>   		}
> -		if (symbol_conf.use_callchain) {
> -			ui__error("Selected -g or --branch-history but no "
> -				  "callchain data. Did\n"
> -				  "you call 'perf record' without -g?\n");
> +		if (symbol_conf.use_callchain &&
> +			!symbol_conf.show_branchflag_count) {
> +			ui__error("Selected -g or --branch-history.\n"
> +				  "But no callchain or branch data.\n"
> +				  "Did you call 'perf record' without -g or -b?\n");
>   			return -1;
>   		}
>   	} else if (!callchain_param.enabled &&
> @@ -397,7 +398,8 @@ static int perf_evlist__tty_browse_hists(struct perf_evlist *evlist,
>   
>   		hists__fprintf_nr_sample_events(hists, rep, evname, stdout);
>   		hists__fprintf(hists, !quiet, 0, 0, rep->min_percent, stdout,
> -			       symbol_conf.use_callchain);
> +			       symbol_conf.use_callchain ||
> +			       symbol_conf.show_branchflag_count);
>   		fprintf(stdout, "\n\n");
>   	}
>   
> diff --git a/tools/perf/util/callchain.c b/tools/perf/util/callchain.c
> index 81fc29a..08d3abf 100644
> --- a/tools/perf/util/callchain.c
> +++ b/tools/perf/util/callchain.c
> @@ -993,11 +993,11 @@ int sample__resolve_callchain(struct perf_sample *sample,
>   			      struct perf_evsel *evsel, struct addr_location *al,
>   			      int max_stack)
>   {
> -	if (sample->callchain == NULL)
> +	if (sample->callchain == NULL && !symbol_conf.show_branchflag_count)
>   		return 0;
>   
>   	if (symbol_conf.use_callchain || symbol_conf.cumulate_callchain ||
> -	    perf_hpp_list.parent) {
> +	    perf_hpp_list.parent || symbol_conf.show_branchflag_count) {
>   		return thread__resolve_callchain(al->thread, cursor, evsel, sample,
>   						 parent, al, max_stack);
>   	}
> @@ -1006,7 +1006,8 @@ int sample__resolve_callchain(struct perf_sample *sample,
>   
>   int hist_entry__append_callchain(struct hist_entry *he, struct perf_sample *sample)
>   {
> -	if (!symbol_conf.use_callchain || sample->callchain == NULL)
> +	if ((!symbol_conf.use_callchain || sample->callchain == NULL) &&
> +		!symbol_conf.show_branchflag_count)
>   		return 0;
>   	return callchain_append(he->callchain, &callchain_cursor, sample->period);
>   }
> diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
> index cf0186a..8b045a5 100644
> --- a/tools/perf/util/hist.c
> +++ b/tools/perf/util/hist.c
> @@ -1762,6 +1762,8 @@ void perf_evsel__output_resort(struct perf_evsel *evsel, struct ui_progress *pro
>   	else
>   		use_callchain = symbol_conf.use_callchain;
>   
> +	use_callchain |= symbol_conf.show_branchflag_count;
> +
>   	output_resort(evsel__hists(evsel), prog, use_callchain, NULL);
>   }
>   
> diff --git a/tools/perf/util/machine.c b/tools/perf/util/machine.c
> index d97e014..7b47080 100644
> --- a/tools/perf/util/machine.c
> +++ b/tools/perf/util/machine.c
> @@ -1901,13 +1901,16 @@ static int thread__resolve_callchain_sample(struct thread *thread,
>   {
>   	struct branch_stack *branch = sample->branch_stack;
>   	struct ip_callchain *chain = sample->callchain;
> -	int chain_nr = chain->nr;
> +	int chain_nr = 0;
>   	u8 cpumode = PERF_RECORD_MISC_USER;
>   	int i, j, err, nr_entries;
>   	int skip_idx = -1;
>   	int first_call = 0;
>   	int nr_loop_iter;
>   
> +	if (chain)
> +		chain_nr = chain->nr;
> +
>   	if (perf_evsel__has_branch_callstack(evsel)) {
>   		err = resolve_lbr_callchain_sample(thread, cursor, sample, parent,
>   						   root_al, max_stack);
> @@ -1945,6 +1948,10 @@ static int thread__resolve_callchain_sample(struct thread *thread,
>   		for (i = 0; i < nr; i++) {
>   			if (callchain_param.order == ORDER_CALLEE) {
>   				be[i] = branch->entries[i];
> +
> +				if (chain == NULL)
> +					continue;
> +
>   				/*
>   				 * Check for overlap into the callchain.
>   				 * The return address is one off compared to
> @@ -1998,6 +2005,10 @@ static int thread__resolve_callchain_sample(struct thread *thread,
>   			if (err)
>   				return err;
>   		}
> +
> +		if (chain_nr == 0)
> +			return 0;
> +
>   		chain_nr -= nr;
>   	}
>