Re: [RFC][PATCH 7/7] perf/annotate: Add branch stack / basic block information

From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Peter Zijlstra <peterz@infradead.org>
Cc: Ingo Molnar <mingo@kernel.org>,
	linux-kernel@vger.kernel.org, andi@firstfloor.org,
	eranian@google.com, jolsa@kernel.org,
	torvalds@linux-foundation.org, davidcc@google.com,
	alexander.shishkin@linux.intel.com, namhyung@kernel.org,
	kan.liang@intel.com, khandual@linux.vnet.ibm.com
Subject: Re: [RFC][PATCH 7/7] perf/annotate: Add branch stack / basic block information
Date: Thu, 8 Sep 2016 13:18:57 -0300	[thread overview]
Message-ID: <20160908161857.GA4801@kernel.org> (raw)
In-Reply-To: <20160708163632.GK30927@twins.programming.kicks-ass.net>

Em Fri, Jul 08, 2016 at 06:36:32PM +0200, Peter Zijlstra escreveu:
> On Fri, Jul 08, 2016 at 06:27:33PM +0200, Peter Zijlstra wrote:
> 
> > I've been thinking of filtering all targets and branches that are
> > smaller than 0.1% in order to avoid this, but so far I've just been
> > ignoring these things.
> 
> Like so... seems to 'work'.

So I merged this one with 7/7 and this is the result, screenshot to
capture the colors:

  http://vger.kernel.org/~acme/perf/annotate_basic_blocks.png

Please let me know if I should go ahead and push with the combined
patch, that is now at:

https://git.kernel.org/cgit/linux/kernel/git/acme/linux.git/commit/?h=perf/annotate_basic_blocks&id=baf41a43fa439ac534d21e41882a7858d5cee1e5

git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux.git perf/annotate_basic_blocks

Is that ok?

The problem with it is that it is done only for --stdio, I'll check how
to properly make it UI agnostic...

- Arnaldo

> ---
>  tools/perf/util/annotate.c | 45 ++++++++++++++++++++++++++-------------------
>  1 file changed, 26 insertions(+), 19 deletions(-)
> 
> diff --git a/tools/perf/util/annotate.c b/tools/perf/util/annotate.c
> index 8eeb151..c78b16f0 100644
> --- a/tools/perf/util/annotate.c
> +++ b/tools/perf/util/annotate.c
> @@ -907,6 +907,7 @@ static void annotate__branch_printf(struct block_range *br, u64 addr)
>  #if 1
>  	if (br->is_target && br->start == addr) {
>  		struct block_range *branch = br;
> +		double p;
>  
>  		/*
>  		 * Find matching branch to our target.
> @@ -914,31 +915,37 @@ static void annotate__branch_printf(struct block_range *br, u64 addr)
>  		while (!branch->is_branch)
>  			branch = block_range__next(branch);
>  
> -		if (emit_comment) {
> -			emit_comment = false;
> -			printf("\t#");
> -		}
> +		p = 100 *(double)br->entry / branch->coverage;
>  
> -		/*
> -		 * The percentage of coverage joined at this target in relation
> -		 * to the next branch.
> -		 */
> -		printf(" +%.2f%%", 100*(double)br->entry / branch->coverage);
> +		if (p > 0.1) {
> +			if (emit_comment) {
> +				emit_comment = false;
> +				printf("\t#");
> +			}
> +
> +			/*
> +			 * The percentage of coverage joined at this target in relation
> +			 * to the next branch.
> +			 */
> +			printf(" +%.2f%%", p);
> +		}
>  	}
>  #endif
>  	if (br->is_branch && br->end == addr) {
> +		double p = 100*(double)br->taken / br->coverage;
>  
> -		if (emit_comment) {
> -			emit_comment = false;
> -			printf("\t#");
> -		}
> +		if (p > 0.1) {
> +			if (emit_comment) {
> +				emit_comment = false;
> +				printf("\t#");
> +			}
>  
> -		/*
> -		 * The percentage of coverage leaving at this branch, and
> -		 * its prediction ratio.
> -		 */
> -		printf(" -%.2f%% / %.2f%%", 100*(double)br->taken / br->coverage,
> -					    100*(double)br->pred  / br->taken);
> +			/*
> +			 * The percentage of coverage leaving at this branch, and
> +			 * its prediction ratio.
> +			 */
> +			printf(" -%.2f%% (p:%.2f%%)", p, 100*(double)br->pred  / br->taken);
> +		}
>  	}
>  }
>  