Hi Ingo,
Please consider pulling,
- Arnaldo
The following changes since commit 38b31bd0cefbb0e69a182d9a94b09a7e648549dc:
perf annotate browser: Don't draw jump connectors for out of function jumps (2012-04-25 14:18:42 -0300)
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 0822cc80d9aee026b1ebe43c02dc01e0a0227864:
perf annotate browser: Don't display 0.00 percentages (2012-04-27 17:13:53 -0300)
----------------------------------------------------------------
Perf annotate improvements and fixes:
. Current output:
avtab_search_node
push %rbp
mov %rsp,%rbp
→ callq mcount
movzwl 0x6(%rsi),%edx
and $0x7fff,%dx
test %rdi,%rdi
┌─────↓ jne 20
│ 17: xor %eax,%eax
│ 19: leaveq
│ ← retq
│ nopl 0x0(%rax,%rax,1)
└─→20: mov (%rdi),%rax
test %rax,%rax
↑ je 17
movzwl (%rsi),%ecx
movzwl 0x2(%rsi),%r9d
movzwl 0x4(%rsi),%r8d
movzwl %cx,%esi
movzwl %r9w,%r10d
shl $0x9,%esi
lea (%rsi,%r10,4),%esi
lea (%r8,%rsi,1),%esi
and 0x10(%rdi),%si
movzwl %si,%esi
mov (%rax,%rsi,8),%rax
1.63 test %rax,%rax
↑ je 19
nopw 0x0(%rax,%rax,1)
4.88 60: cmp %cx,(%rax)
↓ jne 7e
cmp %r9w,0x2(%rax)
↓ jne 7e
cmp %r8w,0x4(%rax)
↓ jne 79
test %dx,0x6(%rax)
↑ jne 19
79: cmp %r8w,0x4(%rax)
86.99 7e:↑ ja 17
3.25 mov 0x10(%rax),%rax
3.25 test %rax,%rax
↑ jne 60
leaveq
← retq
. Changes:
- Don't show the big vertical line.
- Add an arrow to the right before call instructions
- Scrap bogus loop detection and instead start showing
arrows from jump (fwd or back) instructions to its targets
when cursor is on jump instruction. Press 'j' to toggle this.
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
----------------------------------------------------------------
Arnaldo Carvalho de Melo (5):
perf annotate browser: Add a right arrow before call instructions
perf ui browser: Add method to draw up/down arrow line
perf annotate browser: Show current jump, back or forward
perf annotate browser: Remove the vertical line after the percentages
perf annotate browser: Don't display 0.00 percentages
tools/perf/ui/browser.c | 54 ++++++++++++++++++++++++++++--
tools/perf/ui/browser.h | 4 +--
tools/perf/ui/browsers/annotate.c | 66 ++++++++++++++++++-------------------
3 files changed, 86 insertions(+), 38 deletions(-)
From: Arnaldo Carvalho de Melo <[email protected]>
It is confusing when used with jump -> target lines.
Requested-by: Linus Torvalds <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/ui/browsers/annotate.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index d203daf..a90680b 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -72,7 +72,6 @@ static void annotate_browser__write(struct ui_browser *self, void *entry, int ro
slsmg_write_nstring(" ", 9);
}
- ui_browser__write_graph(self, SLSMG_VLINE_CHAR);
SLsmg_write_char(' ');
/* The scroll bar isn't being used */
@@ -83,9 +82,9 @@ static void annotate_browser__write(struct ui_browser *self, void *entry, int ro
ui_browser__set_color(self, HE_COLORSET_CODE);
if (!*dl->line)
- slsmg_write_nstring(" ", width - 10);
+ slsmg_write_nstring(" ", width - 9);
else if (dl->offset == -1)
- slsmg_write_nstring(dl->line, width - 10);
+ slsmg_write_nstring(dl->line, width - 9);
else {
char bf[256];
u64 addr = dl->offset;
@@ -138,7 +137,7 @@ static void annotate_browser__write(struct ui_browser *self, void *entry, int ro
scnprintf(bf, sizeof(bf), "%-6.6s %s", dl->name, dl->ops.raw);
}
- slsmg_write_nstring(bf, width - 12 - printed);
+ slsmg_write_nstring(bf, width - 11 - printed);
}
if (current_entry)
@@ -176,7 +175,7 @@ static void annotate_browser__draw_current_jump(struct ui_browser *browser)
if (!bcursor->jump_target)
start_width += ab->offset_width + 1;
- __ui_browser__line_arrow(browser, 10, from, to, start_width);
+ __ui_browser__line_arrow(browser, 9, from, to, start_width);
}
static unsigned int annotate_browser__refresh(struct ui_browser *browser)
--
1.7.9.2.358.g22243
From: Arnaldo Carvalho de Melo <[email protected]>
Instead of trying to show the current loop by naively looking for the
next backward jump, just use 'j' to toggle showing arrows connecting
jump with its target.
And do it for forward jumps as well.
Loop detection requires more code to follow the flow control, etc.
Reported-by: Linus Torvalds <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/ui/browsers/annotate.c | 50 +++++++++++++++++--------------------
1 file changed, 23 insertions(+), 27 deletions(-)
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 4778172..d203daf 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -30,6 +30,7 @@ struct annotate_browser {
int nr_entries;
bool hide_src_code;
bool use_offset;
+ bool jump_arrows;
bool searching_backwards;
u8 offset_width;
char search_bf[128];
@@ -144,56 +145,47 @@ static void annotate_browser__write(struct ui_browser *self, void *entry, int ro
ab->selection = dl;
}
-static void annotate_browser__draw_current_loop(struct ui_browser *browser)
+static void annotate_browser__draw_current_jump(struct ui_browser *browser)
{
struct annotate_browser *ab = container_of(browser, struct annotate_browser, b);
- struct map_symbol *ms = browser->priv;
- struct symbol *sym = ms->sym;
- struct annotation *notes = symbol__annotation(sym);
- struct disasm_line *cursor = ab->selection, *pos = cursor, *target;
- struct browser_disasm_line *bcursor = disasm_line__browser(cursor),
- *btarget, *bpos;
+ struct disasm_line *cursor = ab->selection, *target;
+ struct browser_disasm_line *btarget, *bcursor;
unsigned int from, to, start_width = 2;
- list_for_each_entry_from(pos, ¬es->src->source, node) {
- if (!pos->ins || !ins__is_jump(pos->ins) ||
- !disasm_line__has_offset(pos))
- continue;
-
- target = ab->offsets[pos->ops.target.offset];
- if (!target)
- continue;
+ if (!cursor->ins || !ins__is_jump(cursor->ins) ||
+ !disasm_line__has_offset(cursor))
+ return;
- btarget = disasm_line__browser(target);
- if (btarget->idx <= bcursor->idx)
- goto found;
- }
+ target = ab->offsets[cursor->ops.target.offset];
+ if (!target)
+ return;
- return;
+ bcursor = disasm_line__browser(cursor);
+ btarget = disasm_line__browser(target);
-found:
- bpos = disasm_line__browser(pos);
if (ab->hide_src_code) {
- from = bpos->idx_asm;
+ from = bcursor->idx_asm;
to = btarget->idx_asm;
} else {
- from = (u64)bpos->idx;
+ from = (u64)bcursor->idx;
to = (u64)btarget->idx;
}
ui_browser__set_color(browser, HE_COLORSET_CODE);
- if (!bpos->jump_target)
+ if (!bcursor->jump_target)
start_width += ab->offset_width + 1;
- __ui_browser__line_arrow_up(browser, 10, from, to, start_width);
+ __ui_browser__line_arrow(browser, 10, from, to, start_width);
}
static unsigned int annotate_browser__refresh(struct ui_browser *browser)
{
+ struct annotate_browser *ab = container_of(browser, struct annotate_browser, b);
int ret = ui_browser__list_head_refresh(browser);
- annotate_browser__draw_current_loop(browser);
+ if (ab->jump_arrows)
+ annotate_browser__draw_current_jump(browser);
return ret;
}
@@ -628,6 +620,9 @@ static int annotate_browser__run(struct annotate_browser *self, int evidx,
case 'o':
self->use_offset = !self->use_offset;
continue;
+ case 'j':
+ self->jump_arrows = !self->jump_arrows;
+ continue;
case '/':
if (annotate_browser__search(self, delay_secs)) {
show_help:
@@ -739,6 +734,7 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
.use_navkeypressed = true,
},
.use_offset = true,
+ .jump_arrows = true,
};
int ret = -1;
--
1.7.9.2.358.g22243
From: Arnaldo Carvalho de Melo <[email protected]>
It figures out the direction and draws downwards arrows too if that is
the case.
Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/ui/browser.c | 54 +++++++++++++++++++++++++++++++++++++++++++++--
tools/perf/ui/browser.h | 4 ++--
2 files changed, 54 insertions(+), 4 deletions(-)
diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
index 32ac116..f4b2530 100644
--- a/tools/perf/ui/browser.c
+++ b/tools/perf/ui/browser.c
@@ -600,8 +600,9 @@ void ui_browser__write_graph(struct ui_browser *browser __used, int graph)
SLsmg_set_char_set(0);
}
-void __ui_browser__line_arrow_up(struct ui_browser *browser, unsigned int column,
- u64 start, u64 end, int start_width)
+static void __ui_browser__line_arrow_up(struct ui_browser *browser,
+ unsigned int column,
+ u64 start, u64 end, int start_width)
{
unsigned int row, end_row;
@@ -639,6 +640,55 @@ out:
SLsmg_set_char_set(0);
}
+static void __ui_browser__line_arrow_down(struct ui_browser *browser,
+ unsigned int column,
+ u64 start, u64 end, int start_width)
+{
+ unsigned int row, end_row;
+
+ SLsmg_set_char_set(1);
+
+ if (start >= browser->top_idx) {
+ row = start - browser->top_idx;
+ ui_browser__gotorc(browser, row, column);
+ SLsmg_write_char(SLSMG_ULCORN_CHAR);
+ ui_browser__gotorc(browser, row, column + 1);
+ SLsmg_draw_hline(start_width);
+
+ if (row++ == 0)
+ goto out;
+ } else
+ row = 0;
+
+ if (end >= browser->top_idx + browser->height)
+ end_row = browser->height - 1;
+ else
+ end_row = end - browser->top_idx;;
+
+ ui_browser__gotorc(browser, row, column);
+ SLsmg_draw_vline(end_row - row + 1);
+
+ ui_browser__gotorc(browser, end_row, column);
+ if (end < browser->top_idx + browser->height) {
+ SLsmg_write_char(SLSMG_LLCORN_CHAR);
+ ui_browser__gotorc(browser, end_row, column + 1);
+ SLsmg_write_char(SLSMG_HLINE_CHAR);
+ ui_browser__gotorc(browser, end_row, column + 2);
+ SLsmg_write_char(SLSMG_RARROW_CHAR);
+ }
+out:
+ SLsmg_set_char_set(0);
+}
+
+void __ui_browser__line_arrow(struct ui_browser *browser, unsigned int column,
+ u64 start, u64 end, int start_width)
+{
+ if (start > end)
+ __ui_browser__line_arrow_up(browser, column, start, end, start_width);
+ else
+ __ui_browser__line_arrow_down(browser, column, start, end, start_width);
+}
+
void ui_browser__init(void)
{
int i = 0;
diff --git a/tools/perf/ui/browser.h b/tools/perf/ui/browser.h
index 2f226cb..059764b 100644
--- a/tools/perf/ui/browser.h
+++ b/tools/perf/ui/browser.h
@@ -38,8 +38,8 @@ void ui_browser__reset_index(struct ui_browser *self);
void ui_browser__gotorc(struct ui_browser *self, int y, int x);
void ui_browser__write_graph(struct ui_browser *browser, int graph);
-void __ui_browser__line_arrow_up(struct ui_browser *browser, unsigned int column,
- u64 start, u64 end, int start_width);
+void __ui_browser__line_arrow(struct ui_browser *browser, unsigned int column,
+ u64 start, u64 end, int start_width);
void __ui_browser__show_title(struct ui_browser *browser, const char *title);
void ui_browser__show_title(struct ui_browser *browser, const char *title);
int ui_browser__show(struct ui_browser *self, const char *title,
--
1.7.9.2.358.g22243
From: Arnaldo Carvalho de Melo <[email protected]>
The counterpart of 'ret' instructions.
Suggested-by: Linus Torvalds <[email protected]>
Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/ui/browsers/annotate.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 077380b..4778172 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -117,6 +117,9 @@ static void annotate_browser__write(struct ui_browser *self, void *entry, int ro
ui_browser__write_graph(self, fwd ? SLSMG_DARROW_CHAR :
SLSMG_UARROW_CHAR);
SLsmg_write_char(' ');
+ } else if (ins__is_call(dl->ins)) {
+ ui_browser__write_graph(self, SLSMG_RARROW_CHAR);
+ SLsmg_write_char(' ');
} else {
slsmg_write_nstring(" ", 2);
}
--
1.7.9.2.358.g22243
From: Arnaldo Carvalho de Melo <[email protected]>
Cleaning up more the output.
Cc: David Ahern <[email protected]>
Cc: Frederic Weisbecker <[email protected]>
Cc: Mike Galbraith <[email protected]>
Cc: Namhyung Kim <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Peter Zijlstra <[email protected]>
Cc: Stephane Eranian <[email protected]>
Link: http://lkml.kernel.org/n/[email protected]
Signed-off-by: Arnaldo Carvalho de Melo <[email protected]>
---
tools/perf/ui/browsers/annotate.c | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index a90680b..44fb6a4 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -64,12 +64,12 @@ static void annotate_browser__write(struct ui_browser *self, void *entry, int ro
!self->navkeypressed)));
int width = self->width;
- if (dl->offset != -1) {
+ if (dl->offset != -1 && bdl->percent != 0.0) {
ui_browser__set_percent_color(self, bdl->percent, current_entry);
- slsmg_printf(" %7.2f ", bdl->percent);
+ slsmg_printf("%6.2f ", bdl->percent);
} else {
ui_browser__set_percent_color(self, 0, current_entry);
- slsmg_write_nstring(" ", 9);
+ slsmg_write_nstring(" ", 7);
}
SLsmg_write_char(' ');
@@ -82,9 +82,9 @@ static void annotate_browser__write(struct ui_browser *self, void *entry, int ro
ui_browser__set_color(self, HE_COLORSET_CODE);
if (!*dl->line)
- slsmg_write_nstring(" ", width - 9);
+ slsmg_write_nstring(" ", width - 7);
else if (dl->offset == -1)
- slsmg_write_nstring(dl->line, width - 9);
+ slsmg_write_nstring(dl->line, width - 7);
else {
char bf[256];
u64 addr = dl->offset;
@@ -137,7 +137,7 @@ static void annotate_browser__write(struct ui_browser *self, void *entry, int ro
scnprintf(bf, sizeof(bf), "%-6.6s %s", dl->name, dl->ops.raw);
}
- slsmg_write_nstring(bf, width - 11 - printed);
+ slsmg_write_nstring(bf, width - 9 - printed);
}
if (current_entry)
@@ -175,7 +175,7 @@ static void annotate_browser__draw_current_jump(struct ui_browser *browser)
if (!bcursor->jump_target)
start_width += ab->offset_width + 1;
- __ui_browser__line_arrow(browser, 9, from, to, start_width);
+ __ui_browser__line_arrow(browser, 7, from, to, start_width);
}
static unsigned int annotate_browser__refresh(struct ui_browser *browser)
--
1.7.9.2.358.g22243
On Wed, 2012-05-02 at 16:42 -0300, Arnaldo Carvalho de Melo wrote:
>
> avtab_search_node
> push %rbp
> mov %rsp,%rbp
> → callq mcount
> movzwl 0x6(%rsi),%edx
> and $0x7fff,%dx
> test %rdi,%rdi
> ┌─────↓ jne 20
> │ 17: xor %eax,%eax
> │ 19: leaveq
> │ ← retq
> │ nopl 0x0(%rax,%rax,1)
> └─→20: mov (%rdi),%rax
> test %rax,%rax
> ↑ je 17
> movzwl (%rsi),%ecx
> movzwl 0x2(%rsi),%r9d
> movzwl 0x4(%rsi),%r8d
> movzwl %cx,%esi
> movzwl %r9w,%r10d
> shl $0x9,%esi
> lea (%rsi,%r10,4),%esi
> lea (%r8,%rsi,1),%esi
> and 0x10(%rdi),%si
> movzwl %si,%esi
> mov (%rax,%rsi,8),%rax
> 1.63 test %rax,%rax
> ↑ je 19
> nopw 0x0(%rax,%rax,1)
> 4.88 60: cmp %cx,(%rax)
> ↓ jne 7e
> cmp %r9w,0x2(%rax)
> ↓ jne 7e
> cmp %r8w,0x4(%rax)
> ↓ jne 79
> test %dx,0x6(%rax)
> ↑ jne 19
> 79: cmp %r8w,0x4(%rax)
> 86.99 7e:↑ ja 17
> 3.25 mov 0x10(%rax),%rax
> 3.25 test %rax,%rax
> ↑ jne 60
> leaveq
> ← retq
>
> . Changes:
>
> - Don't show the big vertical line.
Not sure about that, loosing that separator makes it looks messy.
Em Wed, May 02, 2012 at 09:46:43PM +0200, Peter Zijlstra escreveu:
> On Wed, 2012-05-02 at 16:42 -0300, Arnaldo Carvalho de Melo wrote:
> >
> > avtab_search_node
> > push %rbp
> > mov %rsp,%rbp
> > → callq mcount
> > movzwl 0x6(%rsi),%edx
> > and $0x7fff,%dx
> > test %rdi,%rdi
> > ┌─────↓ jne 20
> > │ 17: xor %eax,%eax
> > │ 19: leaveq
> > │ ← retq
> > │ nopl 0x0(%rax,%rax,1)
> > └─→20: mov (%rdi),%rax
> > test %rax,%rax
> > ↑ je 17
> > movzwl (%rsi),%ecx
> > movzwl 0x2(%rsi),%r9d
> > movzwl 0x4(%rsi),%r8d
> > movzwl %cx,%esi
> > movzwl %r9w,%r10d
> > shl $0x9,%esi
> > lea (%rsi,%r10,4),%esi
> > lea (%r8,%rsi,1),%esi
> > and 0x10(%rdi),%si
> > movzwl %si,%esi
> > mov (%rax,%rsi,8),%rax
> > 1.63 test %rax,%rax
> > ↑ je 19
> > nopw 0x0(%rax,%rax,1)
> > 4.88 60: cmp %cx,(%rax)
> > ↓ jne 7e
> > cmp %r9w,0x2(%rax)
> > ↓ jne 7e
> > cmp %r8w,0x4(%rax)
> > ↓ jne 79
> > test %dx,0x6(%rax)
> > ↑ jne 19
> > 79: cmp %r8w,0x4(%rax)
> > 86.99 7e:↑ ja 17
> > 3.25 mov 0x10(%rax),%rax
> > 3.25 test %rax,%rax
> > ↑ jne 60
> > leaveq
> > ← retq
> >
> > . Changes:
> >
> > - Don't show the big vertical line.
>
> Not sure about that, loosing that separator makes it looks messy.
It was a request from Linus:
commit 3e8b5ddf17d4639d41bc57ecfb51633815b70e49
Author: Arnaldo Carvalho de Melo <[email protected]>
Date: Fri Apr 27 16:44:56 2012 -0300
perf annotate browser: Remove the vertical line after the percentages
It is confusing when used with jump -> target lines.
Requested-by: Linus Torvalds <[email protected]>
Make it configurable? Press 'S' and you get a separator? Linus?
- Arnaldo
Em Wed, May 02, 2012 at 09:46:43PM +0200, Peter Zijlstra escreveu:
> On Wed, 2012-05-02 at 16:42 -0300, Arnaldo Carvalho de Melo wrote:
> > . Changes:
> >
> > - Don't show the big vertical line.
>
> Not sure about that, loosing that separator makes it looks messy.
How about what we discussed on IRC:
avtab_search_node
│ push %rbp
│ mov %rsp,%rbp
│ → callq mcount
│ movzwl 0x6(%rsi),%edx
│ and $0x7fff,%dx
│ test %rdi,%rdi
│ ↓ jne 20
0.64 │17:┌─→xor %eax,%eax
│19:│ leaveq
0.51 │ │← retq
│ │ nopl 0x0(%rax,%rax,1)
│20:│ mov (%rdi),%rax
│ │ test %rax,%rax
│ └──je 17
│ movzwl (%rsi),%ecx
│ movzwl 0x2(%rsi),%r9d
│ movzwl 0x4(%rsi),%r8d
│ movzwl %cx,%esi
│ movzwl %r9w,%r10d
│ shl $0x9,%esi
│ lea (%rsi,%r10,4),%esi
│ lea (%r8,%rsi,1),%esi
│ and 0x10(%rdi),%si
│ movzwl %si,%esi
│ mov (%rax,%rsi,8),%rax
0.89 │ test %rax,%rax
│ ↑ je 19
│ nopw 0x0(%rax,%rax,1)
3.44 │60: cmp %cx,(%rax)
│ ↓ jne 7e
│ cmp %r9w,0x2(%rax)
│ ↓ jne 7e
│ cmp %r8w,0x4(%rax)
│ ↓ jne 79
│ test %dx,0x6(%rax)
│ ↑ jne 19
│79: cmp %r8w,0x4(%rax)
83.46 │7e: ↑ ja 17
3.82 │ mov 0x10(%rax),%rax
7.25 │ test %rax,%rax
│ ↑ jne 60
│ leaveq
│ ← retq
I.e. the fixed vertical line now has a diff color and the jump arrows are
_after_ the jump labels that then stands out as a separated columns.
In addition, as you suggested, the extra arrows on the ends of a jump->label
arrow gets swallowed by the jump->label arrow.
Also the fixed vertical line is drawn after we refresh the lines, i.e.,
it has a solid color, not influenced by each line percentages, as Linus
noticed previously.
Now a question: when I add multiple event column overheads, do you think we
should have N fixed vertical lines separating them?
I probably need to add an space before the instructions and the arrow
start/end, or not?
- Arnaldo
On Wed, May 2, 2012 at 2:18 PM, Arnaldo Carvalho de Melo
<[email protected]> wrote:
>
> Also the fixed vertical line is drawn after we refresh the lines, i.e.,
> it has a solid color, not influenced by each line percentages, as Linus
> noticed previously.
Looks good to me. That said, I do wonder whether the vertical line
really is needed (or maybe it could be at the last decimal, so it only
shows for the instructions without percentages).
But with the color fixed, I doubt it annoys me nearly as much as it
used to. And the other cleanups have already meant that the horizontal
space is not as scarce a resource as it used to be.
Linus
On Wed, 2012-05-02 at 18:18 -0300, Arnaldo Carvalho de Melo wrote:
> I.e. the fixed vertical line now has a diff color and the jump arrows are
> _after_ the jump labels that then stands out as a separated columns.
Note that if you have the source-code annotated thing enabled (which
seems to be the default) the arrows go straight through the source.
Also, in this case the asm is dark blue, which is nearly invisible on my
black background.
> In addition, as you suggested, the extra arrows on the ends of a jump->label
> arrow gets swallowed by the jump->label arrow.
Does look nice, thanks!
An alternative to all this arrow drawing could be to high-light the
jump-target, maybe not a full bar like the cursor line but something
like that.
Also, is there a key to jump to the jump target? I was instinctively
pressing '%' to do that, but that could be my vim brain-damage :-)
> Now a question: when I add multiple event column overheads, do you think we
> should have N fixed vertical lines separating them?
I'd start with that and go from there.
> I probably need to add an space before the instructions and the arrow
> start/end, or not?
Since its a dynamic arrow (in a nearly invisible colour) it doesn't
really matter. But what happens when a function is large enough that
function offset doesn't fit in 2 hex digits anymore? Aah I see, that
width is dynamic, OK.
Em Thu, May 03, 2012 at 10:01:40AM +0200, Peter Zijlstra escreveu:
> On Wed, 2012-05-02 at 18:18 -0300, Arnaldo Carvalho de Melo wrote:
> > I.e. the fixed vertical line now has a diff color and the jump arrows are
> > _after_ the jump labels that then stands out as a separated columns.
>
> Note that if you have the source-code annotated thing enabled (which
> seems to be the default) the arrows go straight through the source.
I thought I had that fixed, damn, will fix.
> Also, in this case the asm is dark blue, which is nearly invisible on my
> black background.
What colour do you suggest? Also what kind of term color scheme do you
use? Finding the right default combo is kinda hard.
> > In addition, as you suggested, the extra arrows on the ends of a jump->label
> > arrow gets swallowed by the jump->label arrow.
>
> Does look nice, thanks!
>
> An alternative to all this arrow drawing could be to high-light the
> jump-target, maybe not a full bar like the cursor line but something
> like that.
>
> Also, is there a key to jump to the jump target? I was instinctively
> pressing '%' to do that, but that could be my vim brain-damage :-)
Enter or ->, just like on calls and rets, what is missing is using <- to
go back the stack of navigated jumps, like with calls.
> > Now a question: when I add multiple event column overheads, do you think we
> > should have N fixed vertical lines separating them?
>
> I'd start with that and go from there.
Ok
> > I probably need to add an space before the instructions and the arrow
> > start/end, or not?
>
> Since its a dynamic arrow (in a nearly invisible colour) it doesn't
> really matter. But what happens when a function is large enough that
> function offset doesn't fit in 2 hex digits anymore? Aah I see, that
> width is dynamic, OK.
Yes, it is, it is calculated at function start and when switching
to/from "O"ffset view.
I need to add a help window with the ever growing list of hot keys.
Also will try to asap make the last features toggled persistent.
- Arnaldo
On Thu, 2012-05-03 at 10:05 -0300, Arnaldo Carvalho de Melo wrote:
> > Also, in this case the asm is dark blue, which is nearly invisible on my
> > black background.
>
> What colour do you suggest? Also what kind of term color scheme do you
> use? Finding the right default combo is kinda hard.
Yeah, I know.. I use white text on black background.
Typically for vim I use :se bg=dark
Anyway, I find it weird that the asm is coloured completely different
between the two modes. If anything I would keep the asm as in the
non-source variant and when you add the source, make that a different
colour.
Hi,
2012-05-03 (목), 15:12 +0200, Peter Zijlstra:
> On Thu, 2012-05-03 at 10:05 -0300, Arnaldo Carvalho de Melo wrote:
> > > Also, in this case the asm is dark blue, which is nearly invisible on my
> > > black background.
> >
> > What colour do you suggest? Also what kind of term color scheme do you
> > use? Finding the right default combo is kinda hard.
>
> Yeah, I know.. I use white text on black background.
>
> Typically for vim I use :se bg=dark
>
> Anyway, I find it weird that the asm is coloured completely different
> between the two modes. If anything I would keep the asm as in the
> non-source variant and when you add the source, make that a different
> colour.
>
It was changed on commit 58e817d997d1 ("perf annotate: Print asm code as
blue when source code is displayed") by me. I just tried to make the
output as same as the --stdio one, but I felt bad on printing the whole
asm lines as blue when no source is shown - I use similar color scheme
of yours - so I just ended up leaving it to the default white.
Thanks,
Namhyung
Em Thu, May 03, 2012 at 03:12:14PM +0200, Peter Zijlstra escreveu:
> On Thu, 2012-05-03 at 10:05 -0300, Arnaldo Carvalho de Melo wrote:
> > > Also, in this case the asm is dark blue, which is nearly invisible on my
> > > black background.
> >
> > What colour do you suggest? Also what kind of term color scheme do you
> > use? Finding the right default combo is kinda hard.
>
> Yeah, I know.. I use white text on black background.
>
> Typically for vim I use :se bg=dark
>
> Anyway, I find it weird that the asm is coloured completely different
> between the two modes. If anything I would keep the asm as in the
> non-source variant and when you add the source, make that a different
> colour.
Its a bug, it should be coloured the same in both modes. Will implement
as you suggest.
- Arnaldo
Em Thu, May 03, 2012 at 11:11:07PM +0900, Namhyung Kim escreveu:
> Hi,
>
> 2012-05-03 (목), 15:12 +0200, Peter Zijlstra:
> > On Thu, 2012-05-03 at 10:05 -0300, Arnaldo Carvalho de Melo wrote:
> > > > Also, in this case the asm is dark blue, which is nearly invisible on my
> > > > black background.
> > >
> > > What colour do you suggest? Also what kind of term color scheme do you
> > > use? Finding the right default combo is kinda hard.
> >
> > Yeah, I know.. I use white text on black background.
> >
> > Typically for vim I use :se bg=dark
> >
> > Anyway, I find it weird that the asm is coloured completely different
> > between the two modes. If anything I would keep the asm as in the
> > non-source variant and when you add the source, make that a different
> > colour.
> >
>
> It was changed on commit 58e817d997d1 ("perf annotate: Print asm code as
> blue when source code is displayed") by me. I just tried to make the
> output as same as the --stdio one, but I felt bad on printing the whole
> asm lines as blue when no source is shown - I use similar color scheme
> of yours - so I just ended up leaving it to the default white.
I'll revert that one, I agree with Peter that we should have the same
color for asm with/without source code.
If you feel like source code should be in another color, then try to
find one that works well with some common term color schemes and propose
a patch.
- Arnaldo