perf/core fixes and improvements

From: Arnaldo Carvalho de Melo <[email protected]>

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 <[email protected]>

Cc Frederic Weisbecker <[email protected]>

Cc Mike Galbraith <[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] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 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

tools/perf/Documentation/perfconfig.example new file +20 -0

new file mode 100644
index 0000000..d144866
@@ -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

tools/perf/util/ui/browser.c +13 -12

index dce16ee..976b957 100644
@@ -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,

tools/perf/util/ui/browser.c +58 -6

+	{
+		.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);
+	}
 }

Quoted 9 lines

tools/perf/util/ui/browser.c +9 -0

+	{
+		.colorset = HE_COLORSET_MEDIUM,
+		.name	  = "medium",
+		.fg	  = "green",
+		.bg	  = "black",
+	},
+	{
+		.colorset = HE_COLORSET_NORMAL,
+		.name	  = "normal",

tools/perf/util/ui/browser.c unsigned int ui_browser__list_head_refresh(struct ui_browser *self) · line 455

Quoted 3 lines

tools/perf/util/ui/browser.c +3 -0

+		.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.

Em Tue, Oct 18, 2011 at 05:13:49PM -0600, David Ahern escreveu:

Quoted 4 lines

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.

Em Tue, Oct 18, 2011 at 09:51:33PM -0200, Arnaldo Carvalho de Melo escreveu:

Quoted 8 lines

Em Tue, Oct 18, 2011 at 05:13:49PM -0600, David Ahern escreveu:

Quoted 2 lines

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 :-)-

Em Tue, Oct 18, 2011 at 11:12:33PM -0200, Arnaldo Carvalho de Melo escreveu:

Quoted 10 lines

Em Tue, Oct 18, 2011 at 09:51:33PM -0200, Arnaldo Carvalho de Melo escreveu:

Quoted 8 lines

Em Tue, Oct 18, 2011 at 05:13:49PM -0600, David Ahern escreveu:

Quoted 2 lines

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.

Quoted 5 lines

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 <[email protected]>

Cc Ingo Molnar <[email protected]>

Cc Mike Galbraith <[email protected]>

Cc Paul Mackerras <[email protected]>

Cc Peter Zijlstra <[email protected]>

Cc Stephane Eranian <[email protected]>

Cc Tom Zanussi <[email protected]>

Link http://lkml.kernel.org/n/[email protected] open here

Signed-off-by Arnaldo Carvalho de Melo <[email protected]>

---
 tools/perf/util/ui/browser.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

tools/perf/util/ui/browser.c +5 -5

index 976b957..06fc9eb 100644
@@ -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,

Quoted 10 lines

Em Tue, Oct 18, 2011 at 11:12:33PM -0200, Arnaldo Carvalho de Melo escreveu:

Quoted 9 lines

Em Tue, Oct 18, 2011 at 09:51:33PM -0200, Arnaldo Carvalho de Melo escreveu:

Quoted 8 lines

Em Tue, Oct 18, 2011 at 05:13:49PM -0600, David Ahern escreveu:

Quoted 2 lines

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.

Quoted 6 lines

Quoted 3 lines

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.

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

Quoted 2 lines

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.

- 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