[tip:perf/core] perf ui annotate browser: Allow toggling addr offset view

[tip:perf/core] perf ui annotate browser: Allow toggling addr offset view

[tip-bot for Arnaldo Carvalho de Melo]

Commit-ID:  e235f3f3bf238eb092ad2fe7c35c6d7fd5dc2aeb
Gitweb:     http://git.kernel.org/tip/e235f3f3bf238eb092ad2fe7c35c6d7fd5dc2aeb
Author:     Arnaldo Carvalho de Melo <acme@redhat.com>
AuthorDate: Mon, 2 Apr 2012 13:21:55 -0300
Committer:  Arnaldo Carvalho de Melo <acme@redhat.com>
CommitDate: Sat, 7 Apr 2012 16:10:19 -0300

perf ui annotate browser: Allow toggling addr offset view

The lines in objdump have this format:

    ffffffff8126543f:       jne    ffffffff81265494 <__list_del_entry+0x84>
<SNIP>
    ffffffff81265494:       mov    %rdi,%rcx

Since we now have objdump_line allowing tools to print the offset
independently from the rest of the line, allow toggling a view where
just offsets from the start of the function are shown:

     2f:       jne    ffffffff81265494 <__list_del_entry+0x84>
<SNIP>
     84:       mov    %rdi,%rcx

The offset view will be the default as soon as operations that deal with
offsets in a function are handled accodringly, i.e. in offset view the
above will become:

     2f:       jne    __list_del_entry+0x84
<SNIP>
     84:       mov    %rdi,%rcx

And then a follow up patch will allow navigating thru jumps, just like
we handle callq instructions.

Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Namhyung Kim <namhyung@gmail.com>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Link: http://lkml.kernel.org/n/tip-4zpgimmz8xv7b5c920el7s45@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/ui/browsers/annotate.c |   15 ++++++++++++---
 1 files changed, 12 insertions(+), 3 deletions(-)

diff --git a/tools/perf/util/ui/browsers/annotate.c b/tools/perf/util/ui/browsers/annotate.c
index 7ac7dd0..5cf9b78 100644
--- a/tools/perf/util/ui/browsers/annotate.c
+++ b/tools/perf/util/ui/browsers/annotate.c
@@ -20,6 +20,7 @@ struct annotate_browser {
 	int		    nr_asm_entries;
 	int		    nr_entries;
 	bool		    hide_src_code;
+	bool		    use_offset;
 };
 
 struct objdump_line_rb_node {
@@ -82,10 +83,13 @@ static void annotate_browser__write(struct ui_browser *self, void *entry, int ro
 		slsmg_write_nstring(ol->line, width - 18);
 	else {
 		char bf[64];
-		u64 addr = ab->start + ol->offset;
-		int printed = scnprintf(bf, sizeof(bf), " %" PRIx64 ":", addr);
-		int color = -1;
+		u64 addr = ol->offset;
+		int printed, color = -1;
 
+		if (!ab->use_offset)
+			addr += ab->start;
+
+		printed = scnprintf(bf, sizeof(bf), " %" PRIx64 ":", addr);
 		if (change_color)
 			color = ui_browser__set_color(self, HE_COLORSET_ADDR);
 		slsmg_write_nstring(bf, printed);
@@ -250,6 +254,7 @@ static int annotate_browser__run(struct annotate_browser *self, int evidx,
 	struct symbol *sym = ms->sym;
 	const char *help = "<-/ESC: Exit, TAB/shift+TAB: Cycle hot lines, "
 			   "H: Go to hottest line, ->/ENTER: Line action, "
+			   "O: Toggle offset view, "
 			   "S: Toggle source code view";
 	int key;
 
@@ -310,6 +315,10 @@ static int annotate_browser__run(struct annotate_browser *self, int evidx,
 			if (annotate_browser__toggle_source(self))
 				ui_helpline__puts(help);
 			continue;
+		case 'O':
+		case 'o':
+			self->use_offset = !self->use_offset;
+			continue;
 		case K_ENTER:
 		case K_RIGHT:
 			if (self->selection == NULL) {

Re: [tip:perf/core] perf ui annotate browser: Allow toggling addr offset view

[Linus Torvalds]

On Fri, Apr 13, 2012 at 11:07 AM, tip-bot for Arnaldo Carvalho de Melo
<acme@redhat.com> wrote:
>
> The offset view will be the default as soon as operations that deal with
> offsets in a function are handled accodringly, i.e. in offset view the
> above will become:
>
>     2f:       jne    __list_del_entry+0x84
> <SNIP>
>     84:       mov    %rdi,%rcx

Oh, please please please make this happen.

Also, if at all possible, please mark offsets that are referenced by a
branch some way. I do think that the "size of instruction" can be a
very valid piece of information when looking at instruction-level
profiles, but quite frankly, I'm also 100% sure that the fact that an
instruction is a branch target is *more* important.

Now, fancy arrows etc might be too hard to do (especially without
cluttering things up too much), but marking just the targets would
already be lovely. IOW, turn this mess:


         :        ffffffff810d7e80 <kmem_cache_free>:
    1.91 :        ffffffff810d7e80:       push   %rbp
    0.05 :        ffffffff810d7e81:       mov    %rsp,%rbp
    2.02 :        ffffffff810d7e84:       push   %r12
    0.00 :        ffffffff810d7e86:       mov    %rdi,%r12
    0.55 :        ffffffff810d7e89:       push   %rbx
    0.00 :        ffffffff810d7e8a:       mov    %rsi,%rdi
    1.20 :        ffffffff810d7e8d:       mov    %rsi,%rbx
    1.64 :        ffffffff810d7e90:       callq  ffffffff8102a730 <__phys_addr>
    0.05 :        ffffffff810d7e95:       movabs $0xffffea0000000000,%rdi
    0.22 :        ffffffff810d7e9f:       shr    $0xc,%rax
    0.11 :        ffffffff810d7ea3:       shl    $0x6,%rax
    1.64 :        ffffffff810d7ea7:       lea    (%rax,%rdi,1),%rdi
   42.80 :        ffffffff810d7eab:       mov    (%rdi),%rax
    2.02 :        ffffffff810d7eae:       test   $0x80,%ah
    0.00 :        ffffffff810d7eb1:       jne    ffffffff810d7ef6
<kmem_cache_free+0x76>
    0.22 :        ffffffff810d7eb3:       mov    0x8(%rbp),%r9
    7.14 :        ffffffff810d7eb7:       mov    (%r12),%rax
    0.65 :        ffffffff810d7ebb:       add    %gs:0xcb88,%rax
    4.58 :        ffffffff810d7ec4:       mov    0x8(%rax),%rdx

(which has tons of unnecessary white-space too - shades of G+) into

<kmem_cache_free>:
    1.91 :          push   %rbp
    0.05 :          mov    %rsp,%rbp
    2.02 :          push   %r12
    0.00 :          mov    %rdi,%r12
    0.55 :          push   %rbx
    0.00 :          mov    %rsi,%rdi
    1.20 :          mov    %rsi,%rbx
    1.64 :          callq  ffffffff8102a730 <__phys_addr>
    0.05 :          movabs $0xffffea0000000000,%rdi
    0.22 :          shr    $0xc,%rax
    0.11 :          shl    $0x6,%rax
    1.64 :          lea    (%rax,%rdi,1),%rdi
   42.80 :          mov    (%rdi),%rax
    2.02 :          test   $0x80,%ah
    0.00 :          jne    kmem_cache_free+0x76
    0.22 : 0x33:    mov    0x8(%rbp),%r9
    7.14 : 0x37:    mov    (%r12),%rax
    0.65 :          add    %gs:0xcb88,%rax
    4.58 :          mov    0x8(%rax),%rdx
   ....

or something. I think the "callq  ffffffff8102a730 <__phys_addr>"
could also be prettified with *zero* downside.

Sure, leave some magic keycombination to get the "full information", but ..

                      Linus

Re: [tip:perf/core] perf ui annotate browser: Allow toggling addr offset view

[Linus Torvalds]

On Fri, Apr 13, 2012 at 11:25 AM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> <kmem_cache_free>:
>    1.91 :          push   %rbp

Oh, btw, talking about kmem_cache_free: that one uses altinstructions,
and so perf report shows the hottest instruction wrong (and I'm not
talking about "ugly"):

   12.38 :        ffffffff810d7ee5:       lea    (%r8),%rsi
    0.71 :        ffffffff810d7ee8:       callq  ffffffff812d3df0
<this_cpu_cmpxchg16b_emu>

that "lea" really isn't very expensive. In reality, it's not
"lea+call", it's a "lock ; cmpxchg16b + setz" instruction. But "perf"
doesn't know about alternative instructions, and if somebody were to
try to teach it, that would be lovely.

Happily, x86-64 doesn't have quite as many of them as x86-32 does. But
they are there, sometimes in interesting functions.

                 Linus

Re: [tip:perf/core] perf ui annotate browser: Allow toggling addr offset view

[Arnaldo Carvalho de Melo]

Em Fri, Apr 13, 2012 at 11:30:52AM -0700, Linus Torvalds escreveu:
> On Fri, Apr 13, 2012 at 11:25 AM, Linus Torvalds
> <torvalds@linux-foundation.org> wrote:
> >
> > <kmem_cache_free>:
> >    1.91 :          push   %rbp
> 
> Oh, btw, talking about kmem_cache_free: that one uses altinstructions,
> and so perf report shows the hottest instruction wrong (and I'm not
> talking about "ugly"):

Well, if we use Masami's disassembler we would use the actual code as it
is being used and not the original DSO that was later patched by
altinstructions.

We would have to dump it somehow in the ~/.debug/ cache so that we could
do offsite analysis, etc.

My plan is to move the objdump_line stuff to something that doesn't have
objdump in its name, i.e. something that would be generated by
disassembler sources.

The only now being binutils' objdump, but also Masami's disassembler and
probably elfutils eu-objdump after it implements a disassembler + adds
support for -debuginfo files, something that is needed to support
userspace with source code intermixed.
 
>    12.38 :        ffffffff810d7ee5:       lea    (%r8),%rsi
>     0.71 :        ffffffff810d7ee8:       callq  ffffffff812d3df0
> <this_cpu_cmpxchg16b_emu>
> 
> that "lea" really isn't very expensive. In reality, it's not
> "lea+call", it's a "lock ; cmpxchg16b + setz" instruction. But "perf"
> doesn't know about alternative instructions, and if somebody were to
> try to teach it, that would be lovely.
> 
> Happily, x86-64 doesn't have quite as many of them as x86-32 does. But
> they are there, sometimes in interesting functions.

>                  Linus

Re: [tip:perf/core] perf ui annotate browser: Allow toggling addr offset view

[Ingo Molnar]

* Arnaldo Carvalho de Melo <acme@redhat.com> wrote:

> Em Fri, Apr 13, 2012 at 11:30:52AM -0700, Linus Torvalds escreveu:
> > On Fri, Apr 13, 2012 at 11:25 AM, Linus Torvalds
> > <torvalds@linux-foundation.org> wrote:
> > >
> > > <kmem_cache_free>:
> > >    1.91 :          push   %rbp
> > 
> > Oh, btw, talking about kmem_cache_free: that one uses altinstructions,
> > and so perf report shows the hottest instruction wrong (and I'm not
> > talking about "ugly"):
> 
> Well, if we use Masami's disassembler we would use the actual 
> code as it is being used and not the original DSO that was 
> later patched by altinstructions.

Key would be to use the kernel's live RAM image of instructions. 

I.e. we should provide a live /proc/vmlinux image in essence: a 
'virtual' ELF binary image constructed out of the live kernel 
RAM image - with no extra RAM overhead. (Maybe with modules 
included in an intelligent way - although personally I don't use 
modules when I instrument the kernel)

That plus the always-available /proc/kallsyms would offer rather 
powerful annotation already: without *any* debug info - out of 
box, on any Linux installation. (This was always the main 
advantage of /proc/profile and readprofile btw: it worked 
everywhere while most other profiling solutions needed a 
debuginfo, etc.)

Doing /proc/vmlinux would be different from /dev/mem as it only 
shows the kernel RAM image, and only in a read-only fashion.

Default permissions of /proc/vmlinux should probably track 
console permissions: it should be possible to allow people 
sitting in front of the computer to read /proc/vmlinux, while 
people logged in over the network wouldn't.

Doing such a live kernel vmlinux would have other debugging and 
instrumentation advantages as well: various code patching 
effects could be checked and observed directly.

Thanks,

	Ingo

Re: [tip:perf/core] perf ui annotate browser: Allow toggling addr offset view

[Arnaldo Carvalho de Melo]

Em Sat, Apr 14, 2012 at 10:55:19AM +0200, Ingo Molnar escreveu:
> 
> * Arnaldo Carvalho de Melo <acme@redhat.com> wrote:
> 
> > Em Fri, Apr 13, 2012 at 11:30:52AM -0700, Linus Torvalds escreveu:
> > > On Fri, Apr 13, 2012 at 11:25 AM, Linus Torvalds
> > > <torvalds@linux-foundation.org> wrote:
> > > >
> > > > <kmem_cache_free>:
> > > >    1.91 :          push   %rbp
> > > 
> > > Oh, btw, talking about kmem_cache_free: that one uses altinstructions,
> > > and so perf report shows the hottest instruction wrong (and I'm not
> > > talking about "ugly"):
> > 
> > Well, if we use Masami's disassembler we would use the actual 
> > code as it is being used and not the original DSO that was 
> > later patched by altinstructions.
> 
> Key would be to use the kernel's live RAM image of instructions. 

Yeah, that is what I meant by "if we use Masami's disassembler" :-)
 
> I.e. we should provide a live /proc/vmlinux image in essence: a 
> 'virtual' ELF binary image constructed out of the live kernel 
> RAM image - with no extra RAM overhead. (Maybe with modules 
> included in an intelligent way - although personally I don't use 
> modules when I instrument the kernel)
> 
> That plus the always-available /proc/kallsyms would offer rather 
> powerful annotation already: without *any* debug info - out of 
> box, on any Linux installation. (This was always the main 
> advantage of /proc/profile and readprofile btw: it worked 
> everywhere while most other profiling solutions needed a 
> debuginfo, etc.)
> 
> Doing /proc/vmlinux would be different from /dev/mem as it only 
> shows the kernel RAM image, and only in a read-only fashion.
> 
> Default permissions of /proc/vmlinux should probably track 
> console permissions: it should be possible to allow people 
> sitting in front of the computer to read /proc/vmlinux, while 
> people logged in over the network wouldn't.
> 
> Doing such a live kernel vmlinux would have other debugging and 
> instrumentation advantages as well: various code patching 
> effects could be checked and observed directly.

I would say that even for userspace we would love to have such virtual
ELF files, as code patching is become more common...

- Arnaldo

Re: [tip:perf/core] perf ui annotate browser: Allow toggling addr offset view

[Masami Hiramatsu]

(2012/04/15 1:28), Arnaldo Carvalho de Melo wrote:
> Em Sat, Apr 14, 2012 at 10:55:19AM +0200, Ingo Molnar escreveu:
>>
>> * Arnaldo Carvalho de Melo <acme@redhat.com> wrote:
>>
>>> Em Fri, Apr 13, 2012 at 11:30:52AM -0700, Linus Torvalds escreveu:
>>>> On Fri, Apr 13, 2012 at 11:25 AM, Linus Torvalds
>>>> <torvalds@linux-foundation.org> wrote:
>>>>>
>>>>> <kmem_cache_free>:
>>>>>    1.91 :          push   %rbp
>>>>
>>>> Oh, btw, talking about kmem_cache_free: that one uses altinstructions,
>>>> and so perf report shows the hottest instruction wrong (and I'm not
>>>> talking about "ugly"):
>>>
>>> Well, if we use Masami's disassembler we would use the actual 
>>> code as it is being used and not the original DSO that was 
>>> later patched by altinstructions.
>>
>> Key would be to use the kernel's live RAM image of instructions. 
> 
> Yeah, that is what I meant by "if we use Masami's disassembler" :-)
>  
>> I.e. we should provide a live /proc/vmlinux image in essence: a 
>> 'virtual' ELF binary image constructed out of the live kernel 
>> RAM image - with no extra RAM overhead. (Maybe with modules 
>> included in an intelligent way - although personally I don't use 
>> modules when I instrument the kernel)
>>
>> That plus the always-available /proc/kallsyms would offer rather 
>> powerful annotation already: without *any* debug info - out of 
>> box, on any Linux installation. (This was always the main 
>> advantage of /proc/profile and readprofile btw: it worked 
>> everywhere while most other profiling solutions needed a 
>> debuginfo, etc.)
>>
>> Doing /proc/vmlinux would be different from /dev/mem as it only 
>> shows the kernel RAM image, and only in a read-only fashion.

If we can ignore permissions, we can check whether the instruction
at the event address is modified or not in /dev/mem.
And that is also useful for modules. That can be done in userspace
with using setuid'd tool.

>> Default permissions of /proc/vmlinux should probably track 
>> console permissions: it should be possible to allow people 
>> sitting in front of the computer to read /proc/vmlinux, while 
>> people logged in over the network wouldn't.
>>
>> Doing such a live kernel vmlinux would have other debugging and 
>> instrumentation advantages as well: various code patching 
>> effects could be checked and observed directly.
> 
> I would say that even for userspace we would love to have such virtual
> ELF files, as code patching is become more common...

Right, if we hope to handle it correctly, we'll need to handle
all self-modifying code in kernel/user space.

Thank you,

-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [tip:perf/core] perf ui annotate browser: Allow toggling addr offset view

[Linus Torvalds]

On Sun, Apr 15, 2012 at 11:25 PM, Masami Hiramatsu
<masami.hiramatsu.pt@hitachi.com> wrote:
>
> If we can ignore permissions, we can check whether the instruction
> at the event address is modified or not in /dev/mem.
> And that is also useful for modules. That can be done in userspace
> with using setuid'd tool.

No, please don't make a setuid tool for this.

It really would be *much* better if you could parse the
'altinstructions' section. You already have a lot of ELF parsing code.

Trust me, that's much better than trying to read the actual kernel
image - and makes things like off-line analysis easier too. The only
thing you need is the active CPU features (default to "current CPU
features"), rather than having to have the whole kernel image.

And no permission issues.

                        Linus

Re: [tip:perf/core] perf ui annotate browser: Allow toggling addr offset view

[Peter Zijlstra]

On Mon, 2012-04-16 at 08:02 -0700, Linus Torvalds wrote:
> 
> It really would be *much* better if you could parse the
> 'altinstructions' section. You already have a lot of ELF parsing code.
> 
altinstructions, altinstr_replacement, smp_locks, __jump_table,
parainstructions, __mcount_loc and maybe a few more.. and that doesn't
allow for kprobes.

But yeah, being able to grok some of that would be neat.

Re: [tip:perf/core] perf ui annotate browser: Allow toggling addr offset view

[Masami Hiramatsu]

(2012/04/17 0:21), Peter Zijlstra wrote:
> On Mon, 2012-04-16 at 08:02 -0700, Linus Torvalds wrote:
>>
>> It really would be *much* better if you could parse the
>> 'altinstructions' section. You already have a lot of ELF parsing code.
>>
> altinstructions, altinstr_replacement, smp_locks, __jump_table,
> parainstructions, __mcount_loc and maybe a few more.. and that doesn't
> allow for kprobes.
> 
> But yeah, being able to grok some of that would be neat.
> 

I sse. For the kprobes, at least live time, we can see where
kprobes are put via /sys/kernel/debug/kprobes/list.

Thank you,

-- 
Masami HIRAMATSU
Software Platform Research Dept. Linux Technology Center
Hitachi, Ltd., Yokohama Research Laboratory
E-mail: masami.hiramatsu.pt@hitachi.com

Re: [tip:perf/core] perf ui annotate browser: Allow toggling addr offset view

[Peter Zijlstra]

On Fri, 2012-04-13 at 22:04 -0300, Arnaldo Carvalho de Melo wrote:
> Well, if we use Masami's disassembler we would use the actual code as it
> is being used and not the original DSO that was later patched by
> altinstructions. 

Just noting that there's tons of disassemblers available, we need not
wait for Masami's one to complete.

Re: [tip:perf/core] perf ui annotate browser: Allow toggling addr offset view

[Arnaldo Carvalho de Melo]

Em Fri, Apr 13, 2012 at 11:25:50AM -0700, Linus Torvalds escreveu:
> On Fri, Apr 13, 2012 at 11:07 AM, tip-bot for Arnaldo Carvalho de Melo
> <acme@redhat.com> wrote:
> >
> > The offset view will be the default as soon as operations that deal with
> > offsets in a function are handled accodringly, i.e. in offset view the
> > above will become:
> >
> >     2f:       jne    __list_del_entry+0x84
> > <SNIP>
> >     84:       mov    %rdi,%rcx
> 
> Oh, please please please make this happen.

I will in the next days. Glad you like it.
 
> Also, if at all possible, please mark offsets that are referenced by a
> branch some way. I do think that the "size of instruction" can be a
> very valid piece of information when looking at instruction-level
> profiles, but quite frankly, I'm also 100% sure that the fact that an
> instruction is a branch target is *more* important.
> 
> Now, fancy arrows etc might be too hard to do (especially without
> cluttering things up too much), but marking just the targets would
> already be lovely. IOW, turn this mess:
> 
> 
>          :        ffffffff810d7e80 <kmem_cache_free>:
>     1.91 :        ffffffff810d7e80:       push   %rbp
>     0.05 :        ffffffff810d7e81:       mov    %rsp,%rbp
>     2.02 :        ffffffff810d7e84:       push   %r12
>     0.00 :        ffffffff810d7e86:       mov    %rdi,%r12
>     0.55 :        ffffffff810d7e89:       push   %rbx
>     0.00 :        ffffffff810d7e8a:       mov    %rsi,%rdi
>     1.20 :        ffffffff810d7e8d:       mov    %rsi,%rbx
>     1.64 :        ffffffff810d7e90:       callq  ffffffff8102a730 <__phys_addr>
>     0.05 :        ffffffff810d7e95:       movabs $0xffffea0000000000,%rdi
>     0.22 :        ffffffff810d7e9f:       shr    $0xc,%rax
>     0.11 :        ffffffff810d7ea3:       shl    $0x6,%rax
>     1.64 :        ffffffff810d7ea7:       lea    (%rax,%rdi,1),%rdi
>    42.80 :        ffffffff810d7eab:       mov    (%rdi),%rax
>     2.02 :        ffffffff810d7eae:       test   $0x80,%ah
>     0.00 :        ffffffff810d7eb1:       jne    ffffffff810d7ef6
> <kmem_cache_free+0x76>
>     0.22 :        ffffffff810d7eb3:       mov    0x8(%rbp),%r9
>     7.14 :        ffffffff810d7eb7:       mov    (%r12),%rax
>     0.65 :        ffffffff810d7ebb:       add    %gs:0xcb88,%rax
>     4.58 :        ffffffff810d7ec4:       mov    0x8(%rax),%rdx
> 
> (which has tons of unnecessary white-space too - shades of G+) into

Grin ;-)

Yeah, even the : column isn't needed I think.
 
> <kmem_cache_free>:
>     1.91 :          push   %rbp
>     0.05 :          mov    %rsp,%rbp
>     2.02 :          push   %r12
>     0.00 :          mov    %rdi,%r12
>     0.55 :          push   %rbx
>     0.00 :          mov    %rsi,%rdi
>     1.20 :          mov    %rsi,%rbx
>     1.64 :          callq  ffffffff8102a730 <__phys_addr>
>     0.05 :          movabs $0xffffea0000000000,%rdi
>     0.22 :          shr    $0xc,%rax
>     0.11 :          shl    $0x6,%rax
>     1.64 :          lea    (%rax,%rdi,1),%rdi
>    42.80 :          mov    (%rdi),%rax
>     2.02 :          test   $0x80,%ah
>     0.00 :          jne    kmem_cache_free+0x76
>     0.22 : 0x33:    mov    0x8(%rbp),%r9
>     7.14 : 0x37:    mov    (%r12),%rax
>     0.65 :          add    %gs:0xcb88,%rax
>     4.58 :          mov    0x8(%rax),%rdx
>    ....
> 
> or something. I think the "callq  ffffffff8102a730 <__phys_addr>"
> could also be prettified with *zero* downside.

Yeah, no need to see the address in default mode.
 
> Sure, leave some magic keycombination to get the "full information", but ..

Yeah, we have o to toggle offset view + s to toggle source code view,
etc. 

>                       Linus

Re: [tip:perf/core] perf ui annotate browser: Allow toggling addr offset view

[Ingo Molnar]

* Arnaldo Carvalho de Melo <acme@redhat.com> wrote:

> > Now, fancy arrows etc might be too hard to do (especially 
> > without cluttering things up too much), but marking just the 
> > targets would already be lovely. IOW, turn this mess:
> > 
> > 
> >          :        ffffffff810d7e80 <kmem_cache_free>:
> >     1.91 :        ffffffff810d7e80:       push   %rbp
> >     0.05 :        ffffffff810d7e81:       mov    %rsp,%rbp
> >     2.02 :        ffffffff810d7e84:       push   %r12
> >     0.00 :        ffffffff810d7e86:       mov    %rdi,%r12
> >     0.55 :        ffffffff810d7e89:       push   %rbx
> >     0.00 :        ffffffff810d7e8a:       mov    %rsi,%rdi
> >     1.20 :        ffffffff810d7e8d:       mov    %rsi,%rbx
> >     1.64 :        ffffffff810d7e90:       callq  ffffffff8102a730 <__phys_addr>
> >     0.05 :        ffffffff810d7e95:       movabs $0xffffea0000000000,%rdi
> >     0.22 :        ffffffff810d7e9f:       shr    $0xc,%rax
> >     0.11 :        ffffffff810d7ea3:       shl    $0x6,%rax
> >     1.64 :        ffffffff810d7ea7:       lea    (%rax,%rdi,1),%rdi
> >    42.80 :        ffffffff810d7eab:       mov    (%rdi),%rax
> >     2.02 :        ffffffff810d7eae:       test   $0x80,%ah
> >     0.00 :        ffffffff810d7eb1:       jne    ffffffff810d7ef6
> > <kmem_cache_free+0x76>
> >     0.22 :        ffffffff810d7eb3:       mov    0x8(%rbp),%r9
> >     7.14 :        ffffffff810d7eb7:       mov    (%r12),%rax
> >     0.65 :        ffffffff810d7ebb:       add    %gs:0xcb88,%rax
> >     4.58 :        ffffffff810d7ec4:       mov    0x8(%rax),%rdx
> > 
> > (which has tons of unnecessary white-space too - shades of G+) into
> 
> Grin ;-)
> 
> Yeah, even the : column isn't needed I think.

Such delineators are sometimes useful visually, it emulates a 
vertical line and makes it easier for the brain to ignore the 
left or right side. We'll fine-tune it.

The occasional color highlighting is useful too, as long as it 
is used sparingly.

> > Sure, leave some magic keycombination to get the "full 
> > information", but ..
> 
> Yeah, we have o to toggle offset view + s to toggle source 
> code view, etc.

I suspect the default view should be the 'most intelligent' mode 
of assembly output we can offer - with easy toggles for the rare 
cases where someone would like to do something special, plus 
.perfconfig overrides for those with weird workflows and those 
with a taste different from ours.

Thanks,

	Ingo

Re: [tip:perf/core] perf ui annotate browser: Allow toggling addr offset view

[Arnaldo Carvalho de Melo]

Em Sat, Apr 14, 2012 at 10:46:16AM +0200, Ingo Molnar escreveu:
> * Arnaldo Carvalho de Melo <acme@redhat.com> wrote:
> > > Sure, leave some magic keycombination to get the "full 
> > > information", but ..
> > 
> > Yeah, we have o to toggle offset view + s to toggle source 
> > code view, etc.
> 
> I suspect the default view should be the 'most intelligent' mode 
> of assembly output we can offer - with easy toggles for the rare 
> cases where someone would like to do something special, plus 
> .perfconfig overrides for those with weird workflows and those 
> with a taste different from ours.

Agreed, that is the plan.
 
- Arnaldo

Re: [tip:perf/core] perf ui annotate browser: Allow toggling addr offset view

[Mike Galbraith]

On Sat, 2012-04-14 at 10:46 +0200, Ingo Molnar wrote:

> I suspect the default view should be the 'most intelligent' mode 
> of assembly output we can offer - with easy toggles for the rare 
> cases where someone would like to do something special, plus 
> .perfconfig overrides for those with weird workflows and those 
> with a taste different from ours.

Maybe you just said that, but it sounded like the opposite.  If 'most
intelligent' means maximum circles and arrows for dummies, I agree ;-)

-Mike

Re: [tip:perf/core] perf ui annotate browser: Allow toggling addr offset view

[Ingo Molnar]

* Mike Galbraith <efault@gmx.de> wrote:

> On Sat, 2012-04-14 at 10:46 +0200, Ingo Molnar wrote:
> 
> > I suspect the default view should be the 'most intelligent' 
> > mode of assembly output we can offer - with easy toggles for 
> > the rare cases where someone would like to do something 
> > special, plus .perfconfig overrides for those with weird 
> > workflows and those with a taste different from ours.
> 
> Maybe you just said that, but it sounded like the opposite.  
> If 'most intelligent' means maximum circles and arrows for 
> dummies, I agree ;-)

LOL, yeah - I think the output that is the most obvious one to 
people versed in those concepts but not intimitely familiar with 
that specific output tends to be pretty close to the most 
intelligent one.

Thanks,

	Ingo