Re: [GIT PULL 0/5] perf/annotate fixes and improvements
From: Arnaldo Carvalho de Melo
Date: Wed May 02 2012 - 17:18:02 EST
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
diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
index f4b2530..562499c 100644
--- a/tools/perf/ui/browser.c
+++ b/tools/perf/ui/browser.c
@@ -602,7 +602,7 @@ void ui_browser__write_graph(struct ui_browser *browser __used, int graph)
static void __ui_browser__line_arrow_up(struct ui_browser *browser,
unsigned int column,
- u64 start, u64 end, int start_width)
+ u64 start, u64 end)
{
unsigned int row, end_row;
@@ -613,7 +613,7 @@ static void __ui_browser__line_arrow_up(struct ui_browser *browser,
ui_browser__gotorc(browser, row, column);
SLsmg_write_char(SLSMG_LLCORN_CHAR);
ui_browser__gotorc(browser, row, column + 1);
- SLsmg_draw_hline(start_width);
+ SLsmg_draw_hline(2);
if (row-- == 0)
goto out;
@@ -642,7 +642,7 @@ out:
static void __ui_browser__line_arrow_down(struct ui_browser *browser,
unsigned int column,
- u64 start, u64 end, int start_width)
+ u64 start, u64 end)
{
unsigned int row, end_row;
@@ -653,7 +653,7 @@ static void __ui_browser__line_arrow_down(struct ui_browser *browser,
ui_browser__gotorc(browser, row, column);
SLsmg_write_char(SLSMG_ULCORN_CHAR);
ui_browser__gotorc(browser, row, column + 1);
- SLsmg_draw_hline(start_width);
+ SLsmg_draw_hline(2);
if (row++ == 0)
goto out;
@@ -681,12 +681,21 @@ out:
}
void __ui_browser__line_arrow(struct ui_browser *browser, unsigned int column,
- u64 start, u64 end, int start_width)
+ u64 start, u64 end)
{
if (start > end)
- __ui_browser__line_arrow_up(browser, column, start, end, start_width);
+ __ui_browser__line_arrow_up(browser, column, start, end);
else
- __ui_browser__line_arrow_down(browser, column, start, end, start_width);
+ __ui_browser__line_arrow_down(browser, column, start, end);
+}
+
+void __ui_browser__vline(struct ui_browser *browser, unsigned int column,
+ u16 start, u16 end)
+{
+ SLsmg_set_char_set(1);
+ ui_browser__gotorc(browser, start, column);
+ SLsmg_draw_vline(end - start + 1);
+ SLsmg_set_char_set(0);
}
void ui_browser__init(void)
diff --git a/tools/perf/ui/browser.h b/tools/perf/ui/browser.h
index 059764b..133432d 100644
--- a/tools/perf/ui/browser.h
+++ b/tools/perf/ui/browser.h
@@ -39,7 +39,9 @@ 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(struct ui_browser *browser, unsigned int column,
- u64 start, u64 end, int start_width);
+ u64 start, u64 end);
+void __ui_browser__vline(struct ui_browser *browser, unsigned int column,
+ u16 start, u16 end);
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,
diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 44fb6a4..931db9f 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -62,7 +62,8 @@ static void annotate_browser__write(struct ui_browser *self, void *entry, int ro
bool change_color = (!ab->hide_src_code &&
(!current_entry || (self->use_navkeypressed &&
!self->navkeypressed)));
- int width = self->width;
+ int width = self->width, printed;
+ char bf[256];
if (dl->offset != -1 && bdl->percent != 0.0) {
ui_browser__set_percent_color(self, bdl->percent, current_entry);
@@ -83,24 +84,26 @@ static void annotate_browser__write(struct ui_browser *self, void *entry, int ro
if (!*dl->line)
slsmg_write_nstring(" ", width - 7);
- else if (dl->offset == -1)
- slsmg_write_nstring(dl->line, width - 7);
- else {
- char bf[256];
+ else if (dl->offset == -1) {
+ printed = scnprintf(bf, sizeof(bf), "%*s ",
+ ab->offset_width, " ");
+ slsmg_write_nstring(bf, printed);
+ slsmg_write_nstring(dl->line, width - printed - 5);
+ } else {
u64 addr = dl->offset;
- int printed, color = -1;
+ int color = -1;
if (!ab->use_offset)
addr += ab->start;
if (!ab->use_offset) {
- printed = scnprintf(bf, sizeof(bf), " %" PRIx64 ":", addr);
+ printed = scnprintf(bf, sizeof(bf), "%" PRIx64 ": ", addr);
} else {
if (bdl->jump_target) {
- printed = scnprintf(bf, sizeof(bf), " %*" PRIx64 ":",
+ printed = scnprintf(bf, sizeof(bf), "%*" PRIx64 ": ",
ab->offset_width, addr);
} else {
- printed = scnprintf(bf, sizeof(bf), " %*s ",
+ printed = scnprintf(bf, sizeof(bf), "%*s ",
ab->offset_width, " ");
}
}
@@ -137,7 +140,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 - 9 - printed);
+ slsmg_write_nstring(bf, width - 10 - printed);
}
if (current_entry)
@@ -149,7 +152,7 @@ static void annotate_browser__draw_current_jump(struct ui_browser *browser)
struct annotate_browser *ab = container_of(browser, struct annotate_browser, b);
struct disasm_line *cursor = ab->selection, *target;
struct browser_disasm_line *btarget, *bcursor;
- unsigned int from, to, start_width = 2;
+ unsigned int from, to;
if (!cursor->ins || !ins__is_jump(cursor->ins) ||
!disasm_line__has_offset(cursor))
@@ -171,11 +174,7 @@ static void annotate_browser__draw_current_jump(struct ui_browser *browser)
}
ui_browser__set_color(browser, HE_COLORSET_CODE);
-
- if (!bcursor->jump_target)
- start_width += ab->offset_width + 1;
-
- __ui_browser__line_arrow(browser, 7, from, to, start_width);
+ __ui_browser__line_arrow(browser, 9 + ab->offset_width, from, to);
}
static unsigned int annotate_browser__refresh(struct ui_browser *browser)
@@ -186,6 +185,8 @@ static unsigned int annotate_browser__refresh(struct ui_browser *browser)
if (ab->jump_arrows)
annotate_browser__draw_current_jump(browser);
+ ui_browser__set_color(browser, HE_COLORSET_NORMAL);
+ __ui_browser__vline(browser, 7, 0, browser->height - 1);
return ret;
}
@@ -618,6 +619,10 @@ static int annotate_browser__run(struct annotate_browser *self, int evidx,
case 'O':
case 'o':
self->use_offset = !self->use_offset;
+ if (self->use_offset)
+ self->offset_width = hex_width(symbol__size(sym));
+ else
+ self->offset_width = hex_width(sym->end);
continue;
case 'j':
self->jump_arrows = !self->jump_arrows;