Re: [GIT PULL 00/13] Annotation improvements (G+ edition)

Re: [GIT PULL 00/13] Annotation improvements (G+ edition)

[George Spelvin]

A simpler way might just be to use the gas syntax of appending a "b" or
"f" to the branch number, indicating whether the target is in front
or behind.

Gas permits integer branch labels to be re-used, so it's necessary to
specify a direction.

Another thing that might help a reader is a comment indicating the number
of branches to a target, divided by direction.  Targets of backward
branches are obviously more interesting from a performance standpoint,
as they're likely to be the start of loops.


If you *want* to re-use branch target labels, there's a simple algorithm
for assigning them, given a list of all branch origins for each target:

First, collapse the list of all branch origins into a range, listing
the first and last reference to the target.  (The target itself counts
as a reference.)

Then repeat the following for n=0, 1, 2, ... until there are
no more targets left:

In some arbitrary order (I suggest increasing order of range size),
consider each of the remaining targets.  If there are no other targets
within its range, it may be assigned the label n.  If there are other
targets within its range, postpone this target for a later pass, and
remove it from the list of remaining targets.

Once the decision has been made to postpone a target to a later pass, it
no longer counts as "in the range" for other targets considered this pass.
This resolves the intersecting branches case:

	je 1f
0:
	foo
1:
	jbe 0b

Here, the 1: target was considered first, and had to be postponed
because the 0: target was in its range.  Then the 0: target saw nothing
conflicting it its range, and so was assigned.

Because it's not possible to postpone every target (if this were about
to happen, there will be no remaining targets within the range of the
last target considered, so it will be assigned and not postponed),
each iteration will assign at least one target.

After considering all remaining targets, forget about the assigned ones,
incrmeent n, and go back and re-consider the postponed ones.

While the algorithm is potentially inefficient in contrived cases,
I don't think it'll be a huge problem in practice.

Even if you don't want to assign labels this way, a similar algorithm
can assign columns to branching arrows so they do not overlap.

Re: [GIT PULL 00/13] Annotation improvements (G+ edition)

[Arnaldo Carvalho de Melo]

After the bandaid, I see some strange constructs like (press 'o' to view
the original disassembly):

    0.00 ||  399f877dc6:v je     399f877dd5 <_int_free+0x735>
    0.00 ||  399f877dc8:  lock   decl (%r12)
    0.00 |+------877dcd:^ jne    399f87bd64 <_L_unlock_5659>
    0.00 |   399f877dd3:v jmp    399f877ddf <_int_free+0x73f> 

There is a bug above, the start_width for the arrow is bigger than it
should, will fix.

Pressing 'o' again this becomes:

    0.00 ||      v je     735
    0.00 ||        lock   decl (%r12)
    0.00 |+------^ jne    0
    0.00 |       v jmp    73f 

The 'jne 0' line, its a misparse of the <....> part, it ass-umed that a
+ was there all the time, which is not true, so I should instead do a
fixup when traversing the ->offsets array, looking for dl->target.ops =
0 and, since we know where the symbol starts at that point, fix it.

- Arnaldo

Re: [GIT PULL 00/13] Annotation improvements (G+ edition)

[Arnaldo Carvalho de Melo]

CCing Hagen, who also reported this crash on G+.

Em Wed, Apr 25, 2012 at 09:05:38AM +0200, Ingo Molnar escreveu:
> The new output looks very (very!) nice - but it is still a bit 
> fragile:

Glad you liked it :-)
 
> earth5:~/tip/tools/perf> ./perf record -a sleep 1
> [ perf record: Woken up 1 times to write data ]
> [ perf record: Captured and wrote 0.092 MB perf.data (~4008 samples) ]
> 
> earth5:~/tip/tools/perf> ./perf annotate _int_free
> perf: Segmentation fault
> 
> The crash happens here:
> 
> Program received signal SIGSEGV, Segmentation fault.
> 0x0000000000486cda in annotate_browser__mark_jump_targets (size=<optimized out>, 
>     browser=<optimized out>) at ui/browsers/annotate.c:704
> 704			bdlt->jump_target = true;
> 
> But other than that the new assembly output is awesome :-)

Humm...

683 static void annotate_browser__mark_jump_targets(struct annotate_browser *browser,
684							size_t size)
685	{
686		u64 offset;
687	
688		for (offset = 0; offset < size; ++offset) {
689			struct disasm_line *dl = browser->offsets[offset], *dlt;
690			struct browser_disasm_line *bdlt;
691	
692			if (!dl || !dl->ins || !ins__is_jump(dl->ins))
693				continue;
694	
695			if (dl->ops.target >= size) {
696				ui__error("jump to after symbol!\n"
697					  "size: %zx, jump target: %" PRIx64,
698					  size, dl->ops.target);
699				continue;
700			}
701	
702			dlt = browser->offsets[dl->ops.target];
703			bdlt = disasm_line__browser(dlt);
704			bdlt->jump_target = true;
705		}
706			
707	}

(gdb) p size
$5 = 2415
(gdb) p offset
$6 = 140
(gdb) p dl->ops.target
$7 = 143
(gdb) p browser->offsets[143]
$8 = (struct disasm_line *) 0x0
(gdb) p dl->name
$9 = 0x2363bd0 "je"
(gdb)

Really strange, the code assumed that at the jump target we would have an
assembly line, but only in the previous instruction offset we have a 'lock':

(gdb) p browser->offsets[144]
$10 = (struct disasm_line *) 0x0
(gdb) p browser->offsets[142]
$11 = (struct disasm_line *) 0x27bd620
(gdb) p browser->offsets[142]->name
$12 = 0x237a8a0 "lock"
(gdb)

I'll study this more, but for now I'll just check if there is a disasm_line
at dl->ops.target, i.e. a valid jump target.

- Arnaldo

Re: [GIT PULL 00/13] Annotation improvements (G+ edition)

[Ingo Molnar]

* Arnaldo Carvalho de Melo <acme@infradead.org> wrote:

> Hi Ingo,
> 
> 	Please consider pulling,
> 
> - Arnaldo
> 
> The following changes since commit a385ec4f11bdcf81af094c03e2444ee9b7fad2e5:
> 
>   Merge tag 'v3.4-rc2' into perf/core (2012-04-13 09:57:10 +0200)
> 
> are available in the git repository at:
> 
> 
>   git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux perf/annotate
> 
> for you to fetch changes up to 3f862fd076275c442dfe295eddb5650a6e0aecd4:
> 
>   perf annotate: Add missing jump variants (2012-04-19 17:10:12 -0300)
> 
> ----------------------------------------------------------------
> Annotate improvements
> 
> Now the default annotate browser uses a much more compact format, implementing
> suggestions made made by several people, notably Linus.
> 
> Here is part of the new __list_del_entry annotation:
> 
> __list_del_entry
>     8.47 │      push   %rbp
>     8.47 │      mov    (%rdi),%rdx
>    20.34 │      mov    $0xdead000000100100,%rcx
>     3.39 │      mov    0x8(%rdi),%rax
>     0.00 │      mov    %rsp,%rbp
>     1.69 │      cmp    %rcx,%rdx
>     0.00 │      je     43
>     1.69 │      mov    $0xdead000000200200,%rcx
>     3.39 │      cmp    %rcx,%rax
>     0.00 │      je     a3
>     5.08 │      mov    (%rax),%r8
>    18.64 │      cmp    %r8,%rdi
>     0.00 │      jne    84
>     1.69 │      mov    0x8(%rdx),%r8
>    25.42 │      cmp    %r8,%rdi
>     0.00 │      jne    65
>     1.69 │      mov    %rax,0x8(%rdx)
>     0.00 │      mov    %rdx,(%rax)
>     0.00 │      leaveq
>     0.00 │      retq
>     0.00 │ 43:  mov    %rdx,%r8
>     0.00 │      mov    %rdi,%rcx
>     0.00 │      mov    $0xffffffff817cd6a8,%rdx
>     0.00 │      mov    $0x31,%esi
>     0.00 │      mov    $0xffffffff817cd6e0,%rdi
>     0.00 │      xor    %eax,%eax
>     0.00 │      callq  ffffffff8104eab0 <warn_slowpath_fmt>
>     0.00 │      leaveq
>     0.00 │      retq
>     0.00 │ 65:  mov    %rdi,%rcx
>     0.00 │      mov    $0xffffffff817cd780,%rdx
>     0.00 │      mov    $0x3a,%esi
>     0.00 │      mov    $0xffffffff817cd6e0,%rdi
>     0.00 │      xor    %eax,%eax
>     0.00 │      callq  ffffffff8104eab0 <warn_slowpath_fmt>
>     0.00 │      leaveq
>     0.00 │      retq
> 
> The infrastructure is there to provide formatters for any instruction,
> like the one I'll do for call functions to elide the address.
> 
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> ----------------------------------------------------------------
> Arnaldo Carvalho de Melo (13):
>       perf annotate: Rename objdump_line to disasm_line
>       perf annotate: Parse instruction
>       perf annotate browser: Use the disasm_line instruction name and operand fields
>       perf annotate: Disassembler instruction parsing
>       perf annotate: Parse call targets earlier
>       perf annotate: Introduce scnprintf ins_ops method
>       perf annotate browser: Rename disasm_line_rb_node
>       perf symbols: Introduce symbol__size method
>       perf annotate browser: Hide non jump target addresses in offset mode
>       perf annotate browser: Align jump labels
>       perf annotate browser: Make lines more compact
>       perf annotate browser: Use a vertical line as percentage separator
>       perf annotate: Add missing jump variants
> 
>  tools/perf/ui/browsers/annotate.c |  323 +++++++++++++++++++++----------------
>  tools/perf/util/annotate.c        |  263 +++++++++++++++++++++++++-----
>  tools/perf/util/annotate.h        |   32 +++-
>  tools/perf/util/symbol.h          |    5 +
>  tools/perf/util/util.c            |   10 ++
>  tools/perf/util/util.h            |    2 +
>  6 files changed, 446 insertions(+), 189 deletions(-)

The new output looks very (very!) nice - but it is still a bit 
fragile:

earth5:~/tip/tools/perf> ./perf record -a sleep 1
[ perf record: Woken up 1 times to write data ]
[ perf record: Captured and wrote 0.092 MB perf.data (~4008 samples) ]

earth5:~/tip/tools/perf> ./perf annotate _int_free
perf: Segmentation fault

The crash happens here:

Program received signal SIGSEGV, Segmentation fault.
0x0000000000486cda in annotate_browser__mark_jump_targets (size=<optimized out>, 
    browser=<optimized out>) at ui/browsers/annotate.c:704
704			bdlt->jump_target = true;

But other than that the new assembly output is awesome :-)

Thanks,

	Ingo

Re: [GIT PULL 00/13] Annotation improvements (G+ edition)

[Arnaldo Carvalho de Melo]

Em Thu, Apr 19, 2012 at 05:31:04PM -0700, Linus Torvalds escreveu:
> On Thu, Apr 19, 2012 at 1:33 PM, Arnaldo Carvalho de Melo wrote:
> Ok, so I did a quick test-run, and things are definitely more readable.
> 
> However, from a "where is the loop" angle, the one thing that is
> actually fairly hard to visually see is the branches.
>
> Sure, you can find them, but they are not nearly as easy to pick out
> visually as the branch targets are, and I do wonder if it wouldn't
> make sense to try to make them stand out a bit more.
> 
> What is visually important is
>  (a) seeing that it's a branch
>  (b) seeing the direction of the branch (to pick up on loops more easily).
> 
> I suspect that doing a whole arrow is just going to be a horrible
> criss-crossing mess (although with some graphical UI it might not be
> as bad as with the textual one), but I think even just doing it in
> entirely local scope of the particular line that contains the branch
> would be perfectly fine.

You mean we could draw the arrow all the way from the branch to the
label when the cursor is on one of them (branch, label)?

It would avoid the criss-crossing mess, and with identing it would get
even easier to draw it, i.e. it would be a straight line with a tip just
below the branch and a "feather" just above the target label.
 
> This, for example, is a snippet of a pretty simple function
> (avc_lookup()) with a few branches and a few branch targets:
> 
>     2.08 │      test   %rdx,%rdx
>     0.00 │      jne    38
>     0.00 │      jmp    53
>     5.13 │ 30:  mov    (%rdx),%rdx
>     0.74 │      test   %rdx,%rdx
>     0.00 │      je     53
>     0.06 │ 38:  lea    -0x20(%rdx),%rax
>    59.64 │      cmp    -0x20(%rdx),%edi
>     0.40 │      jne    30
>    17.36 │      cmp    -0x18(%rdx),%cx
>     0.00 │      jne    30
>     1.91 │      cmp    0x4(%rax),%esi
>     0.00 │      jne    30
>     0.22 │      test   %rax,%rax
>     0.20 │      je     53
>     1.68 │      leaveq
>     1.90 │      retq
>     0.00 │ 53:  incl   %gs:0xe214
>     0.00 │      xor    %eax,%eax
> 
> and the branch targets are fairly easy to pick up, but seeing the
> branches - and their directions - themselves takes much more parsing.
> Ok, it's actually pretty simple above, because they have a faily
> obvious pattern, but in bigger functions they really are harder to
> pick out.
> 
> Now, what if the branch instruction was just followed with a simple
> "arrow up/down" thing, and to make the distinction really simple to
> see visually, make the indentation slightly different for the
> back/forwards case, so that you don't have to really look at the
> (crappy) arrow head to find possible loops (backwards jumps).
> 
> The above snippet would then become:
> 
>     2.08        test   %rdx,%rdx
>     0.00        jne    38                  ----v
>     0.00        jmp    53                  ----v
>     5.13   30:  mov    (%rdx),%rdx
>     0.74        test   %rdx,%rdx
>     0.00        je     53                  ----v
>     0.06   38:  lea    -0x20(%rdx),%rax
>    59.64        cmp    -0x20(%rdx),%edi
>     0.40        jne    30                      ----^
>    17.36        cmp    -0x18(%rdx),%cx
>     0.00        jne    30                      ----^
>     1.91        cmp    0x4(%rax),%esi
>     0.00        jne    30                      ----^
>     0.22        test   %rax,%rax
>     0.20        je     53                  ----v
>     1.68        leaveq
>     1.90        retq
>     0.00   53:  incl   %gs:0xe214
>     0.00        xor    %eax,%eax
> 
> where I think it's fairly easy to pick up the branches. Sure, once you
> really need to connect them, you do need to compare the offsets (easy
> in the above case, less obvious with big complex functions), but just
> making it easy to *see* the backwards branches makes it easier to see
> the loop, methinks.
> 
> I dunno. Just a suggestion. I definitely like the new format better
> already, regardless of any arrows.

Great, keep suggestions flowing. :-)

- Arnaldo

Re: [GIT PULL 00/13] Annotation improvements (G+ edition)

[Arnaldo Carvalho de Melo]

Em Thu, Apr 19, 2012 at 06:01:15PM -0600, David Ahern escreveu:
> Something hosed up on your send: I only have 9 of the 13, I don't
> see the missing ones here: https://lkml.org/lkml/2012/4/19/, and the
> subject line on the lkml summary page is messed up.

Subject line probably was git send-email asking if I wanted it to be
UTF-8, due to the vertical line characters :-\

The others I'm checking now, but its all at git.k.o.

- Arnaldo
 
> David
> 
> 
> On 4/19/12 2:33 PM, Arnaldo Carvalho de Melo wrote:
> >Hi Ingo,
> >
> >	Please consider pulling,
> >
> >- Arnaldo
> >
> >The following changes since commit a385ec4f11bdcf81af094c03e2444ee9b7fad2e5:
> >
> >   Merge tag 'v3.4-rc2' into perf/core (2012-04-13 09:57:10 +0200)
> >
> >are available in the git repository at:
> >
> >
> >   git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux perf/annotate
> >
> >for you to fetch changes up to 3f862fd076275c442dfe295eddb5650a6e0aecd4:
> >
> >   perf annotate: Add missing jump variants (2012-04-19 17:10:12 -0300)
> >
> >----------------------------------------------------------------
> >Annotate improvements
> >
> >Now the default annotate browser uses a much more compact format, implementing
> >suggestions made made by several people, notably Linus.
> >
> >Here is part of the new __list_del_entry annotation:
> >
> >__list_del_entry
> >     8.47 │      push   %rbp
> >     8.47 │      mov    (%rdi),%rdx
> >    20.34 │      mov    $0xdead000000100100,%rcx
> >     3.39 │      mov    0x8(%rdi),%rax
> >     0.00 │      mov    %rsp,%rbp
> >     1.69 │      cmp    %rcx,%rdx
> >     0.00 │      je     43
> >     1.69 │      mov    $0xdead000000200200,%rcx
> >     3.39 │      cmp    %rcx,%rax
> >     0.00 │      je     a3
> >     5.08 │      mov    (%rax),%r8
> >    18.64 │      cmp    %r8,%rdi
> >     0.00 │      jne    84
> >     1.69 │      mov    0x8(%rdx),%r8
> >    25.42 │      cmp    %r8,%rdi
> >     0.00 │      jne    65
> >     1.69 │      mov    %rax,0x8(%rdx)
> >     0.00 │      mov    %rdx,(%rax)
> >     0.00 │      leaveq
> >     0.00 │      retq
> >     0.00 │ 43:  mov    %rdx,%r8
> >     0.00 │      mov    %rdi,%rcx
> >     0.00 │      mov    $0xffffffff817cd6a8,%rdx
> >     0.00 │      mov    $0x31,%esi
> >     0.00 │      mov    $0xffffffff817cd6e0,%rdi
> >     0.00 │      xor    %eax,%eax
> >     0.00 │      callq  ffffffff8104eab0<warn_slowpath_fmt>
> >     0.00 │      leaveq
> >     0.00 │      retq
> >     0.00 │ 65:  mov    %rdi,%rcx
> >     0.00 │      mov    $0xffffffff817cd780,%rdx
> >     0.00 │      mov    $0x3a,%esi
> >     0.00 │      mov    $0xffffffff817cd6e0,%rdi
> >     0.00 │      xor    %eax,%eax
> >     0.00 │      callq  ffffffff8104eab0<warn_slowpath_fmt>
> >     0.00 │      leaveq
> >     0.00 │      retq
> >
> >The infrastructure is there to provide formatters for any instruction,
> >like the one I'll do for call functions to elide the address.
> >
> >Signed-off-by: Arnaldo Carvalho de Melo<acme@redhat.com>
> >
> >----------------------------------------------------------------
> >Arnaldo Carvalho de Melo (13):
> >       perf annotate: Rename objdump_line to disasm_line
> >       perf annotate: Parse instruction
> >       perf annotate browser: Use the disasm_line instruction name and operand fields
> >       perf annotate: Disassembler instruction parsing
> >       perf annotate: Parse call targets earlier
> >       perf annotate: Introduce scnprintf ins_ops method
> >       perf annotate browser: Rename disasm_line_rb_node
> >       perf symbols: Introduce symbol__size method
> >       perf annotate browser: Hide non jump target addresses in offset mode
> >       perf annotate browser: Align jump labels
> >       perf annotate browser: Make lines more compact
> >       perf annotate browser: Use a vertical line as percentage separator
> >       perf annotate: Add missing jump variants
> >
> >  tools/perf/ui/browsers/annotate.c |  323 +++++++++++++++++++++----------------
> >  tools/perf/util/annotate.c        |  263 +++++++++++++++++++++++++-----
> >  tools/perf/util/annotate.h        |   32 +++-
> >  tools/perf/util/symbol.h          |    5 +
> >  tools/perf/util/util.c            |   10 ++
> >  tools/perf/util/util.h            |    2 +
> >  6 files changed, 446 insertions(+), 189 deletions(-)

Re: [GIT PULL 00/13] Annotation improvements (G+ edition)

[Linus Torvalds]

On Thu, Apr 19, 2012 at 5:31 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> On Thu, Apr 19, 2012 at 1:33 PM, Arnaldo Carvalho de Melo
> (crappy) arrow head to find possible loops (backwards jumps).
>
> The above snippet would then become:
>
>    2.08        test   %rdx,%rdx
>    0.00        jne    38                  ----v
>    0.00        jmp    53                  ----v
>    5.13   30:  mov    (%rdx),%rdx
>    0.74        test   %rdx,%rdx
>    0.00        je     53                  ----v
>    0.06   38:  lea    -0x20(%rdx),%rax
>   59.64        cmp    -0x20(%rdx),%edi
>    0.40        jne    30                      ----^
>   17.36        cmp    -0x18(%rdx),%cx
>    0.00        jne    30                      ----^
>    1.91        cmp    0x4(%rax),%esi
>    0.00        jne    30                      ----^
>    0.22        test   %rax,%rax
>    0.20        je     53                  ----v
>    1.68        leaveq
>    1.90        retq
>    0.00   53:  incl   %gs:0xe214
>    0.00        xor    %eax,%eax

Side note: I'm not at all certain that the arrows should be at the end
of the line. It might be even better to show it at the head of the
line, ie something like


    2.08        test   %rdx,%rdx
    0.00 v      jne    38
    0.00 v      jmp    53
    5.13   30:  mov    (%rdx),%rdx
    0.74        test   %rdx,%rdx
    0.00 v      je     53
    0.06   38:  lea    -0x20(%rdx),%rax
   59.64        cmp    -0x20(%rdx),%edi
    0.40 ^--    jne    30
   17.36        cmp    -0x18(%rdx),%cx
    0.00 ^--    jne    30
    1.91        cmp    0x4(%rax),%esi
    0.00 ^--    jne    30
    0.22        test   %rax,%rax
    0.20 v      je     53
    1.68        leaveq
    1.90        retq
    0.00   53:  incl   %gs:0xe214
    0.00        xor    %eax,%eax

but then you obviously need to make it work right if a branch is also
a branch target.

Oh, and one other visual clue that could be useful: unconditional
branches (including "ret") that actually end the flow of code and
don't have a fallthrough to the next instruction would be nice to see
clearly. Even perhaps an empty line following them? Although maybe
that would be wasting precious vertical space..

                              Linus

Re: [GIT PULL 00/13] Annotation improvements (G+ edition)

[Linus Torvalds]

On Thu, Apr 19, 2012 at 1:33 PM, Arnaldo Carvalho de Melo
<acme@infradead.org> wrote:
>
> Annotate improvements
>
> Now the default annotate browser uses a much more compact format, implementing
> suggestions made made by several people, notably Linus.
>
> Here is part of the new __list_del_entry annotation:

Ok, so I did a quick test-run, and things are definitely more readable.

However, from a "where is the loop" angle, the one thing that is
actually fairly hard to visually see is the branches.

Sure, you can find them, but they are not nearly as easy to pick out
visually as the branch targets are, and I do wonder if it wouldn't
make sense to try to make them stand out a bit more.

What is visually important is
 (a) seeing that it's a branch
 (b) seeing the direction of the branch (to pick up on loops more easily).

I suspect that doing a whole arrow is just going to be a horrible
criss-crossing mess (although with some graphical UI it might not be
as bad as with the textual one), but I think even just doing it in
entirely local scope of the particular line that contains the branch
would be perfectly fine.

This, for example, is a snippet of a pretty simple function
(avc_lookup()) with a few branches and a few branch targets:

    2.08 │      test   %rdx,%rdx
    0.00 │      jne    38
    0.00 │      jmp    53
    5.13 │ 30:  mov    (%rdx),%rdx
    0.74 │      test   %rdx,%rdx
    0.00 │      je     53
    0.06 │ 38:  lea    -0x20(%rdx),%rax
   59.64 │      cmp    -0x20(%rdx),%edi
    0.40 │      jne    30
   17.36 │      cmp    -0x18(%rdx),%cx
    0.00 │      jne    30
    1.91 │      cmp    0x4(%rax),%esi
    0.00 │      jne    30
    0.22 │      test   %rax,%rax
    0.20 │      je     53
    1.68 │      leaveq
    1.90 │      retq
    0.00 │ 53:  incl   %gs:0xe214
    0.00 │      xor    %eax,%eax

and the branch targets are fairly easy to pick up, but seeing the
branches - and their directions - themselves takes much more parsing.
Ok, it's actually pretty simple above, because they have a faily
obvious pattern, but in bigger functions they really are harder to
pick out.

Now, what if the branch instruction was just followed with a simple
"arrow up/down" thing, and to make the distinction really simple to
see visually, make the indentation slightly different for the
back/forwards case, so that you don't have to really look at the
(crappy) arrow head to find possible loops (backwards jumps).

The above snippet would then become:

    2.08        test   %rdx,%rdx
    0.00        jne    38                  ----v
    0.00        jmp    53                  ----v
    5.13   30:  mov    (%rdx),%rdx
    0.74        test   %rdx,%rdx
    0.00        je     53                  ----v
    0.06   38:  lea    -0x20(%rdx),%rax
   59.64        cmp    -0x20(%rdx),%edi
    0.40        jne    30                      ----^
   17.36        cmp    -0x18(%rdx),%cx
    0.00        jne    30                      ----^
    1.91        cmp    0x4(%rax),%esi
    0.00        jne    30                      ----^
    0.22        test   %rax,%rax
    0.20        je     53                  ----v
    1.68        leaveq
    1.90        retq
    0.00   53:  incl   %gs:0xe214
    0.00        xor    %eax,%eax

where I think it's fairly easy to pick up the branches. Sure, once you
really need to connect them, you do need to compare the offsets (easy
in the above case, less obvious with big complex functions), but just
making it easy to *see* the backwards branches makes it easier to see
the loop, methinks.

I dunno. Just a suggestion. I definitely like the new format better
already, regardless of any arrows.

                   Linus

Re: [GIT PULL 00/13] Annotation improvements (G+ edition)

[David Ahern]

Something hosed up on your send: I only have 9 of the 13, I don't see 
the missing ones here: https://lkml.org/lkml/2012/4/19/, and the subject 
line on the lkml summary page is messed up.

David


On 4/19/12 2:33 PM, Arnaldo Carvalho de Melo wrote:
> Hi Ingo,
>
> 	Please consider pulling,
>
> - Arnaldo
>
> The following changes since commit a385ec4f11bdcf81af094c03e2444ee9b7fad2e5:
>
>    Merge tag 'v3.4-rc2' into perf/core (2012-04-13 09:57:10 +0200)
>
> are available in the git repository at:
>
>
>    git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux perf/annotate
>
> for you to fetch changes up to 3f862fd076275c442dfe295eddb5650a6e0aecd4:
>
>    perf annotate: Add missing jump variants (2012-04-19 17:10:12 -0300)
>
> ----------------------------------------------------------------
> Annotate improvements
>
> Now the default annotate browser uses a much more compact format, implementing
> suggestions made made by several people, notably Linus.
>
> Here is part of the new __list_del_entry annotation:
>
> __list_del_entry
>      8.47 │      push   %rbp
>      8.47 │      mov    (%rdi),%rdx
>     20.34 │      mov    $0xdead000000100100,%rcx
>      3.39 │      mov    0x8(%rdi),%rax
>      0.00 │      mov    %rsp,%rbp
>      1.69 │      cmp    %rcx,%rdx
>      0.00 │      je     43
>      1.69 │      mov    $0xdead000000200200,%rcx
>      3.39 │      cmp    %rcx,%rax
>      0.00 │      je     a3
>      5.08 │      mov    (%rax),%r8
>     18.64 │      cmp    %r8,%rdi
>      0.00 │      jne    84
>      1.69 │      mov    0x8(%rdx),%r8
>     25.42 │      cmp    %r8,%rdi
>      0.00 │      jne    65
>      1.69 │      mov    %rax,0x8(%rdx)
>      0.00 │      mov    %rdx,(%rax)
>      0.00 │      leaveq
>      0.00 │      retq
>      0.00 │ 43:  mov    %rdx,%r8
>      0.00 │      mov    %rdi,%rcx
>      0.00 │      mov    $0xffffffff817cd6a8,%rdx
>      0.00 │      mov    $0x31,%esi
>      0.00 │      mov    $0xffffffff817cd6e0,%rdi
>      0.00 │      xor    %eax,%eax
>      0.00 │      callq  ffffffff8104eab0<warn_slowpath_fmt>
>      0.00 │      leaveq
>      0.00 │      retq
>      0.00 │ 65:  mov    %rdi,%rcx
>      0.00 │      mov    $0xffffffff817cd780,%rdx
>      0.00 │      mov    $0x3a,%esi
>      0.00 │      mov    $0xffffffff817cd6e0,%rdi
>      0.00 │      xor    %eax,%eax
>      0.00 │      callq  ffffffff8104eab0<warn_slowpath_fmt>
>      0.00 │      leaveq
>      0.00 │      retq
>
> The infrastructure is there to provide formatters for any instruction,
> like the one I'll do for call functions to elide the address.
>
> Signed-off-by: Arnaldo Carvalho de Melo<acme@redhat.com>
>
> ----------------------------------------------------------------
> Arnaldo Carvalho de Melo (13):
>        perf annotate: Rename objdump_line to disasm_line
>        perf annotate: Parse instruction
>        perf annotate browser: Use the disasm_line instruction name and operand fields
>        perf annotate: Disassembler instruction parsing
>        perf annotate: Parse call targets earlier
>        perf annotate: Introduce scnprintf ins_ops method
>        perf annotate browser: Rename disasm_line_rb_node
>        perf symbols: Introduce symbol__size method
>        perf annotate browser: Hide non jump target addresses in offset mode
>        perf annotate browser: Align jump labels
>        perf annotate browser: Make lines more compact
>        perf annotate browser: Use a vertical line as percentage separator
>        perf annotate: Add missing jump variants
>
>   tools/perf/ui/browsers/annotate.c |  323 +++++++++++++++++++++----------------
>   tools/perf/util/annotate.c        |  263 +++++++++++++++++++++++++-----
>   tools/perf/util/annotate.h        |   32 +++-
>   tools/perf/util/symbol.h          |    5 +
>   tools/perf/util/util.c            |   10 ++
>   tools/perf/util/util.h            |    2 +
>   6 files changed, 446 insertions(+), 189 deletions(-)