[GIT PULL 0/7] perf/core fixes and improvements

[GIT PULL 0/7] perf/core fixes and improvements

[Arnaldo Carvalho de Melo]

        Please consider pulling from:

git://github.com/acmel/linux.git perf/core

	The TUI now should be much closer to the old 'perf top' stdio
visual/experience, more on that vein in the next pull req.

- Arnaldo

Arnaldo Carvalho de Melo (7):
  perf hists browser: Add missing hotkeys to the help window
  perf tui: Catch signals to exit gracefully
  perf ui browser: Allow initial use without navigation UI elements
  perf hists: Don't format the percentage on hist_entry__snprintf
  perf tui: Remove unneeded call to newtCls on startup
  perf ui browser: Make the colors configurable and change the defaults
  perf top tui: Give color hints just on the percentage, like on --stdio

 tools/perf/Documentation/perfconfig.example |   20 ++++
 tools/perf/util/hist.c                      |   29 ++++--
 tools/perf/util/hist.h                      |    4 +-
 tools/perf/util/ui/browser.c                |  127 ++++++++++++++++++++++-----
 tools/perf/util/ui/browser.h                |    2 +
 tools/perf/util/ui/browsers/annotate.c      |    6 ++
 tools/perf/util/ui/browsers/hists.c         |   51 ++++++-----
 tools/perf/util/ui/setup.c                  |   25 +++++-
 8 files changed, 203 insertions(+), 61 deletions(-)
 create mode 100644 tools/perf/Documentation/perfconfig.example

[PATCH 1/7] perf hists browser: Add missing hotkeys to the help window

[Arnaldo Carvalho de Melo]

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

The navigation keys were missing (UP, DOWN arrows, etc).

Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
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-3pnln0bws5v0yoqwd3f020nx@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/ui/browsers/hists.c |   23 +++++++++++++----------
 1 files changed, 13 insertions(+), 10 deletions(-)

diff --git a/tools/perf/util/ui/browsers/hists.c b/tools/perf/util/ui/browsers/hists.c
index b144b10..873b852 100644
--- a/tools/perf/util/ui/browsers/hists.c
+++ b/tools/perf/util/ui/browsers/hists.c
@@ -886,17 +886,20 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
 		case NEWT_KEY_F1:
 		case 'h':
 		case '?':
-			ui__help_window("h/?/F1    Show this window\n"
-					"TAB/UNTAB Switch events\n"
-					"q/CTRL+C  Exit browser\n\n"
+			ui__help_window("h/?/F1        Show this window\n"
+					"UP/DOWN/PGUP\n"
+					"PGDN/SPACE    Navigate\n"
+					"q/ESC/CTRL+C  Exit browser\n\n"
+					"For multiple event sessions:\n\n"
+					"TAB/UNTAB Switch events\n\n"
 					"For symbolic views (--sort has sym):\n\n"
-					"->        Zoom into DSO/Threads & Annotate current symbol\n"
-					"<-        Zoom out\n"
-					"a         Annotate current symbol\n"
-					"C         Collapse all callchains\n"
-					"E         Expand all callchains\n"
-					"d         Zoom into current DSO\n"
-					"t         Zoom into current Thread\n");
+					"->            Zoom into DSO/Threads & Annotate current symbol\n"
+					"<-            Zoom out\n"
+					"a             Annotate current symbol\n"
+					"C             Collapse all callchains\n"
+					"E             Expand all callchains\n"
+					"d             Zoom into current DSO\n"
+					"t             Zoom into current Thread\n");
 			continue;
 		case NEWT_KEY_ENTER:
 		case NEWT_KEY_RIGHT:
-- 
1.6.2.5

[PATCH 2/7] perf tui: Catch signals to exit gracefully

[Arnaldo Carvalho de Melo]

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

Resetting the terminal to a sane state.

Reported-by: Ingo Molnar <mingo@elte.hu>
Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
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-myu44ujofadcy3y6an2mk383@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/ui/setup.c |   24 +++++++++++++++++++++++-
 1 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/ui/setup.c b/tools/perf/util/ui/setup.c
index ee46d67..8b8a57b 100644
--- a/tools/perf/util/ui/setup.c
+++ b/tools/perf/util/ui/setup.c
@@ -7,6 +7,7 @@
 #include "browser.h"
 #include "helpline.h"
 #include "ui.h"
+#include "libslang.h"
 
 pthread_mutex_t ui__lock = PTHREAD_MUTEX_INITIALIZER;
 
@@ -17,6 +18,21 @@ static void newt_suspend(void *d __used)
 	newtResume();
 }
 
+static void ui__exit(void)
+{
+	SLtt_set_cursor_visibility(1);
+	SLsmg_refresh();
+	SLsmg_reset_smg();
+	SLang_reset_tty();
+}
+
+static void ui__signal(int sig)
+{
+	ui__exit();
+	psignal(sig, "perf");
+	exit(0);
+}
+
 void setup_browser(bool fallback_to_pager)
 {
 	if (!isatty(1) || !use_browser || dump_trace) {
@@ -32,6 +48,12 @@ void setup_browser(bool fallback_to_pager)
 	newtSetSuspendCallback(newt_suspend, NULL);
 	ui_helpline__init();
 	ui_browser__init();
+
+	signal(SIGSEGV, ui__signal);
+	signal(SIGFPE, ui__signal);
+	signal(SIGINT, ui__signal);
+	signal(SIGQUIT, ui__signal);
+	signal(SIGTERM, ui__signal);
 }
 
 void exit_browser(bool wait_for_ok)
@@ -41,6 +63,6 @@ void exit_browser(bool wait_for_ok)
 			char title[] = "Fatal Error", ok[] = "Ok";
 			newtWinMessage(title, ok, ui_helpline__last_msg);
 		}
-		newtFinished();
+		ui__exit();
 	}
 }
-- 
1.6.2.5

[PATCH 3/7] perf ui browser: Allow initial use without navigation UI elements

[Arnaldo Carvalho de Melo]

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

The selection and scroll bar are really needed only when the user starts
navigating, before that it just provide distractions.

This also brings the initial screen to look more like the stdio UI,
which more people are used to.

The new code is flexible enough that menu like browsers can opt out and
start with those UI elements.

Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
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-jfgok30kkerpfw8wtcltgy6z@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/ui/browser.c           |   28 +++++++++++++++++++++++-----
 tools/perf/util/ui/browser.h           |    2 ++
 tools/perf/util/ui/browsers/annotate.c |    6 ++++++
 tools/perf/util/ui/browsers/hists.c    |   20 ++++++++------------
 4 files changed, 39 insertions(+), 17 deletions(-)

diff --git a/tools/perf/util/ui/browser.c b/tools/perf/util/ui/browser.c
index a54b926..dce16ee 100644
--- a/tools/perf/util/ui/browser.c
+++ b/tools/perf/util/ui/browser.c
@@ -14,9 +14,10 @@
 
 int newtGetKey(void);
 
-static int ui_browser__percent_color(double percent, bool current)
+static int ui_browser__percent_color(struct ui_browser *browser,
+				     double percent, bool current)
 {
-	if (current)
+	if (current && (!browser->use_navkeypressed || browser->navkeypressed))
 		return HE_COLORSET_SELECTED;
 	if (percent >= MIN_RED)
 		return HE_COLORSET_TOP;
@@ -33,7 +34,7 @@ void ui_browser__set_color(struct ui_browser *self __used, int color)
 void ui_browser__set_percent_color(struct ui_browser *self,
 				   double percent, bool current)
 {
-	 int color = ui_browser__percent_color(percent, current);
+	 int color = ui_browser__percent_color(self, percent, current);
 	 ui_browser__set_color(self, color);
 }
 
@@ -241,12 +242,18 @@ static void ui_browser__scrollbar_set(struct ui_browser *browser)
 static int __ui_browser__refresh(struct ui_browser *browser)
 {
 	int row;
+	int width = browser->width;
 
 	row = browser->refresh(browser);
 	ui_browser__set_color(browser, HE_COLORSET_NORMAL);
+
+	if (!browser->use_navkeypressed || browser->navkeypressed)
+		ui_browser__scrollbar_set(browser);
+	else
+		width += 1;
+
 	SLsmg_fill_region(browser->y + row, browser->x,
-			  browser->height - row, browser->width, ' ');
-	ui_browser__scrollbar_set(browser);
+			  browser->height - row, width, ' ');
 
 	return 0;
 }
@@ -326,6 +333,17 @@ int ui_browser__run(struct ui_browser *self, int delay_secs)
 			continue;
 		}
 
+		if (self->use_navkeypressed && !self->navkeypressed) {
+			if (key == NEWT_KEY_DOWN || key == NEWT_KEY_UP ||
+			    key == NEWT_KEY_PGDN || key == NEWT_KEY_PGUP ||
+			    key == NEWT_KEY_HOME || key == NEWT_KEY_END ||
+			    key == ' ') {
+				self->navkeypressed = true;
+				continue;
+			} else
+				return key;
+		}
+
 		switch (key) {
 		case NEWT_KEY_DOWN:
 			if (self->index == self->nr_entries - 1)
diff --git a/tools/perf/util/ui/browser.h b/tools/perf/util/ui/browser.h
index 351c006..a2c707d 100644
--- a/tools/perf/util/ui/browser.h
+++ b/tools/perf/util/ui/browser.h
@@ -23,6 +23,8 @@ struct ui_browser {
 	void	      (*seek)(struct ui_browser *self, off_t offset, int whence);
 	bool	      (*filter)(struct ui_browser *self, void *entry);
 	u32	      nr_entries;
+	bool	      navkeypressed;
+	bool	      use_navkeypressed;
 };
 
 void ui_browser__set_color(struct ui_browser *self, int color);
diff --git a/tools/perf/util/ui/browsers/annotate.c b/tools/perf/util/ui/browsers/annotate.c
index eb2712e..a2c351c 100644
--- a/tools/perf/util/ui/browsers/annotate.c
+++ b/tools/perf/util/ui/browsers/annotate.c
@@ -69,6 +69,11 @@ static void annotate_browser__write(struct ui_browser *self, void *entry, int ro
 
 	SLsmg_write_char(':');
 	slsmg_write_nstring(" ", 8);
+
+	/* The scroll bar isn't being used */
+	if (!self->navkeypressed)
+		width += 1;
+
 	if (!*ol->line)
 		slsmg_write_nstring(" ", width - 18);
 	else
@@ -386,6 +391,7 @@ int symbol__tui_annotate(struct symbol *sym, struct map *map, int evidx,
 			.write	 = annotate_browser__write,
 			.filter  = objdump_line__filter,
 			.priv	 = &ms,
+			.use_navkeypressed = true,
 		},
 	};
 	int ret;
diff --git a/tools/perf/util/ui/browsers/hists.c b/tools/perf/util/ui/browsers/hists.c
index 873b852..2d390b9 100644
--- a/tools/perf/util/ui/browsers/hists.c
+++ b/tools/perf/util/ui/browsers/hists.c
@@ -547,7 +547,7 @@ static int hist_browser__show_entry(struct hist_browser *self,
 	char s[256];
 	double percent;
 	int printed = 0;
-	int color, width = self->b.width;
+	int width = self->b.width;
 	char folded_sign = ' ';
 	bool current_entry = ui_browser__is_current_entry(&self->b, row);
 	off_t row_offset = entry->row_offset;
@@ -567,22 +567,17 @@ static int hist_browser__show_entry(struct hist_browser *self,
 				     0, false, self->hists->stats.total_period);
 		percent = (entry->period * 100.0) / self->hists->stats.total_period;
 
-		color = HE_COLORSET_SELECTED;
-		if (!current_entry) {
-			if (percent >= MIN_RED)
-				color = HE_COLORSET_TOP;
-			else if (percent >= MIN_GREEN)
-				color = HE_COLORSET_MEDIUM;
-			else
-				color = HE_COLORSET_NORMAL;
-		}
-
-		ui_browser__set_color(&self->b, color);
+		ui_browser__set_percent_color(&self->b, percent, current_entry);
 		ui_browser__gotorc(&self->b, row, 0);
 		if (symbol_conf.use_callchain) {
 			slsmg_printf("%c ", folded_sign);
 			width -= 2;
 		}
+
+		/* The scroll bar isn't being used */
+		if (!self->b.navkeypressed)
+			width += 1;
+
 		slsmg_write_nstring(s, width);
 		++row;
 		++printed;
@@ -787,6 +782,7 @@ static struct hist_browser *hist_browser__new(struct hists *hists)
 		self->hists = hists;
 		self->b.refresh = hist_browser__refresh;
 		self->b.seek = ui_browser__hists_seek;
+		self->b.use_navkeypressed = true,
 		self->has_symbols = sort_sym.list.next != NULL;
 	}
 
-- 
1.6.2.5

[PATCH 4/7] perf hists: Don't format the percentage on hist_entry__snprintf

[Arnaldo Carvalho de Melo]

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

We can't have color correctly set there because in libslang (and in a future
GUI) the colors must be set on a separate function call, so move that part to a
separate function and make the stdio fprintf function call it.

Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
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-jpgy42438ce9tgbqppm397lq@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hist.c              |   29 ++++++++++++++++++++---------
 tools/perf/util/hist.h              |    4 +---
 tools/perf/util/ui/browsers/hists.c |    7 ++++---
 3 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index 48da373..8d15e9f 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -707,12 +707,11 @@ void hists__output_recalc_col_len(struct hists *hists, int max_rows)
 	}
 }
 
-int hist_entry__snprintf(struct hist_entry *self, char *s, size_t size,
-			 struct hists *hists, struct hists *pair_hists,
-			 bool show_displacement, long displacement,
-			 bool color, u64 session_total)
+static int hist_entry__pcnt_snprintf(struct hist_entry *self, char *s,
+				     size_t size, struct hists *pair_hists,
+				     bool show_displacement, long displacement,
+				     bool color, u64 session_total)
 {
-	struct sort_entry *se;
 	u64 period, total, period_sys, period_us, period_guest_sys, period_guest_us;
 	u64 nr_events;
 	const char *sep = symbol_conf.field_sep;
@@ -818,12 +817,22 @@ int hist_entry__snprintf(struct hist_entry *self, char *s, size_t size,
 		}
 	}
 
+	return ret;
+}
+
+int hist_entry__snprintf(struct hist_entry *he, char *s, size_t size,
+			 struct hists *hists)
+{
+	const char *sep = symbol_conf.field_sep;
+	struct sort_entry *se;
+	int ret = 0;
+
 	list_for_each_entry(se, &hist_entry__sort_list, list) {
 		if (se->elide)
 			continue;
 
 		ret += snprintf(s + ret, size - ret, "%s", sep ?: "  ");
-		ret += se->se_snprintf(self, s + ret, size - ret,
+		ret += se->se_snprintf(he, s + ret, size - ret,
 				       hists__col_len(hists, se->se_width_idx));
 	}
 
@@ -835,13 +844,15 @@ int hist_entry__fprintf(struct hist_entry *he, size_t size, struct hists *hists,
 			long displacement, FILE *fp, u64 session_total)
 {
 	char bf[512];
+	int ret;
 
 	if (size == 0 || size > sizeof(bf))
 		size = sizeof(bf);
 
-	hist_entry__snprintf(he, bf, size, hists, pair_hists,
-			     show_displacement, displacement,
-			     true, session_total);
+	ret = hist_entry__pcnt_snprintf(he, bf, size, pair_hists,
+					show_displacement, displacement,
+					true, session_total);
+	hist_entry__snprintf(he, bf + ret, size - ret, hists);
 	return fprintf(fp, "%s\n", bf);
 }
 
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index ea96c1c..f338cb0 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -68,9 +68,7 @@ int hist_entry__fprintf(struct hist_entry *he, size_t size, struct hists *hists,
 			struct hists *pair_hists, bool show_displacement,
 			long displacement, FILE *fp, u64 session_total);
 int hist_entry__snprintf(struct hist_entry *self, char *bf, size_t size,
-			 struct hists *hists, struct hists *pair_hists,
-			 bool show_displacement, long displacement,
-			 bool color, u64 total);
+			 struct hists *hists);
 void hist_entry__free(struct hist_entry *);
 
 void hists__output_resort(struct hists *self);
diff --git a/tools/perf/util/ui/browsers/hists.c b/tools/perf/util/ui/browsers/hists.c
index 2d390b9..0eced19 100644
--- a/tools/perf/util/ui/browsers/hists.c
+++ b/tools/perf/util/ui/browsers/hists.c
@@ -547,7 +547,7 @@ static int hist_browser__show_entry(struct hist_browser *self,
 	char s[256];
 	double percent;
 	int printed = 0;
-	int width = self->b.width;
+	int width = self->b.width - 6; /* The percentage */
 	char folded_sign = ' ';
 	bool current_entry = ui_browser__is_current_entry(&self->b, row);
 	off_t row_offset = entry->row_offset;
@@ -563,8 +563,7 @@ static int hist_browser__show_entry(struct hist_browser *self,
 	}
 
 	if (row_offset == 0) {
-		hist_entry__snprintf(entry, s, sizeof(s), self->hists, NULL, false,
-				     0, false, self->hists->stats.total_period);
+		hist_entry__snprintf(entry, s, sizeof(s), self->hists);
 		percent = (entry->period * 100.0) / self->hists->stats.total_period;
 
 		ui_browser__set_percent_color(&self->b, percent, current_entry);
@@ -574,6 +573,8 @@ static int hist_browser__show_entry(struct hist_browser *self,
 			width -= 2;
 		}
 
+		slsmg_printf(" %5.2f%%", percent);
+
 		/* The scroll bar isn't being used */
 		if (!self->b.navkeypressed)
 			width += 1;
-- 
1.6.2.5

[PATCH 5/7] perf tui: Remove unneeded call to newtCls on startup

[Arnaldo Carvalho de Melo]

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

That was just filling the screen with blue, even if not a crash, not
something pleasant nor useful ;-)

Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
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-58znjqvan9b1mv5pojxboidg@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/ui/setup.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/tools/perf/util/ui/setup.c b/tools/perf/util/ui/setup.c
index 8b8a57b..5111f1a 100644
--- a/tools/perf/util/ui/setup.c
+++ b/tools/perf/util/ui/setup.c
@@ -44,7 +44,6 @@ void setup_browser(bool fallback_to_pager)
 
 	use_browser = 1;
 	newtInit();
-	newtCls();
 	newtSetSuspendCallback(newt_suspend, NULL);
 	ui_helpline__init();
 	ui_browser__init();
-- 
1.6.2.5

[PATCH 6/7] perf ui browser: Make the colors configurable and change the defaults

[Arnaldo Carvalho de Melo]

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

Just use as a starting point the "[colors]" section of
tools/perf/Documentation/perfconfig.example.

Changed the colors to be the ones in the old perf tool if used in a green on
black xterm.

The next patches should allow using the colors configured for the xterm.

Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
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-3vqmyerkaqltqolmnlehonew@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Documentation/perfconfig.example |   20 ++++++
 tools/perf/util/ui/browser.c                |   99 ++++++++++++++++++++++-----
 2 files changed, 101 insertions(+), 18 deletions(-)
 create mode 100644 tools/perf/Documentation/perfconfig.example

diff --git a/tools/perf/Documentation/perfconfig.example b/tools/perf/Documentation/perfconfig.example
new file mode 100644
index 0000000..d144866
--- /dev/null
+++ b/tools/perf/Documentation/perfconfig.example
@@ -0,0 +1,20 @@
+[colors]
+
+	# These were the old defaults
+	top = red, lightgray
+	medium = green, lightgray
+	normal = black, lightgray
+	selected = lightgray, magenta
+	code = blue, lightgray
+
+[tui]
+
+	# Defaults if linked with libslang
+	report = on
+	annotate = on
+	top = on
+
+[buildid]
+
+	# Default, disable using /dev/null
+	dir = /root/.debug
diff --git a/tools/perf/util/ui/browser.c b/tools/perf/util/ui/browser.c
index dce16ee..976b957 100644
--- a/tools/perf/util/ui/browser.c
+++ b/tools/perf/util/ui/browser.c
@@ -1,4 +1,5 @@
 #include "../util.h"
+#include "../cache.h"
 #include "../../perf.h"
 #include "libslang.h"
 #include <newt.h>
@@ -430,27 +431,89 @@ unsigned int ui_browser__list_head_refresh(struct ui_browser *self)
 	return row;
 }
 
-static struct ui_browser__colors {
-	const char *topColorFg, *topColorBg;
-	const char *mediumColorFg, *mediumColorBg;
-	const char *normalColorFg, *normalColorBg;
-	const char *selColorFg, *selColorBg;
-	const char *codeColorFg, *codeColorBg;
-} ui_browser__default_colors = {
-	"red",       "lightgray",
-	"green",     "lightgray",
-	"black",     "lightgray",
-	"lightgray", "magenta",
-	"blue",	     "lightgray",
+static struct ui_browser__colorset {
+	const char *name, *fg, *bg;
+	int colorset;
+} ui_browser__colorsets[] = {
+	{
+		.colorset = HE_COLORSET_TOP,
+		.name	  = "top",
+		.fg	  = "red",
+		.bg	  = "black",
+	},
+	{
+		.colorset = HE_COLORSET_MEDIUM,
+		.name	  = "medium",
+		.fg	  = "green",
+		.bg	  = "black",
+	},
+	{
+		.colorset = HE_COLORSET_NORMAL,
+		.name	  = "normal",
+		.fg	  = "brightgreen",
+		.bg	  = "black",
+	},
+	{
+		.colorset = HE_COLORSET_SELECTED,
+		.name	  = "selected",
+		.fg	  = "black",
+		.bg	  = "lightgray",
+	},
+	{
+		.colorset = HE_COLORSET_CODE,
+		.name	  = "code",
+		.fg	  = "blue",
+		.bg	  = "black",
+	},
+	{
+		.name = NULL,
+	}
 };
 
+
+static int ui_browser__color_config(const char *var, const char *value,
+				    void *data __used)
+{
+	char *fg = NULL, *bg;
+	int i;
+
+	/* same dir for all commands */
+	if (prefixcmp(var, "colors.") != 0)
+		return 0;
+
+	for (i = 0; ui_browser__colorsets[i].name != NULL; ++i) {
+		const char *name = var + 7;
+
+		if (strcmp(ui_browser__colorsets[i].name, name) != 0)
+			continue;
+
+		fg = strdup(value);
+		if (fg == NULL)
+			break;
+
+		bg = strchr(fg, ',');
+		if (bg == NULL)
+			break;
+
+		*bg = '\0';
+		while (isspace(*++bg));
+		ui_browser__colorsets[i].bg = bg;
+		ui_browser__colorsets[i].fg = fg;
+		return 0;
+	}
+
+	free(fg);
+	return -1;
+}
+
 void ui_browser__init(void)
 {
-	struct ui_browser__colors *c = &ui_browser__default_colors;
+	int i = 0;
 
-	sltt_set_color(HE_COLORSET_TOP, NULL, c->topColorFg, c->topColorBg);
-	sltt_set_color(HE_COLORSET_MEDIUM, NULL, c->mediumColorFg, c->mediumColorBg);
-	sltt_set_color(HE_COLORSET_NORMAL, NULL, c->normalColorFg, c->normalColorBg);
-	sltt_set_color(HE_COLORSET_SELECTED, NULL, c->selColorFg, c->selColorBg);
-	sltt_set_color(HE_COLORSET_CODE, NULL, c->codeColorFg, c->codeColorBg);
+	perf_config(ui_browser__color_config, NULL);
+
+	while (ui_browser__colorsets[i].name) {
+		struct ui_browser__colorset *c = &ui_browser__colorsets[i++];
+		sltt_set_color(c->colorset, c->name, c->fg, c->bg);
+	}
 }
-- 
1.6.2.5

Re: [PATCH 6/7] perf ui browser: Make the colors configurable and change the defaults

[David Ahern]

On 10/18/2011 03:44 PM, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> Just use as a starting point the "[colors]" section of
> tools/perf/Documentation/perfconfig.example.
> 
> Changed the colors to be the ones in the old perf tool if used in a green on
> black xterm.
> 
> The next patches should allow using the colors configured for the xterm.
> 
> Cc: David Ahern <dsahern@gmail.com>
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Mike Galbraith <efault@gmx.de>
> 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-3vqmyerkaqltqolmnlehonew@git.kernel.org
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  tools/perf/Documentation/perfconfig.example |   20 ++++++
>  tools/perf/util/ui/browser.c                |   99 ++++++++++++++++++++++-----
>  2 files changed, 101 insertions(+), 18 deletions(-)
>  create mode 100644 tools/perf/Documentation/perfconfig.example
> 
> diff --git a/tools/perf/Documentation/perfconfig.example b/tools/perf/Documentation/perfconfig.example
> new file mode 100644
> index 0000000..d144866
> --- /dev/null
> +++ b/tools/perf/Documentation/perfconfig.example
> @@ -0,0 +1,20 @@
> +[colors]
> +
> +	# These were the old defaults
> +	top = red, lightgray
> +	medium = green, lightgray
> +	normal = black, lightgray
> +	selected = lightgray, magenta
> +	code = blue, lightgray
> +
> +[tui]
> +
> +	# Defaults if linked with libslang
> +	report = on
> +	annotate = on
> +	top = on
> +
> +[buildid]
> +
> +	# Default, disable using /dev/null
> +	dir = /root/.debug
> diff --git a/tools/perf/util/ui/browser.c b/tools/perf/util/ui/browser.c
> index dce16ee..976b957 100644
> --- a/tools/perf/util/ui/browser.c
> +++ b/tools/perf/util/ui/browser.c
> @@ -1,4 +1,5 @@
>  #include "../util.h"
> +#include "../cache.h"
>  #include "../../perf.h"
>  #include "libslang.h"
>  #include <newt.h>
> @@ -430,27 +431,89 @@ unsigned int ui_browser__list_head_refresh(struct ui_browser *self)
>  	return row;
>  }
>  
> -static struct ui_browser__colors {
> -	const char *topColorFg, *topColorBg;
> -	const char *mediumColorFg, *mediumColorBg;
> -	const char *normalColorFg, *normalColorBg;
> -	const char *selColorFg, *selColorBg;
> -	const char *codeColorFg, *codeColorBg;
> -} ui_browser__default_colors = {
> -	"red",       "lightgray",
> -	"green",     "lightgray",
> -	"black",     "lightgray",
> -	"lightgray", "magenta",
> -	"blue",	     "lightgray",
> +static struct ui_browser__colorset {
> +	const char *name, *fg, *bg;
> +	int colorset;
> +} ui_browser__colorsets[] = {
> +	{
> +		.colorset = HE_COLORSET_TOP,
> +		.name	  = "top",
> +		.fg	  = "red",
> +		.bg	  = "black",
> +	},
> +	{
> +		.colorset = HE_COLORSET_MEDIUM,
> +		.name	  = "medium",
> +		.fg	  = "green",
> +		.bg	  = "black",
> +	},
> +	{
> +		.colorset = HE_COLORSET_NORMAL,
> +		.name	  = "normal",
> +		.fg	  = "brightgreen",
> +		.bg	  = "black",
> +	},

Using gnome3 with Fedora 15 these last 2 color sets don't look so good.
The 'black' background is not really black - more of a gray'ish look and
the green font does not show up well. Yes, I get that they are
configurable, but the defaults should be readable too.

David


> +	{
> +		.colorset = HE_COLORSET_SELECTED,
> +		.name	  = "selected",
> +		.fg	  = "black",
> +		.bg	  = "lightgray",
> +	},
> +	{
> +		.colorset = HE_COLORSET_CODE,
> +		.name	  = "code",
> +		.fg	  = "blue",
> +		.bg	  = "black",
> +	},
> +	{
> +		.name = NULL,
> +	}
>  };
>  
> +
> +static int ui_browser__color_config(const char *var, const char *value,
> +				    void *data __used)
> +{
> +	char *fg = NULL, *bg;
> +	int i;
> +
> +	/* same dir for all commands */
> +	if (prefixcmp(var, "colors.") != 0)
> +		return 0;
> +
> +	for (i = 0; ui_browser__colorsets[i].name != NULL; ++i) {
> +		const char *name = var + 7;
> +
> +		if (strcmp(ui_browser__colorsets[i].name, name) != 0)
> +			continue;
> +
> +		fg = strdup(value);
> +		if (fg == NULL)
> +			break;
> +
> +		bg = strchr(fg, ',');
> +		if (bg == NULL)
> +			break;
> +
> +		*bg = '\0';
> +		while (isspace(*++bg));
> +		ui_browser__colorsets[i].bg = bg;
> +		ui_browser__colorsets[i].fg = fg;
> +		return 0;
> +	}
> +
> +	free(fg);
> +	return -1;
> +}
> +
>  void ui_browser__init(void)
>  {
> -	struct ui_browser__colors *c = &ui_browser__default_colors;
> +	int i = 0;
>  
> -	sltt_set_color(HE_COLORSET_TOP, NULL, c->topColorFg, c->topColorBg);
> -	sltt_set_color(HE_COLORSET_MEDIUM, NULL, c->mediumColorFg, c->mediumColorBg);
> -	sltt_set_color(HE_COLORSET_NORMAL, NULL, c->normalColorFg, c->normalColorBg);
> -	sltt_set_color(HE_COLORSET_SELECTED, NULL, c->selColorFg, c->selColorBg);
> -	sltt_set_color(HE_COLORSET_CODE, NULL, c->codeColorFg, c->codeColorBg);
> +	perf_config(ui_browser__color_config, NULL);
> +
> +	while (ui_browser__colorsets[i].name) {
> +		struct ui_browser__colorset *c = &ui_browser__colorsets[i++];
> +		sltt_set_color(c->colorset, c->name, c->fg, c->bg);
> +	}
>  }

Re: [PATCH 6/7] perf ui browser: Make the colors configurable and change the defaults

[Arnaldo Carvalho de Melo]

Em Tue, Oct 18, 2011 at 05:13:49PM -0600, David Ahern escreveu:
> > +	{
> > +		.colorset = HE_COLORSET_MEDIUM,
> > +		.name	  = "medium",
> > +		.fg	  = "green",
> > +		.bg	  = "black",
> > +	},
> > +	{
> > +		.colorset = HE_COLORSET_NORMAL,
> > +		.name	  = "normal",
> > +		.fg	  = "brightgreen",
> > +		.bg	  = "black",
> > +	},
 
> Using gnome3 with Fedora 15 these last 2 color sets don't look so good.
> The 'black' background is not really black - more of a gray'ish look and
> the green font does not show up well. Yes, I get that they are
> configurable, but the defaults should be readable too.

I'm trying to get there :)

Yeah, I also noticed that libslang's "black" is kinda like a
bright black, really strange, I need to figure this out and also how to
get the colors currently set in the terminal to get this finally like
--stdio.

I.e. the defaults should be what are on the xterm, like --stdio.
 
- Arnaldo

Re: [PATCH 6/7] perf ui browser: Make the colors configurable and change the defaults

[Arnaldo Carvalho de Melo]

Em Tue, Oct 18, 2011 at 09:51:33PM -0200, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Oct 18, 2011 at 05:13:49PM -0600, David Ahern escreveu:
> > Using gnome3 with Fedora 15 these last 2 color sets don't look so good.
> > The 'black' background is not really black - more of a gray'ish look and
> > the green font does not show up well. Yes, I get that they are
> > configurable, but the defaults should be readable too.
 
> Yeah, I also noticed that libslang's "black" is kinda like a
> bright black, really strange, I need to figure this out and also how to
> get the colors currently set in the terminal to get this finally like
> --stdio.

Gack, its this problem:

https://bugzilla.redhat.com/show_bug.cgi?id=426948

I know need to continue reading to figure out how to overcome this, as
it seems to only affect libslang based apps, grrr.

Out to bed now tho :-)

- Arnaldo

[PATCH] perf ui browser: Honour the xterm colors

[Arnaldo Carvalho de Melo]

Em Tue, Oct 18, 2011 at 11:12:33PM -0200, Arnaldo Carvalho de Melo escreveu:
> Em Tue, Oct 18, 2011 at 09:51:33PM -0200, Arnaldo Carvalho de Melo escreveu:
> > Em Tue, Oct 18, 2011 at 05:13:49PM -0600, David Ahern escreveu:
> > > Using gnome3 with Fedora 15 these last 2 color sets don't look so good.
> > > The 'black' background is not really black - more of a gray'ish look and
>  
> > Yeah, I also noticed that libslang's "black" is kinda like a
> > bright black, really strange, I need to figure this out and also how to
> > get the colors currently set in the terminal to get this finally like
> > --stdio.
> 
> https://bugzilla.redhat.com/show_bug.cgi?id=426948
> 
> I know need to continue reading to figure out how to overcome this, as
> 
> -Out to bed now tho :-)-

Ok, the following patch, that I pushed to my perf/core branch, does the
trick...

_Out to bed now really :-)_

- Arnaldo

--------------------------

perf ui browser: Honour the xterm colors

So slang after all _has_ a 'default' color, call me color blind. Change
the default to it.

Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Ingo Molnar <mingo@elte.hu>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Paul Mackerras <paulus@samba.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Stephane Eranian <eranian@google.com>
Cc: Tom Zanussi <tzanussi@gmail.com>
Link: http://lkml.kernel.org/n/tip-1dfxivxv0jhwldpds3v4zla2@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/ui/browser.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/ui/browser.c b/tools/perf/util/ui/browser.c
index 976b957..06fc9eb 100644
--- a/tools/perf/util/ui/browser.c
+++ b/tools/perf/util/ui/browser.c
@@ -439,19 +439,19 @@ static struct ui_browser__colorset {
 		.colorset = HE_COLORSET_TOP,
 		.name	  = "top",
 		.fg	  = "red",
-		.bg	  = "black",
+		.bg	  = "default",
 	},
 	{
 		.colorset = HE_COLORSET_MEDIUM,
 		.name	  = "medium",
 		.fg	  = "green",
-		.bg	  = "black",
+		.bg	  = "default",
 	},
 	{
 		.colorset = HE_COLORSET_NORMAL,
 		.name	  = "normal",
-		.fg	  = "brightgreen",
-		.bg	  = "black",
+		.fg	  = "default",
+		.bg	  = "default",
 	},
 	{
 		.colorset = HE_COLORSET_SELECTED,
@@ -463,7 +463,7 @@ static struct ui_browser__colorset {
 		.colorset = HE_COLORSET_CODE,
 		.name	  = "code",
 		.fg	  = "blue",
-		.bg	  = "black",
+		.bg	  = "default",
 	},
 	{
 		.name = NULL,
-- 
1.6.2.5

Re: [PATCH] perf ui browser: Honour the xterm colors

[David Ahern]

On 10/18/2011 08:41 PM, Arnaldo Carvalho de Melo wrote:
> Em Tue, Oct 18, 2011 at 11:12:33PM -0200, Arnaldo Carvalho de Melo escreveu:
>> Em Tue, Oct 18, 2011 at 09:51:33PM -0200, Arnaldo Carvalho de Melo escreveu:
>>> Em Tue, Oct 18, 2011 at 05:13:49PM -0600, David Ahern escreveu:
>>>> Using gnome3 with Fedora 15 these last 2 color sets don't look so good.
>>>> The 'black' background is not really black - more of a gray'ish look and
>>  
>>> Yeah, I also noticed that libslang's "black" is kinda like a
>>> bright black, really strange, I need to figure this out and also how to
>>> get the colors currently set in the terminal to get this finally like
>>> --stdio.
>>
>> https://bugzilla.redhat.com/show_bug.cgi?id=426948
>>
>> I know need to continue reading to figure out how to overcome this, as
>>
>> -Out to bed now tho :-)-
> 
> Ok, the following patch, that I pushed to my perf/core branch, does the
> trick...

Indeed it does. Much better color contrasts -- for me anyways.

David


> 
> _Out to bed now really :-)_
> 
> - Arnaldo
> 
> --------------------------
> 
> perf ui browser: Honour the xterm colors
> 
> So slang after all _has_ a 'default' color, call me color blind. Change
> the default to it.
> 
> Cc: Frederic Weisbecker <fweisbec@gmail.com>
> Cc: Ingo Molnar <mingo@elte.hu>
> Cc: Mike Galbraith <efault@gmx.de>
> Cc: Paul Mackerras <paulus@samba.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Stephane Eranian <eranian@google.com>
> Cc: Tom Zanussi <tzanussi@gmail.com>
> Link: http://lkml.kernel.org/n/tip-1dfxivxv0jhwldpds3v4zla2@git.kernel.org
> Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
> ---
>  tools/perf/util/ui/browser.c |   10 +++++-----
>  1 files changed, 5 insertions(+), 5 deletions(-)
> 
> diff --git a/tools/perf/util/ui/browser.c b/tools/perf/util/ui/browser.c
> index 976b957..06fc9eb 100644
> --- a/tools/perf/util/ui/browser.c
> +++ b/tools/perf/util/ui/browser.c
> @@ -439,19 +439,19 @@ static struct ui_browser__colorset {
>  		.colorset = HE_COLORSET_TOP,
>  		.name	  = "top",
>  		.fg	  = "red",
> -		.bg	  = "black",
> +		.bg	  = "default",
>  	},
>  	{
>  		.colorset = HE_COLORSET_MEDIUM,
>  		.name	  = "medium",
>  		.fg	  = "green",
> -		.bg	  = "black",
> +		.bg	  = "default",
>  	},
>  	{
>  		.colorset = HE_COLORSET_NORMAL,
>  		.name	  = "normal",
> -		.fg	  = "brightgreen",
> -		.bg	  = "black",
> +		.fg	  = "default",
> +		.bg	  = "default",
>  	},
>  	{
>  		.colorset = HE_COLORSET_SELECTED,
> @@ -463,7 +463,7 @@ static struct ui_browser__colorset {
>  		.colorset = HE_COLORSET_CODE,
>  		.name	  = "code",
>  		.fg	  = "blue",
> -		.bg	  = "black",
> +		.bg	  = "default",
>  	},
>  	{
>  		.name = NULL,

Re: [PATCH] perf ui browser: Honour the xterm colors

[Arnaldo Carvalho de Melo]

Em Tue, Oct 18, 2011 at 09:03:02PM -0600, David Ahern escreveu:
> On 10/18/2011 08:41 PM, Arnaldo Carvalho de Melo wrote:
> > Em Tue, Oct 18, 2011 at 11:12:33PM -0200, Arnaldo Carvalho de Melo escreveu:
> >> Em Tue, Oct 18, 2011 at 09:51:33PM -0200, Arnaldo Carvalho de Melo escreveu:
> >>> Em Tue, Oct 18, 2011 at 05:13:49PM -0600, David Ahern escreveu:
> >>>> Using gnome3 with Fedora 15 these last 2 color sets don't look so good.
> >>>> The 'black' background is not really black - more of a gray'ish look and

> >>> Yeah, I also noticed that libslang's "black" is kinda like a
> >>> bright black, really strange, I need to figure this out and also how to
> >>> get the colors currently set in the terminal to get this finally like
> >>> --stdio.

> >> https://bugzilla.redhat.com/show_bug.cgi?id=426948

> >> I know need to continue reading to figure out how to overcome this, as

> > Ok, the following patch, that I pushed to my perf/core branch, does the
> > trick...

> Indeed it does. Much better color contrasts -- for me anyways.

Thanks for testing it, really appreciated!

- Arnaldo

[PATCH 7/7] perf top tui: Give color hints just on the percentage, like on --stdio

[Arnaldo Carvalho de Melo]

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

And like it was in the old top.

Another change so that the familiarity with the old visual is maintained.

Cc: David Ahern <dsahern@gmail.com>
Cc: Frederic Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
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-ypmyx9p0ah4byqaygrnb09x8@git.kernel.org
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/ui/browsers/hists.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/tools/perf/util/ui/browsers/hists.c b/tools/perf/util/ui/browsers/hists.c
index 0eced19..936e641 100644
--- a/tools/perf/util/ui/browsers/hists.c
+++ b/tools/perf/util/ui/browsers/hists.c
@@ -579,6 +579,9 @@ static int hist_browser__show_entry(struct hist_browser *self,
 		if (!self->b.navkeypressed)
 			width += 1;
 
+		if (!current_entry || !self->b.navkeypressed)
+			ui_browser__set_color(&self->b, HE_COLORSET_NORMAL);
+
 		slsmg_write_nstring(s, width);
 		++row;
 		++printed;
-- 
1.6.2.5

Re: [GIT PULL 0/7] perf/core fixes and improvements

[Ingo Molnar]

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

>         Please consider pulling from:
> 
> git://github.com/acmel/linux.git perf/core
> 
> 	The TUI now should be much closer to the old 'perf top' stdio
> visual/experience, more on that vein in the next pull req.
> 
> - Arnaldo
> 
> Arnaldo Carvalho de Melo (7):
>   perf hists browser: Add missing hotkeys to the help window
>   perf tui: Catch signals to exit gracefully
>   perf ui browser: Allow initial use without navigation UI elements
>   perf hists: Don't format the percentage on hist_entry__snprintf
>   perf tui: Remove unneeded call to newtCls on startup
>   perf ui browser: Make the colors configurable and change the defaults
>   perf top tui: Give color hints just on the percentage, like on --stdio
> 
>  tools/perf/Documentation/perfconfig.example |   20 ++++
>  tools/perf/util/hist.c                      |   29 ++++--
>  tools/perf/util/hist.h                      |    4 +-
>  tools/perf/util/ui/browser.c                |  127 ++++++++++++++++++++++-----
>  tools/perf/util/ui/browser.h                |    2 +
>  tools/perf/util/ui/browsers/annotate.c      |    6 ++
>  tools/perf/util/ui/browsers/hists.c         |   51 ++++++-----
>  tools/perf/util/ui/setup.c                  |   25 +++++-
>  8 files changed, 203 insertions(+), 61 deletions(-)
>  create mode 100644 tools/perf/Documentation/perfconfig.example

Pulled, thanks Arnaldo.

The way the TUI now follows terminal colors is really nice. There's a 
few regressions that sneaked in, and there's also still a few rough 
edges as well:

 - in callq following if i exit a secondary annotation screen via 'q' 
   or left-arrow, it does not jump back to the callq line as it did 
   earlier.

 - it's still hard to find all the callq's in the stream of assembly,
   i think it should be highlighted in a minimal fashion.

 - the mixed assembly and source code output for annotation now 
   became *harder* to follow, that the instruction opcodes are not 
   embedded. The reason is that there's now fewer visual patterns 
   that set apart the two types of lines.

   Not sure what to do about it, but it's not really usable this way, 
   to me at least. Color differentiation would certainly help, if 
   it's not too intrusive - could assembly be drawn grey? That would 
   put it into the 'visual background' on most terminal color 
   schemes.

   I tried a few mockup screens of splitting the screen 
   intelligently, and found one variant that works pretty well for 
   me. The main UI design complication is that the assembly opcodes 
   look so C source code-ish when put next to each other.

   So this is the original output:

         :        static u8 kallsyms2elf_type(char type)                                            ▒
         :        {                                                                                 ▒
         :                if (type == 'W')                                                          ▒
    0.00 :          43fd18:       mov    %rdx,%rdi                                                  ▒
         :                struct rb_node **p = &symbols->rb_node;                                   ▒
         :                struct rb_node *parent = NULL;                                            ▒
         :                const u64 ip = sym->start;                                                ▒
         :                struct symbol *s;                                                         ▒
         :                                                                                          ▒
         :                while (*p != NULL) {                                                      ▒
    0.00 :          43fd1b:       mov    (%rcx),%rdx                                                ▒
    0.00 :          43fd1e:       test   %rdx,%rdx                                                  ▒
    0.00 :          43fd21:       jne    43fd08 <map__process_kallsym_symbol+0xc8>                  ▒


   and here's the mockup:

        .                              | static u8 kallsyms2elf_type(char type)                     ▒
        .                              | {                                                          ▒
        .                              |         if (type == 'W')                                   ▒
   0.00 #  43fd18: mov    %rdx,%rdi                                                                 ▒
        .                              |         struct rb_node **p = &symbols->rb_node;            ▒
        .                              |         struct rb_node *parent = NULL;                     ▒
        .                              |         const u64 ip = sym->start;                         ▒
        .                              |         struct symbol *s;                                  ▒
        .                              |                                                            ▒
        .                              |         while (*p != NULL) {                               ▒
   0.00 #  43fd1b: mov    (%rcx),%rdx                                                               ▒
   0.00 #  43fd1e: test   %rdx,%rdx                                                                 ▒
   0.00 #  43fd21: jne    43fd08 <map__process_kallsym_symbol+0xc8>                                 ▒

There's several UI tricks:

 - typical short opcodes (80% of assembly) will fit on the left side 
   of the screen.

 - lines can still be arbitrarily long and overlap, so it's not a 
   true split screen - but the vertical helper line prefixing source 
   code lines keeps the eye focused on whichever side one intends to 
   concentrate on.

 - the first column separator uses two types of characters, '.' and 
   '#', to help the eye find the blocks of assembly.

 - we could, in addition, print assembly in grey.

 - i cut one character from the percentage column - the maximum value 
   is 100.00 so we don't need the original 7.2 format, 6.2 is enough.

We could eventually further compress the assembly display later on, 
but auto-labeling function-local labels (which are 99% of the jump 
targets). This would compress such jumps:

   0.00 #  43fd21: jne    43fd08 <map__process_kallsym_symbol+0xc8>

into:

   0.00 #  43fd21: jne    43fd08 <L3>

Thanks,

	Ingo

Re: [GIT PULL 0/7] perf/core fixes and improvements

[Arnaldo Carvalho de Melo]

Em Wed, Oct 19, 2011 at 09:14:57AM +0200, Ingo Molnar escreveu:
> * Arnaldo Carvalho de Melo <acme@infradead.org> wrote:
> > 	The TUI now should be much closer to the old 'perf top' stdio
> > visual/experience, more on that vein in the next pull req.

> Pulled, thanks Arnaldo.
> 
> The way the TUI now follows terminal colors is really nice. There's a 
> few regressions that sneaked in, and there's also still a few rough 
> edges as well:

>  - in callq following if i exit a secondary annotation screen via 'q' 
>    or left-arrow, it does not jump back to the callq line as it did 
>    earlier.

Oh, I noticed this at some point, but couldn't quickly reproduce as I
was concentrated on something else, but I think its case of using
'continue' versus 'break' when coming back from the nested annotation
browser, so that we don't change the current selection, will check that
now.
 
>  - it's still hard to find all the callq's in the stream of assembly,
>    i think it should be highlighted in a minimal fashion.

I'll use a graphic '->', like » or something, have to look at the
libslang docs.
 
>  - the mixed assembly and source code output for annotation now 
>    became *harder* to follow, that the instruction opcodes are not 
>    embedded. The reason is that there's now fewer visual patterns 
>    that set apart the two types of lines.
> 
>    Not sure what to do about it, but it's not really usable this way, 
>    to me at least. Color differentiation would certainly help, if 
>    it's not too intrusive - could assembly be drawn grey? That would 
>    put it into the 'visual background' on most terminal color 
>    schemes.

I'll make that configurable so that we can play a bit with it and then
decide on some sane default.
 
>    I tried a few mockup screens of splitting the screen 
>    intelligently, and found one variant that works pretty well for 
>    me. The main UI design complication is that the assembly opcodes 
>    look so C source code-ish when put next to each other.
> 
>    So this is the original output:
> 
>          :        static u8 kallsyms2elf_type(char type)                                            ▒
>          :        {                                                                                 ▒
>          :                if (type == 'W')                                                          ▒
>     0.00 :          43fd18:       mov    %rdx,%rdi                                                  ▒
>          :                struct rb_node **p = &symbols->rb_node;                                   ▒
>          :                struct rb_node *parent = NULL;                                            ▒
>          :                const u64 ip = sym->start;                                                ▒
>          :                struct symbol *s;                                                         ▒
>          :                                                                                          ▒
>          :                while (*p != NULL) {                                                      ▒
>     0.00 :          43fd1b:       mov    (%rcx),%rdx                                                ▒
>     0.00 :          43fd1e:       test   %rdx,%rdx                                                  ▒
>     0.00 :          43fd21:       jne    43fd08 <map__process_kallsym_symbol+0xc8>                  ▒
> 
> 
>    and here's the mockup:
> 
>         .                              | static u8 kallsyms2elf_type(char type)                     ▒
>         .                              | {                                                          ▒
>         .                              |         if (type == 'W')                                   ▒
>    0.00 #  43fd18: mov    %rdx,%rdi                                                                 ▒
>         .                              |         struct rb_node **p = &symbols->rb_node;            ▒
>         .                              |         struct rb_node *parent = NULL;                     ▒
>         .                              |         const u64 ip = sym->start;                         ▒
>         .                              |         struct symbol *s;                                  ▒
>         .                              |                                                            ▒
>         .                              |         while (*p != NULL) {                               ▒
>    0.00 #  43fd1b: mov    (%rcx),%rdx                                                               ▒
>    0.00 #  43fd1e: test   %rdx,%rdx                                                                 ▒
>    0.00 #  43fd21: jne    43fd08 <map__process_kallsym_symbol+0xc8>                                 ▒
> 
> There's several UI tricks:
> 
>  - typical short opcodes (80% of assembly) will fit on the left side 
>    of the screen.

right
 
>  - lines can still be arbitrarily long and overlap, so it's not a 
>    true split screen - but the vertical helper line prefixing source 
>    code lines keeps the eye focused on whichever side one intends to 
>    concentrate on.

Ok, I'll play with that.

	
>  - the first column separator uses two types of characters, '.' and 
>    '#', to help the eye find the blocks of assembly.

>  - we could, in addition, print assembly in grey.
> 
>  - i cut one character from the percentage column - the maximum value 
>    is 100.00 so we don't need the original 7.2 format, 6.2 is enough.

OK, after the first there was some 8 spaces that came from how it was
done long ago.
 
> We could eventually further compress the assembly display later on, 
> but auto-labeling function-local labels (which are 99% of the jump 
> targets). This would compress such jumps:
> 
>    0.00 #  43fd21: jne    43fd08 <map__process_kallsym_symbol+0xc8>
> 
> into:
> 
>    0.00 #  43fd21: jne    43fd08 <L3>

Yeah, this is something definetely in the plans, together with jumping
to labels, etc.
 
> Thanks,
> 
> 	Ingo