Re: [PATCH 0/4] perf tools: improve branch stack sampling

[Ingo Molnar <mingo@elte.hu>]

* Stephane Eranian <eranian@google.com> wrote:

> This patch set improves upon my LBR v6 patch series posted on
> LKML several weeks ago. It needs to be applied on top of it.
> 
> This series improves perf report and perf record when used
> with taken branch stack sampling.
> 
> For perf record, and based on users' feedback, it is possible
> to omit the branch type with the -b option. In that case,
> the filter is set to the default value of any branch type.
> The branch-filter option is now used to specify more precise
> branch types and privilege levels:
> 
>    $ perf record -b foo
>    equivalent to:
>    $ perf record --branch-filter any foo
> 
>    To filter on specific branch types:
>    $ perf record --branch-filter any_call foo
> 
> For perf report, it is now possible to omit the -b option
> and yet get branch view mode. Perf report is able to auto-detect
> if the perf.data file contains samples with branch stack and
> to switch to the branch mode. It is possible to override this
> by passing --no-branch-stack.
> 
> Furthermore, perf report now supports TUI in branch mode.
> It is possible to naviguate to both the source and destination
> functions of branches.
> 
> Signed-off-by: Stephane Eranian <eranian@google.com>
> 
> Stephane Eranian (4):
>   perf record: provide default branch stack sampling mode option
>   perf record: add HEADER_BRANCH_STACK tag
>   perf report: auto-detect branch stack sampling mode
>   perf report: enable TUI in branch view mode
> 
>  tools/perf/Documentation/perf-record.txt |   23 ++++--
>  tools/perf/Documentation/perf-report.txt |    7 +-
>  tools/perf/builtin-record.c              |   71 +++++++++++------
>  tools/perf/builtin-report.c              |  121 ++++++++++++++++++++---------
>  tools/perf/util/header.c                 |   13 +++
>  tools/perf/util/header.h                 |    2 +-
>  tools/perf/util/session.c                |    1 +
>  tools/perf/util/sort.c                   |    2 +-
>  tools/perf/util/sort.h                   |    8 +-
>  tools/perf/util/symbol.h                 |    7 ++-
>  tools/perf/util/ui/browsers/hists.c      |   62 ++++++++++++---
>  11 files changed, 225 insertions(+), 92 deletions(-)

Ok, I tried it out and this is now very usable. I particularly 
liked the detail how the TUI now allows annotation of both the 
source and the target symbols.

I've noticed two small usability glitches:

1) when annotating a symbol and typing 'q' to go back one level, 
   it goes back *two* levels. This is intuitive when just one 
   symbol is annotated, but confusing when there are two 
   symbols: I frequently want to go back to annotate the other 
   symbol as well.

2) two symbols are offered for annotation even when the source
   RIP and target RIP is within the same symbol.

Anyway, I've applied all your patches to 
tip:perf/hw-branch-sampling branch, and if the remaining issues 
are resolved will merge them into tip:perf/core for upstream 
merging. Depending on when the fixes arrive this could be v3.4 
or v3.5.

Thanks,

	Ingo