[PATCH 1/3] perf ui browser: Introduce routine to draw vertical line

[PATCH 1/3] perf ui browser: Introduce routine to draw vertical line

[Arnaldo Carvalho de Melo]

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

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-umb4jlu0ee8r2rc3x4jkahgk@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browser.c |    9 +++++++++
 tools/perf/ui/browser.h |    2 ++
 2 files changed, 11 insertions(+)

diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
index f4b2530..b075e09 100644
--- a/tools/perf/ui/browser.c
+++ b/tools/perf/ui/browser.c
@@ -593,6 +593,15 @@ unsigned int ui_browser__argv_refresh(struct ui_browser *browser)
 	return row;
 }
 
+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__write_graph(struct ui_browser *browser __used, int graph)
 {
 	SLsmg_set_char_set(1);
diff --git a/tools/perf/ui/browser.h b/tools/perf/ui/browser.h
index 059764b..511e24d 100644
--- a/tools/perf/ui/browser.h
+++ b/tools/perf/ui/browser.h
@@ -49,6 +49,8 @@ int ui_browser__refresh(struct ui_browser *self);
 int ui_browser__run(struct ui_browser *browser, int delay_secs);
 void ui_browser__update_nr_entries(struct ui_browser *browser, u32 nr_entries);
 void ui_browser__handle_resize(struct ui_browser *browser);
+void __ui_browser__vline(struct ui_browser *browser, unsigned int column,
+			 u16 start, u16 end);
 
 int ui_browser__warning(struct ui_browser *browser, int timeout,
 			const char *format, ...);
-- 
1.7.9.2.358.g22243

[PATCH 2/3] perf annotate browser: More clearly separate columns

[Arnaldo Carvalho de Melo]

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

The first column (columns in the near future) are for the per line event
overhead(s), that only appear when they are not zero.

To clearly separate it, add back a solid vertical line, with just one
colour, not influenced by the per line overheads.

Then have the addr/offset column, then optionally the dynamic
(static in the future) jump->target arrows, if 'j' enables it.

Then the instructions.

Requested-by: Peter Zijlstra <peterz@infradead.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-r415t4sps0oyr9y8kd9j7clz@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browser.c           |   14 +++++------
 tools/perf/ui/browser.h           |    2 +-
 tools/perf/ui/browsers/annotate.c |   48 +++++++++++++++++++++----------------
 3 files changed, 36 insertions(+), 28 deletions(-)

diff --git a/tools/perf/ui/browser.c b/tools/perf/ui/browser.c
index b075e09..cde4d0f 100644
--- a/tools/perf/ui/browser.c
+++ b/tools/perf/ui/browser.c
@@ -611,7 +611,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;
 
@@ -622,7 +622,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;
@@ -651,7 +651,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;
 
@@ -662,7 +662,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;
@@ -690,12 +690,12 @@ 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__init(void)
diff --git a/tools/perf/ui/browser.h b/tools/perf/ui/browser.h
index 511e24d..dd96d82 100644
--- a/tools/perf/ui/browser.h
+++ b/tools/perf/ui/browser.h
@@ -39,7 +39,7 @@ 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__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..74104a4 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -32,7 +32,9 @@ struct annotate_browser {
 	bool		    use_offset;
 	bool		    jump_arrows;
 	bool		    searching_backwards;
-	u8		    offset_width;
+	u8		    addr_width;
+	u8		    min_addr_width;
+	u8		    max_addr_width;
 	char		    search_bf[128];
 };
 
@@ -62,7 +64,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,25 +86,27 @@ 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->addr_width, " ");
+		slsmg_write_nstring(bf, printed);
+		slsmg_write_nstring(dl->line, width - printed - 6);
+	} 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 ":",
-						    ab->offset_width, addr);
+				printed = scnprintf(bf, sizeof(bf), "%*" PRIx64 ": ",
+						    ab->addr_width, addr);
 			} else {
-				printed = scnprintf(bf, sizeof(bf), "  %*s ",
-						    ab->offset_width, " ");
+				printed = scnprintf(bf, sizeof(bf), "%*s  ",
+						    ab->addr_width, " ");
 			}
 		}
 
@@ -137,7 +142,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 +154,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 +176,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->addr_width, from, to);
 }
 
 static unsigned int annotate_browser__refresh(struct ui_browser *browser)
@@ -186,6 +187,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 +621,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->addr_width = self->min_addr_width;
+			else
+				self->addr_width = self->max_addr_width;
 			continue;
 		case 'j':
 			self->jump_arrows = !self->jump_arrows;
@@ -784,7 +791,8 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
 
 	annotate_browser__mark_jump_targets(&browser, size);
 
-	browser.offset_width = hex_width(size);
+	browser.addr_width = browser.min_addr_width = hex_width(size);
+	browser.max_addr_width = hex_width(sym->end);
 	browser.b.nr_entries = browser.nr_entries;
 	browser.b.entries = &notes->src->source,
 	browser.b.width += 18; /* Percentage */
-- 
1.7.9.2.358.g22243

[PATCH 3/3] perf annotate browser: Don't change the asm line color when toggling source

[Arnaldo Carvalho de Melo]

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

Gets confusing. Remains to be chosen an appropriate different color for
source code.

This effectively reverts 58e817d997d1 ("perf annotate: Print asm code as
blue when source code is displayed")

Requested-by: Peter Zijlstra <peterz@infradead.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-qy9iq32nj3uqe5dbiuq9e3j9@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/ui/browsers/annotate.c |    3 ---
 1 file changed, 3 deletions(-)

diff --git a/tools/perf/ui/browsers/annotate.c b/tools/perf/ui/browsers/annotate.c
index 74104a4..b94da57 100644
--- a/tools/perf/ui/browsers/annotate.c
+++ b/tools/perf/ui/browsers/annotate.c
@@ -81,9 +81,6 @@ static void annotate_browser__write(struct ui_browser *self, void *entry, int ro
 	if (!self->navkeypressed)
 		width += 1;
 
-	if (dl->offset != -1 && change_color)
-		ui_browser__set_color(self, HE_COLORSET_CODE);
-
 	if (!*dl->line)
 		slsmg_write_nstring(" ", width - 7);
 	else if (dl->offset == -1) {
-- 
1.7.9.2.358.g22243

Re: [GIT PULL 0/3] perf/annotate improvements

[Ingo Molnar]

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

> Hi Ingo,
> 
> 	Please consider pulling,
> 
> 	Just a bit on top of yesterday's pile.
> 
> - Arnaldo
> 
> The following changes since commit 0822cc80d9aee026b1ebe43c02dc01e0a0227864:
> 
>   perf annotate browser: Don't display 0.00 percentages (2012-04-27 17:13:53 -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 64aa17ca5a4e428fcb6d0806823a99a18c548506:
> 
>   perf annotate browser: Don't change the asm line color when toggling source (2012-05-03 13:23:00 -0300)
> 
> ----------------------------------------------------------------
> Perf annotate browser improvements.
> 
> . Get back the line separating the overheads from the disassembly, requested by
>   Peter Zijlstra, Linus agreed now that it is a solid line and more column real
>   state was harvested. Also it has the jump->arrow lines separated from it by
>   the address/jump target column.
> 
> . Don't change asm line color when toggling source code view. Requested by
>   Peter Zijlstra.
> 
> Current snapshot:
> 
> avtab_search_node
>        │      push   %rbp
>        │      mov    %rsp,%rbp
>        │    → callq  mcount
>        │      movzwl 0x6(%rsi),%edx
>        │      and    $0x7fff,%dx
>        │      test   %rdi,%rdi
>        │    ↓ jne    20
>   0.42 │17:┌─→xor    %eax,%eax
>        │19:│  leaveq
>   0.42 │   │← retq
>        │   │  nopl   0x0(%rax,%rax,1)
>        │20:│  mov    (%rdi),%rax
>   0.08 │   │  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.01 │      test   %rax,%rax
>        │    ↑ je     19
>        │      nopw   0x0(%rax,%rax,1)
>   3.19 │60:   cmp    %cx,(%rax)
>        │    ↓ jne    7e
>   0.08 │      cmp    %r9w,0x2(%rax)
>        │    ↓ jne    7e
>        │      cmp    %r8w,0x4(%rax)
>        │    ↓ jne    79
>        │      test   %dx,0x6(%rax)
>        │    ↑ jne    19
>        │79:   cmp    %r8w,0x4(%rax)
>  83.45 │7e: ↑ ja     17
>   3.36 │      mov    0x10(%rax),%rax
>   7.98 │      test   %rax,%rax
>        │    ↑ jne    60
>        │      leaveq
>        │    ← retq
> 
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> ----------------------------------------------------------------
> Arnaldo Carvalho de Melo (3):
>       perf ui browser: Introduce routine to draw vertical line
>       perf annotate browser: More clearly separate columns
>       perf annotate browser: Don't change the asm line color when toggling source
> 
>  tools/perf/ui/browser.c           |   23 ++++++++++++-----
>  tools/perf/ui/browser.h           |    4 ++-
>  tools/perf/ui/browsers/annotate.c |   51 ++++++++++++++++++++-----------------
>  3 files changed, 47 insertions(+), 31 deletions(-)

Hm, this regresses for some symbols, the annotation window only 
shows this:

__strlen_sse2_pminub                                                                               
       │
       │

It used to show the full thing:

__strlen_sse2_pminub                                                                               
    0.00 :          392a35dbde:   jne    392a35dc30 
<__strlen_sse2_pminub+0x2a0>                  ▒
    0.00 :          392a35dbe0:   pcmpeqb 0x40(%rax),%xmm3                                        
▒
    0.00 :          392a35dbe5:   pmovmskb %xmm3,%edx                                             
▒
    0.00 :          392a35dbe9:   sub    %rdi,%rax                                                
▒
    0.00 :          392a35dbec:   bsf    %rdx,%rdx                                                
▒
    0.00 :          392a35dbf0:   add    %rdx,%rax                                                
▒
    0.00 :          392a35dbf3:   add    $0x40,%rax                                               
▒
    0.00 :          392a35dbf7:   retq                                           

Other symbols are fine.

	Ingo

Re: [GIT PULL 0/3] perf/annotate improvements

[Arnaldo Carvalho de Melo]

Em Fri, May 04, 2012 at 09:00:48AM +0200, Ingo Molnar escreveu:
> * Arnaldo Carvalho de Melo <acme@infradead.org> wrote:
> 
> Hm, this regresses for some symbols, the annotation window only 
> shows this:

I'll check this, and as well provide a "screenshot" feature to write a
__strlen_sse2_pminub.annotation file stripping the spaces all the way to
the scrollbar, to ease posting problems :-)

- Arnaldo
 
> __strlen_sse2_pminub                                                                               
>        │
>        │
> 
> It used to show the full thing:
> 
> __strlen_sse2_pminub                                                                               
>     0.00 :          392a35dbde:   jne    392a35dc30 
> <__strlen_sse2_pminub+0x2a0>                  ▒
>     0.00 :          392a35dbe0:   pcmpeqb 0x40(%rax),%xmm3                                        
> ▒
>     0.00 :          392a35dbe5:   pmovmskb %xmm3,%edx                                             
> ▒
>     0.00 :          392a35dbe9:   sub    %rdi,%rax                                                
> ▒
>     0.00 :          392a35dbec:   bsf    %rdx,%rdx                                                
> ▒
>     0.00 :          392a35dbf0:   add    %rdx,%rax                                                
> ▒
>     0.00 :          392a35dbf3:   add    $0x40,%rax                                               
> ▒
>     0.00 :          392a35dbf7:   retq                                           
> 
> Other symbols are fine.
> 
> 	Ingo