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

From: Stephane Eranian
Date: Thu Sep 08 2016 - 12:43:59 EST


Hi,

On Thu, Sep 8, 2016 at 9:18 AM, Arnaldo Carvalho de Melo
<acme@xxxxxxxxxx> wrote:
>
> 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?
>
I like the idea and yes, branch stack can be used for this, but I have
a hard time understanding the colored output.
What is the explanation for the color changes?
How do I interpret the percentages in the comments of the assembly:
-54.50% (p: 42%)
Why not have dedicated columns before the assembly with proper column headers?

As for the command line:

$ perf record -b my_workload

Will do it right, for both kernel and user by default.

If you want user level only, you can simply do:

$ perf record -b -e cycles:up my_workload

The branch stack inherit the priv level of the event automatically.

>
> 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);
> > + }
> > }
> > }
> >