Merge branch 'perf/annotate' of git://git.kernel.org/pub/scm/linux/kernel/git/acme/linux into perf/core

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>
Signed-off-by: Ingo Molnar <mingo@kernel.org>

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);
@@ -600,8 +609,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)
 {
 	unsigned int row, end_row;
 
@@ -612,7 +622,7 @@ void __ui_browser__line_arrow_up(struct ui_browser *browser, unsigned int column
 		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;
@@ -639,6 +649,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)
+{
+	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(2);
+
+		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)
+{
+	if (start > end)
+		__ui_browser__line_arrow_up(browser, column, start, end);
+	else
+		__ui_browser__line_arrow_down(browser, column, start, end);
+}
+
 void ui_browser__init(void)
 {
 	int i = 0;

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);
 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,
@@ -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, ...);

tools/perf/ui/browsers/annotate.c

@@ -30,8 +30,11 @@ struct annotate_browser {
 	int		    nr_entries;
 	bool		    hide_src_code;
 	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];
 };
 
@@ -61,47 +64,46 @@ 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) {
+	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);
 	}
 
-	ui_browser__write_graph(self, SLSMG_VLINE_CHAR);
 	SLsmg_write_char(' ');
 
 	/* The scroll bar isn't being used */
 	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 - 10);
-	else if (dl->offset == -1)
-		slsmg_write_nstring(dl->line, width - 10);
-	else {
-		char bf[256];
+		slsmg_write_nstring(" ", width - 7);
+	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, " ");
 			}
 		}
 
@@ -117,12 +119,12 @@ 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);
 			}
-
-			dl->ins->ops->scnprintf(dl->ins, bf, sizeof(bf), &dl->ops,
-						!ab->use_offset);
 		} else {
 			if (strcmp(dl->name, "retq")) {
 				slsmg_write_nstring(" ", 2);
@@ -130,68 +132,56 @@ static void annotate_browser__write(struct ui_browser *self, void *entry, int ro
 				ui_browser__write_graph(self, SLSMG_LARROW_CHAR);
 				SLsmg_write_char(' ');
 			}
-
-			scnprintf(bf, sizeof(bf), "%-6.6s %s", dl->name, dl->ops.raw);
 		}
 
-		slsmg_write_nstring(bf, width - 12 - printed);
+		disasm_line__scnprintf(dl, bf, sizeof(bf), !ab->use_offset);
+		slsmg_write_nstring(bf, width - 10 - printed);
 	}
 
 	if (current_entry)
 		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;
-	unsigned int from, to, start_width = 2;
-
-	list_for_each_entry_from(pos, &notes->src->source, node) {
-		if (!pos->ins || !ins__is_jump(pos->ins) ||
-		    !disasm_line__has_offset(pos))
-			continue;
+	struct disasm_line *cursor = ab->selection, *target;
+	struct browser_disasm_line *btarget, *bcursor;
+	unsigned int from, to;
 
-		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)
-		start_width += ab->offset_width + 1;
-
-	__ui_browser__line_arrow_up(browser, 10, 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)
 {
+	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);
 
+	ui_browser__set_color(browser, HE_COLORSET_NORMAL);
+	__ui_browser__vline(browser, 7, 0, browser->height - 1);
 	return ret;
 }
 
@@ -624,6 +614,13 @@ 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;
 			continue;
 		case '/':
 			if (annotate_browser__search(self, delay_secs)) {
@@ -736,6 +733,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;
 
@@ -786,7 +784,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 */

tools/perf/util/annotate.c

@@ -18,6 +18,21 @@
 
 const char 	*disassembler_style;
 
+static int ins__raw_scnprintf(struct ins *ins, char *bf, size_t size,
+			      struct ins_operands *ops)
+{
+	return scnprintf(bf, size, "%-6.6s %s", ins->name, ops->raw);
+}
+
+int ins__scnprintf(struct ins *ins, char *bf, size_t size,
+		  struct ins_operands *ops)
+{
+	if (ins->ops->scnprintf)
+		return ins->ops->scnprintf(ins, bf, size, ops);
+
+	return ins__raw_scnprintf(ins, bf, size, ops);
+}
+
 static int call__parse(struct ins_operands *ops)
 {
 	char *endptr, *tok, *name;
@@ -50,11 +65,8 @@ indirect_call:
 }
 
 static int call__scnprintf(struct ins *ins, char *bf, size_t size,
-			   struct ins_operands *ops, bool addrs)
+			   struct ins_operands *ops)
 {
-	if (addrs)
-		return scnprintf(bf, size, "%-6.6s %s", ins->name, ops->raw);
-
 	if (ops->target.name)
 		return scnprintf(bf, size, "%-6.6s %s", ins->name, ops->target.name);
 
@@ -86,11 +98,8 @@ static int jump__parse(struct ins_operands *ops)
 }
 
 static int jump__scnprintf(struct ins *ins, char *bf, size_t size,
-			   struct ins_operands *ops, bool addrs)
+			   struct ins_operands *ops)
 {
-	if (addrs)
-		return scnprintf(bf, size, "%-6.6s %s", ins->name, ops->raw);
-
 	return scnprintf(bf, size, "%-6.6s %" PRIx64, ins->name, ops->target.offset);
 }
 
@@ -104,6 +113,16 @@ bool ins__is_jump(const struct ins *ins)
 	return ins->ops == &jump_ops;
 }
 
+static int nop__scnprintf(struct ins *ins __used, char *bf, size_t size,
+			  struct ins_operands *ops __used)
+{
+	return scnprintf(bf, size, "%-6.6s", "nop");
+}
+
+static struct ins_ops nop_ops = {
+	.scnprintf = nop__scnprintf,
+};
+
 /*
  * Must be sorted by name!
  */
@@ -145,6 +164,9 @@ static struct ins instructions[] = {
 	{ .name = "jrcxz", .ops  = &jump_ops, },
 	{ .name = "js",	   .ops  = &jump_ops, },
 	{ .name = "jz",	   .ops  = &jump_ops, },
+	{ .name = "nop",   .ops  = &nop_ops, },
+	{ .name = "nopl",  .ops  = &nop_ops, },
+	{ .name = "nopw",  .ops  = &nop_ops, },
 };
 
 static int ins__cmp(const void *name, const void *insp)
@@ -296,6 +318,14 @@ void disasm_line__free(struct disasm_line *dl)
 	free(dl);
 }
 
+int disasm_line__scnprintf(struct disasm_line *dl, char *bf, size_t size, bool raw)
+{
+	if (raw || !dl->ins)
+		return scnprintf(bf, size, "%-6.6s %s", dl->name, dl->ops.raw);
+
+	return ins__scnprintf(dl->ins, bf, size, &dl->ops);
+}
+
 static void disasm__add(struct list_head *head, struct disasm_line *line)
 {
 	list_add_tail(&line->node, head);

tools/perf/util/annotate.h

@@ -22,7 +22,7 @@ struct ins_operands {
 struct ins_ops {
 	int (*parse)(struct ins_operands *ops);
 	int (*scnprintf)(struct ins *ins, char *bf, size_t size,
-			 struct ins_operands *ops, bool addrs);
+			 struct ins_operands *ops);
 };
 
 struct ins {
@@ -32,6 +32,7 @@ struct ins {
 
 bool ins__is_jump(const struct ins *ins);
 bool ins__is_call(const struct ins *ins);
+int ins__scnprintf(struct ins *ins, char *bf, size_t size, struct ins_operands *ops);
 
 struct disasm_line {
 	struct list_head    node;
@@ -49,6 +50,7 @@ static inline bool disasm_line__has_offset(const struct disasm_line *dl)
 
 void disasm_line__free(struct disasm_line *dl);
 struct disasm_line *disasm__get_next_ip_line(struct list_head *head, struct disasm_line *pos);
+int disasm_line__scnprintf(struct disasm_line *dl, char *bf, size_t size, bool raw);
 size_t disasm__fprintf(struct list_head *head, FILE *fp);
 
 struct sym_hist {