[PATCH v2 1/5] perf symbols: Bump plt synthesizing warning debug level

[PATCH v2 1/5] perf symbols: Bump plt synthesizing warning debug level

[Arnaldo Carvalho de Melo]

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

Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/symbol.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/tools/perf/util/symbol.c b/tools/perf/util/symbol.c
index 323c0ae..75cd468 100644
--- a/tools/perf/util/symbol.c
+++ b/tools/perf/util/symbol.c
@@ -862,8 +862,8 @@ out_close:
 	if (err == 0)
 		return nr;
 out:
-	pr_warning("%s: problems reading %s PLT info.\n",
-		   __func__, self->long_name);
+	pr_debug("%s: problems reading %s PLT info.\n",
+		 __func__, self->long_name);
 	return 0;
 }
 
-- 
1.6.2.5

[PATCH v2 2/5] perf top: Export get_window_dimensions

[Arnaldo Carvalho de Melo]

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

Will be used by the newt code too.

Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/builtin-top.c |    2 +-
 tools/perf/perf.h        |    4 ++++
 2 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/tools/perf/builtin-top.c b/tools/perf/builtin-top.c
index ec48223..57e232f 100644
--- a/tools/perf/builtin-top.c
+++ b/tools/perf/builtin-top.c
@@ -133,7 +133,7 @@ static inline struct symbol *sym_entry__symbol(struct sym_entry *self)
        return ((void *)self) + symbol_conf.priv_size;
 }
 
-static void get_term_dimensions(struct winsize *ws)
+void get_term_dimensions(struct winsize *ws)
 {
 	char *s = getenv("LINES");
 
diff --git a/tools/perf/perf.h b/tools/perf/perf.h
index 6fb379b..aa78615 100644
--- a/tools/perf/perf.h
+++ b/tools/perf/perf.h
@@ -1,6 +1,10 @@
 #ifndef _PERF_PERF_H
 #define _PERF_PERF_H
 
+struct winsize;
+
+void get_term_dimensions(struct winsize *ws);
+
 #if defined(__i386__)
 #include "../../arch/x86/include/asm/unistd.h"
 #define rmb()		asm volatile("lock; addl $0,0(%%esp)" ::: "memory")
-- 
1.6.2.5

[PATCH v2 3/5] perf tools: Use eprintf for pr_{err,warning,info} too

[Arnaldo Carvalho de Melo]

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

Just like we do for pr_debug, so that we can have a single point where to
redirect to the currently used output system, be it stdio or newt.

Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/debug.h                |    2 --
 tools/perf/util/include/linux/kernel.h |    9 ++++++---
 2 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/tools/perf/util/debug.h b/tools/perf/util/debug.h
index c6c24c5..58720a1 100644
--- a/tools/perf/util/debug.h
+++ b/tools/perf/util/debug.h
@@ -7,8 +7,6 @@
 extern int verbose;
 extern int dump_trace;
 
-int eprintf(int level,
-	    const char *fmt, ...) __attribute__((format(printf, 2, 3)));
 int dump_printf(const char *fmt, ...) __attribute__((format(printf, 1, 2)));
 void trace_event(event_t *event);
 
diff --git a/tools/perf/util/include/linux/kernel.h b/tools/perf/util/include/linux/kernel.h
index f261165..388ab1b 100644
--- a/tools/perf/util/include/linux/kernel.h
+++ b/tools/perf/util/include/linux/kernel.h
@@ -85,16 +85,19 @@ simple_strtoul(const char *nptr, char **endptr, int base)
 	return strtoul(nptr, endptr, base);
 }
 
+int eprintf(int level,
+	    const char *fmt, ...) __attribute__((format(printf, 2, 3)));
+
 #ifndef pr_fmt
 #define pr_fmt(fmt) fmt
 #endif
 
 #define pr_err(fmt, ...) \
-	do { fprintf(stderr, pr_fmt(fmt), ##__VA_ARGS__); } while (0)
+	eprintf(0, pr_fmt(fmt), ##__VA_ARGS__)
 #define pr_warning(fmt, ...) \
-	do { fprintf(stderr, pr_fmt(fmt), ##__VA_ARGS__); } while (0)
+	eprintf(0, pr_fmt(fmt), ##__VA_ARGS__)
 #define pr_info(fmt, ...) \
-	do { fprintf(stderr, pr_fmt(fmt), ##__VA_ARGS__); } while (0)
+	eprintf(0, pr_fmt(fmt), ##__VA_ARGS__)
 #define pr_debug(fmt, ...) \
 	eprintf(1, pr_fmt(fmt), ##__VA_ARGS__)
 #define pr_debugN(n, fmt, ...) \
-- 
1.6.2.5

[PATCH v2 4/5] perf tools: Add missing bytes printed in hist_entry__fprintf

[Arnaldo Carvalho de Melo]

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

We need those to properly size the browser widht in the newt TUI.

Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/util/hist.c |   24 ++++++++++++------------
 1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c
index bdcfd61..d43be34 100644
--- a/tools/perf/util/hist.c
+++ b/tools/perf/util/hist.c
@@ -455,11 +455,11 @@ static size_t hist_entry_callchain__fprintf(FILE *fp, struct hist_entry *self,
 	return ret;
 }
 
-static size_t hist_entry__fprintf(struct hist_entry *self,
-				  struct perf_session *pair_session,
-				  bool show_displacement,
-				  long displacement, FILE *fp,
-				  u64 session_total)
+size_t hist_entry__fprintf(struct hist_entry *self,
+			   struct perf_session *pair_session,
+			   bool show_displacement,
+			   long displacement, FILE *fp,
+			   u64 session_total)
 {
 	struct sort_entry *se;
 	u64 count, total;
@@ -485,9 +485,9 @@ static size_t hist_entry__fprintf(struct hist_entry *self,
 
 	if (symbol_conf.show_nr_samples) {
 		if (sep)
-			fprintf(fp, "%c%lld", *sep, count);
+			ret += fprintf(fp, "%c%lld", *sep, count);
 		else
-			fprintf(fp, "%11lld", count);
+			ret += fprintf(fp, "%11lld", count);
 	}
 
 	if (pair_session) {
@@ -518,9 +518,9 @@ static size_t hist_entry__fprintf(struct hist_entry *self,
 				snprintf(bf, sizeof(bf), " ");
 
 			if (sep)
-				fprintf(fp, "%c%s", *sep, bf);
+				ret += fprintf(fp, "%c%s", *sep, bf);
 			else
-				fprintf(fp, "%6.6s", bf);
+				ret += fprintf(fp, "%6.6s", bf);
 		}
 	}
 
@@ -528,7 +528,7 @@ static size_t hist_entry__fprintf(struct hist_entry *self,
 		if (se->elide)
 			continue;
 
-		fprintf(fp, "%s", sep ?: "  ");
+		ret += fprintf(fp, "%s", sep ?: "  ");
 		ret += se->print(fp, self, se->width ? *se->width : 0);
 	}
 
@@ -544,8 +544,8 @@ static size_t hist_entry__fprintf(struct hist_entry *self,
 			left_margin -= thread__comm_len(self->thread);
 		}
 
-		hist_entry_callchain__fprintf(fp, self, session_total,
-					      left_margin);
+		ret += hist_entry_callchain__fprintf(fp, self, session_total,
+						     left_margin);
 	}
 
 	return ret;
-- 
1.6.2.5

[PATCH v2 5/5] perf report: Initial TUI using newt

[Arnaldo Carvalho de Melo]

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

Newt has widespread availability and provides a rather simple API as can be
seen by the size of this patch.

The work needed to support it will benefit other frontends too.

In this initial patch it just checks if the output is a tty, if not it falls
back to the previous behaviour, also if newt-devel/libnewt-dev is not installed
the previous behaviour is maintaned.

Pressing enter on a symbol will annotate it, ESC in the annotation window will
return to the report symbol list.

More work will be done to remove the special casing in color_fprintf, stop
using fmemopen/FILE in the printing of hist_entries, etc.

Also the annotation doesn't need to be done via spawning "perf annotate" and
then browsing its output, we can do better by calling directly the
builtin-annotate.c functions, that would then be moved to
tools/perf/util/annotate.c and shared with perf top, etc

But lets go by baby steps, this patch already improves perf usability by
allowing to quickly do annotations on symbols from the report screen and
provides a first experimentation with libnewt/TUI integration of tools.

Tested on RHEL5 and Fedora12 X86_64 and on Debian PARISC64 to browse a
perf.data file collected on a Fedora12 x86_64 box.

Cc: Avi Kivity <avi@redhat.com>
Cc: Frédéric Weisbecker <fweisbec@gmail.com>
Cc: Mike Galbraith <efault@gmx.de>
Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Paul Mackerras <paulus@samba.org>
Signed-off-by: Arnaldo Carvalho de Melo <acme@redhat.com>
---
 tools/perf/Makefile         |    8 ++
 tools/perf/builtin-report.c |   47 ++++++----
 tools/perf/perf.c           |    2 +
 tools/perf/util/cache.h     |   14 +++
 tools/perf/util/color.c     |    5 +-
 tools/perf/util/debug.c     |    6 +-
 tools/perf/util/debug.h     |    1 +
 tools/perf/util/hist.h      |    5 +
 tools/perf/util/newt.c      |  194 +++++++++++++++++++++++++++++++++++++++++++
 tools/perf/util/session.h   |    9 ++
 10 files changed, 270 insertions(+), 21 deletions(-)
 create mode 100644 tools/perf/util/newt.c

diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index 8a8f52d..0abd25e 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -513,6 +513,14 @@ else
 	LIB_OBJS += util/probe-finder.o
 endif
 
+ifneq ($(shell sh -c "(echo '\#include <newt.h>'; echo 'int main(void) { newtInit(); newtCls(); return newtFinished(); }') | $(CC) -x c - $(ALL_CFLAGS) -D_LARGEFILE64_SOURCE -D_FILE_OFFSET_BITS=64 -lnewt -o $(BITBUCKET) $(ALL_LDFLAGS) $(EXTLIBS) "$(QUIET_STDERR)" && echo y"), y)
+	msg := $(warning newt not found, disables TUI support. Please install newt-devel or libnewt-dev);
+	BASIC_CFLAGS += -DNO_NEWT_SUPPORT
+else
+	EXTLIBS += -lnewt
+	LIB_OBJS += util/newt.o
+endif
+
 ifndef NO_LIBPERL
 PERL_EMBED_LDOPTS = `perl -MExtUtils::Embed -e ldopts 2>/dev/null`
 PERL_EMBED_CCOPTS = `perl -MExtUtils::Embed -e ccopts 2>/dev/null`
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index f815de2..1f9f869 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -267,6 +267,7 @@ static int __cmd_report(void)
 	int ret = -EINVAL;
 	struct perf_session *session;
 	struct rb_node *next;
+	const char *help = "For a higher level overview, try: perf report --sort comm,dso";
 
 	session = perf_session__new(input_name, O_RDONLY, force);
 	if (session == NULL)
@@ -301,30 +302,38 @@ static int __cmd_report(void)
 		stats = rb_entry(next, struct event_stat_id, rb_node);
 		perf_session__collapse_resort(&stats->hists);
 		perf_session__output_resort(&stats->hists, stats->stats.total);
-		if (rb_first(&session->stats_by_id) ==
-		    rb_last(&session->stats_by_id))
-			fprintf(stdout, "# Samples: %Ld\n#\n",
-				stats->stats.total);
-		else
-			fprintf(stdout, "# Samples: %Ld %s\n#\n",
-				stats->stats.total,
-				__event_name(stats->type, stats->config));
 
-		perf_session__fprintf_hists(&stats->hists, NULL, false, stdout,
+		if (use_browser)
+			perf_session__browse_hists(&stats->hists,
+						   stats->stats.total, help);
+		else {
+			if (rb_first(&session->stats_by_id) ==
+			    rb_last(&session->stats_by_id))
+				fprintf(stdout, "# Samples: %Ld\n#\n",
+					stats->stats.total);
+			else
+				fprintf(stdout, "# Samples: %Ld %s\n#\n",
+					stats->stats.total,
+					__event_name(stats->type, stats->config));
+
+			perf_session__fprintf_hists(&stats->hists, NULL, false, stdout,
 					    stats->stats.total);
-		fprintf(stdout, "\n\n");
+			fprintf(stdout, "\n\n");
+		}
+
 		next = rb_next(&stats->rb_node);
 	}
 
-	if (sort_order == default_sort_order &&
-	    parent_pattern == default_parent_pattern)
-		fprintf(stdout, "#\n# (For a higher level overview, try: perf report --sort comm,dso)\n#\n");
+	if (!use_browser && sort_order == default_sort_order &&
+	    parent_pattern == default_parent_pattern) {
+		fprintf(stdout, "#\n# (%s)\n#\n", help);
 
-	if (show_threads) {
-		bool raw_printing_style = !strcmp(pretty_printing_style, "raw");
-		perf_read_values_display(stdout, &show_threads_values,
-					 raw_printing_style);
-		perf_read_values_destroy(&show_threads_values);
+		if (show_threads) {
+			bool style = !strcmp(pretty_printing_style, "raw");
+			perf_read_values_display(stdout, &show_threads_values,
+						 style);
+			perf_read_values_destroy(&show_threads_values);
+		}
 	}
 out_delete:
 	perf_session__delete(session);
@@ -447,7 +456,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __used)
 {
 	argc = parse_options(argc, argv, options, report_usage, 0);
 
-	setup_pager();
+	setup_browser();
 
 	if (symbol__init() < 0)
 		return -1;
diff --git a/tools/perf/perf.c b/tools/perf/perf.c
index 57cb107..9ff186b 100644
--- a/tools/perf/perf.c
+++ b/tools/perf/perf.c
@@ -265,6 +265,8 @@ static int run_builtin(struct cmd_struct *p, int argc, const char **argv)
 	if (status)
 		return status & 0xff;
 
+	exit_browser();
+
 	/* Somebody closed stdout? */
 	if (fstat(fileno(stdout), &st))
 		return 0;
diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
index 918eb37..47b12a3 100644
--- a/tools/perf/util/cache.h
+++ b/tools/perf/util/cache.h
@@ -1,6 +1,7 @@
 #ifndef __PERF_CACHE_H
 #define __PERF_CACHE_H
 
+#include <stdbool.h>
 #include "util.h"
 #include "strbuf.h"
 #include "../perf.h"
@@ -69,6 +70,19 @@ extern const char *pager_program;
 extern int pager_in_use(void);
 extern int pager_use_color;
 
+extern bool use_browser;
+
+#ifdef NO_NEWT_SUPPORT
+static inline void setup_browser(void)
+{
+	setup_pager();
+}
+static inline void exit_browser(void) {}
+#else
+void setup_browser(void);
+void exit_browser(void);
+#endif
+
 extern const char *editor_program;
 extern const char *excludes_file;
 
diff --git a/tools/perf/util/color.c b/tools/perf/util/color.c
index e88bca5..9da0191 100644
--- a/tools/perf/util/color.c
+++ b/tools/perf/util/color.c
@@ -203,7 +203,10 @@ int color_fprintf(FILE *fp, const char *color, const char *fmt, ...)
 	int r;
 
 	va_start(args, fmt);
-	r = color_vfprintf(fp, color, fmt, args);
+	if (use_browser)
+		r = vfprintf(fp, fmt, args);
+	else
+		r = color_vfprintf(fp, color, fmt, args);
 	va_end(args);
 	return r;
 }
diff --git a/tools/perf/util/debug.c b/tools/perf/util/debug.c
index 0905600..033d66d 100644
--- a/tools/perf/util/debug.c
+++ b/tools/perf/util/debug.c
@@ -6,6 +6,7 @@
 #include <stdarg.h>
 #include <stdio.h>
 
+#include "cache.h"
 #include "color.h"
 #include "event.h"
 #include "debug.h"
@@ -21,7 +22,10 @@ int eprintf(int level, const char *fmt, ...)
 
 	if (verbose >= level) {
 		va_start(args, fmt);
-		ret = vfprintf(stderr, fmt, args);
+		if (use_browser)
+			ret = browser__show_help(fmt, args);
+		else
+			ret = vfprintf(stderr, fmt, args);
 		va_end(args);
 	}
 
diff --git a/tools/perf/util/debug.h b/tools/perf/util/debug.h
index 58720a1..03accb8 100644
--- a/tools/perf/util/debug.h
+++ b/tools/perf/util/debug.h
@@ -9,5 +9,6 @@ extern int dump_trace;
 
 int dump_printf(const char *fmt, ...) __attribute__((format(printf, 1, 2)));
 void trace_event(event_t *event);
+int browser__show_help(const char *format, va_list ap);
 
 #endif	/* __PERF_DEBUG_H */
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 16f360c..fe366ce 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -18,6 +18,11 @@ struct hist_entry *__perf_session__add_hist_entry(struct rb_root *hists,
 						  u64 count, bool *hit);
 extern int64_t hist_entry__cmp(struct hist_entry *, struct hist_entry *);
 extern int64_t hist_entry__collapse(struct hist_entry *, struct hist_entry *);
+size_t hist_entry__fprintf(struct hist_entry *self,
+			   struct perf_session *pair_session,
+			   bool show_displacement,
+			   long displacement, FILE *fp,
+			   u64 session_total);
 void hist_entry__free(struct hist_entry *);
 
 void perf_session__output_resort(struct rb_root *hists, u64 total_samples);
diff --git a/tools/perf/util/newt.c b/tools/perf/util/newt.c
new file mode 100644
index 0000000..3d3a936
--- /dev/null
+++ b/tools/perf/util/newt.c
@@ -0,0 +1,194 @@
+#define _GNU_SOURCE
+#include <stdio.h>
+#undef _GNU_SOURCE
+
+#include <stdlib.h>
+#include <newt.h>
+
+#include "cache.h"
+#include "hist.h"
+#include "session.h"
+#include "sort.h"
+#include "symbol.h"
+
+static size_t hist_entry__append_browser(struct hist_entry *self,
+					 newtComponent listbox, u64 total)
+{
+	char bf[1024];
+	size_t len;
+	FILE *fp;
+
+	if (symbol_conf.exclude_other && !self->parent)
+		return 0;
+
+	fp = fmemopen(bf, sizeof(bf), "w");
+	if (fp == NULL)
+		return 0;
+
+	len = hist_entry__fprintf(self, NULL, false, 0, fp, total);
+
+	fclose(fp);
+	newtListboxAppendEntry(listbox, bf, self);
+	return len;
+}
+
+static void hist_entry__annotate_browser(struct hist_entry *self)
+{
+	FILE *fp;
+	struct winsize ws;
+	newtComponent form, listbox;
+	struct newtExitStruct es;
+	char *str;
+	size_t line_len, max_line_len = 0;
+	size_t max_usable_width;
+	char *line = NULL;
+
+	if (self->sym == NULL)
+		return;
+
+	if (asprintf(&str, "perf annotate %s | expand", self->sym->name) < 0)
+		return;
+
+	fp = popen(str, "r");
+	if (fp == NULL)
+		goto out_free_str;
+
+	newtPushHelpLine("Press ESC to exit");
+	get_term_dimensions(&ws);
+	listbox = newtListbox(0, 0, ws.ws_row - 5, NEWT_FLAG_SCROLL);
+
+	while (!feof(fp)) {
+		if (getline(&line, &line_len, fp) < 0 || !line_len)
+			break;
+		while (line_len != 0 && isspace(line[line_len - 1]))
+			line[--line_len] = '\0';
+
+		if (line_len > max_line_len)
+			max_line_len = line_len;
+		newtListboxAppendEntry(listbox, line, NULL);
+	}
+	fclose(fp);
+	free(line);
+
+	max_usable_width = ws.ws_col - 22;
+	if (max_line_len > max_usable_width)
+		max_line_len = max_usable_width;
+
+	newtListboxSetWidth(listbox, max_line_len);
+
+	newtCenteredWindow(max_line_len + 2, ws.ws_row - 5, self->sym->name);
+	form = newtForm(NULL, NULL, 0);
+	newtFormAddHotKey(form, NEWT_KEY_ESCAPE);
+	newtFormAddComponents(form, listbox, NULL);
+
+	newtFormRun(form, &es);
+	newtFormDestroy(form);
+	newtPopWindow();
+	newtPopHelpLine();
+out_free_str:
+	free(str);
+}
+
+void perf_session__browse_hists(struct rb_root *hists, u64 session_total,
+				const char *helpline)
+{
+	struct sort_entry *se;
+	struct rb_node *nd;
+	unsigned int width;
+	char *col_width = symbol_conf.col_width_list_str;
+	struct winsize ws;
+	size_t max_len = 0;
+	char str[1024];
+	newtComponent form, listbox;
+	struct newtExitStruct es;
+
+	snprintf(str, sizeof(str), "Samples: %Ld", session_total);
+	newtDrawRootText(0, 0, str);
+	newtPushHelpLine(helpline);
+
+	get_term_dimensions(&ws);
+
+	form = newtForm(NULL, NULL, 0);
+	newtFormAddHotKey(form, NEWT_KEY_ESCAPE);
+
+	listbox = newtListbox(1, 1, ws.ws_row - 2, (NEWT_FLAG_SCROLL |
+						    NEWT_FLAG_BORDER |
+						    NEWT_FLAG_RETURNEXIT));
+
+	list_for_each_entry(se, &hist_entry__sort_list, list) {
+		if (se->elide)
+			continue;
+		width = strlen(se->header);
+		if (se->width) {
+			if (symbol_conf.col_width_list_str) {
+				if (col_width) {
+					*se->width = atoi(col_width);
+					col_width = strchr(col_width, ',');
+					if (col_width)
+						++col_width;
+				}
+			}
+			*se->width = max(*se->width, width);
+		}
+	}
+
+	for (nd = rb_first(hists); nd; nd = rb_next(nd)) {
+		struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
+		size_t len = hist_entry__append_browser(h, listbox, session_total);
+		if (len > max_len)
+			max_len = len;
+	}
+
+	newtListboxSetWidth(listbox, max_len);
+	newtFormAddComponents(form, listbox, NULL);
+
+	while (1) {
+		struct hist_entry *selection;
+
+		newtFormRun(form, &es);
+		if (es.reason == NEWT_EXIT_HOTKEY)
+			break;
+		selection = newtListboxGetCurrent(listbox);
+		hist_entry__annotate_browser(selection);
+	}
+
+	newtFormDestroy(form);
+}
+
+int browser__show_help(const char *format, va_list ap)
+{
+	int ret;
+	static int backlog;
+	static char msg[1024];
+
+        ret = vsnprintf(msg + backlog, sizeof(msg) - backlog, format, ap);
+	backlog += ret;
+
+	if (msg[backlog - 1] == '\n') {
+		newtPopHelpLine();
+		newtPushHelpLine(msg);
+		newtRefresh();
+		backlog = 0;
+	}
+
+	return ret;
+}
+
+bool use_browser;
+
+void setup_browser(void)
+{
+	if (!isatty(1))
+		return;
+
+	use_browser = true;
+	newtInit();
+	newtCls();
+	newtPushHelpLine(" ");
+}
+
+void exit_browser(void)
+{
+	if (use_browser)
+		newtFinished();
+}
diff --git a/tools/perf/util/session.h b/tools/perf/util/session.h
index 5c33417..34d7339 100644
--- a/tools/perf/util/session.h
+++ b/tools/perf/util/session.h
@@ -86,4 +86,13 @@ static inline struct map *
 {
 	return map_groups__new_module(&self->kmaps, start, filename);
 }
+
+#ifdef NO_NEWT_SUPPORT
+static inline void perf_session__browse_hists(struct rb_root *hists __used,
+					      u64 session_total __used,
+					      const char *helpline __used) {}
+#else
+void perf_session__browse_hists(struct rb_root *hists, u64 session_total,
+				const char *helpline);
+#endif
 #endif /* __PERF_SESSION_H */
-- 
1.6.2.5

Re: [PATCH v2 5/5] perf report: Initial TUI using newt

[Arnaldo Carvalho de Melo]

Em Thu, Mar 11, 2010 at 08:12:44PM -0300, Arnaldo Carvalho de Melo escreveu:
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> Newt has widespread availability and provides a rather simple API as can be
> seen by the size of this patch.
> 
> The work needed to support it will benefit other frontends too.
> 
> In this initial patch it just checks if the output is a tty, if not it falls
> back to the previous behaviour, also if newt-devel/libnewt-dev is not installed
> the previous behaviour is maintaned.
> 
> Pressing enter on a symbol will annotate it, ESC in the annotation window will
> return to the report symbol list.

For those not so curious as to apply and try it out, here is a
screenshot of it in action:

http://tglx.de/~acme/perf-newt.png

:-)

- Arnaldo

Re: [PATCH v2 5/5] perf report: Initial TUI using newt

[Frederic Weisbecker]

On Thu, Mar 11, 2010 at 08:29:53PM -0300, Arnaldo Carvalho de Melo wrote:
> Em Thu, Mar 11, 2010 at 08:12:44PM -0300, Arnaldo Carvalho de Melo escreveu:
> > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> > 
> > Newt has widespread availability and provides a rather simple API as can be
> > seen by the size of this patch.
> > 
> > The work needed to support it will benefit other frontends too.
> > 
> > In this initial patch it just checks if the output is a tty, if not it falls
> > back to the previous behaviour, also if newt-devel/libnewt-dev is not installed
> > the previous behaviour is maintaned.
> > 
> > Pressing enter on a symbol will annotate it, ESC in the annotation window will
> > return to the report symbol list.
> 
> For those not so curious as to apply and try it out, here is a
> screenshot of it in action:
> 
> http://tglx.de/~acme/perf-newt.png
> 
> :-)


Wow! Linking from perf report to perf annotate in a simple key
in a TUI, my dreams become true :-)

Sweet.

Re: [PATCH v2 5/5] perf report: Initial TUI using newt

[Avi Kivity]

On 03/12/2010 01:29 AM, Arnaldo Carvalho de Melo wrote:
> Em Thu, Mar 11, 2010 at 08:12:44PM -0300, Arnaldo Carvalho de Melo escreveu:
>    
>> From: Arnaldo Carvalho de Melo<acme@redhat.com>
>>
>> Newt has widespread availability and provides a rather simple API as can be
>> seen by the size of this patch.
>>
>> The work needed to support it will benefit other frontends too.
>>
>> In this initial patch it just checks if the output is a tty, if not it falls
>> back to the previous behaviour, also if newt-devel/libnewt-dev is not installed
>> the previous behaviour is maintaned.
>>
>> Pressing enter on a symbol will annotate it, ESC in the annotation window will
>> return to the report symbol list.
>>      
> For those not so curious as to apply and try it out, here is a
> screenshot of it in action:
>
> http://tglx.de/~acme/perf-newt.png
>
>    

Looks really useful!

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [PATCH v2 5/5] perf report: Initial TUI using newt

[Mike Galbraith]

On Thu, 2010-03-11 at 20:12 -0300, Arnaldo Carvalho de Melo wrote:
> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> 

> In this initial patch it just checks if the output is a tty, if not it falls
> back to the previous behaviour, also if newt-devel/libnewt-dev is not installed
> the previous behaviour is maintaned.
> 
> Pressing enter on a symbol will annotate it, ESC in the annotation window will
> return to the report symbol list.

<build>

Oooooo, major case of happiness.  Nice!

	-Mike

Re: [PATCH v2 5/5] perf report: Initial TUI using newt

[Arnaldo Carvalho de Melo]

Em Fri, Mar 12, 2010 at 08:01:57AM +0100, Mike Galbraith escreveu:
> On Fri, 2010-03-12 at 07:48 +0100, Mike Galbraith wrote:
> > On Thu, 2010-03-11 at 20:12 -0300, Arnaldo Carvalho de Melo wrote:
> > > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> > > 
> > 
> > > In this initial patch it just checks if the output is a tty, if not it falls
> > > back to the previous behaviour, also if newt-devel/libnewt-dev is not installed
> > > the previous behaviour is maintaned.
> > > 
> > > Pressing enter on a symbol will annotate it, ESC in the annotation window will
> > > return to the report symbol list.
> > 
> > <build>
> > 
> > Oooooo, major case of happiness.  Nice!
> 
> On the fly event switching ala top in the oven?

Yeah, newt has a FormTimer thing that will exit if the user doesn't
press a key, that will be the time when we refresh the Listbox, etc.
When some key is pressed, we can use the opportunity to switch the
counter or to do annotation, sharing a widget with the report, etc.

Switching perf.data files should be possible, doing diffs using side by
side screens, etc. The widget should allow for N 'perf top' windows, one
per counter, so that we can monitor multiple counters at the same time,
going from live (perf top) to post morten (perf report), doing live
while collecting (perf top + perf record) for later, on another machine,
doing post morten (perf report) also should be possible, etc.

- Arnaldo

Re: [PATCH v2 5/5] perf report: Initial TUI using newt

[Ingo Molnar]

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

> From: Arnaldo Carvalho de Melo <acme@redhat.com>
> 
> Newt has widespread availability and provides a rather simple API as can be 
> seen by the size of this patch.
> 
> The work needed to support it will benefit other frontends too.
> 
> In this initial patch it just checks if the output is a tty, if not it falls 
> back to the previous behaviour, also if newt-devel/libnewt-dev is not 
> installed the previous behaviour is maintaned.
> 
> Pressing enter on a symbol will annotate it, ESC in the annotation window will
> return to the report symbol list.

Very nice!

a few observations. Firstly, could we perhaps make most of the interface 
functions GUI/TUI invariant? I.e. things like:

> +	if (use_browser)
> +		r = vfprintf(fp, fmt, args);
> +	else
> +		r = color_vfprintf(fp, color, fmt, args);

should be abstracted away into a single method:

	r = color_vprintf(fp, color, fmt, args);

where color_vprintf() knows about which current GUI front-end to use.

(The old color_printf() should be renamed to ascii_color_printf() or so, and 
put into a front-end driver structure perhaps - instead of explicit flags.)

There's a similar situation here too:

> -		ret = vfprintf(stderr, fmt, args);
> +		if (use_browser)
> +			ret = browser__show_help(fmt, args);
> +		else
> +			ret = vfprintf(stderr, fmt, args);

Plus a few other observations about the newt TUI itself:

 - The most important first-impression thing in a TUI is to make it obvious to
   exit it. I eventually found that Escape would exit - but it would be nice 
   to map 'Q' and 'Ctrl-C' to it as well. Nothing is more annoying than a TUI 
   you cannot exit from.

 - There's still a 'perf annotate' bug that has been introduced recently, and 
   it shows up in the TUI too. The bug is due to us passing this to objdump 
   and grep:

     18573 execve("/bin/sh", ["sh", "-c", "objdump --start-address=0xffffffff81387b36
       --stop-address=0xffffffff81387b4f -dS [kernel.kallsyms]|grep -v [kernel.kallsyms]"]

   Look at how [kernel.kallsyms] goes unquoted to the shell, so globbing will 
   match it on random file names in the current directory - which will then be 
   showed by objdump, much to the surprise of the user!

 - I suspect we should finally make use of the .perfconfig parser and enable people
   to use a different front-end from Newt? Just in case they prefer ASCII.

 - When i hit enter on a symbol to annotate it, but the annotation fails, the TUI
   just does nothing currently. Instead it should print something informative 
   (and eye-catching) into a status line at the top or the bottom of the 
   screen, possibly printed in red characters or so. Not a separate window as that
   needs extra key-hits to get rid of - just a sufficiently visible status 
   line would be perfect. There can be a few reasons why some functions can be 
   annotated while others cannot be.

 - [ call-graph data is not represented yet :-) ]

Anyway, very nice stuff!

	Ingo

Re: [PATCH v2 5/5] perf report: Initial TUI using newt

[Avi Kivity]

On 03/12/2010 11:45 AM, Ingo Molnar wrote:
>
>   - When i hit enter on a symbol to annotate it, but the annotation fails, the TUI
>     just does nothing currently. Instead it should print something informative
>     (and eye-catching) into a status line at the top or the bottom of the
>     screen, possibly printed in red characters or so. Not a separate window as that
>     needs extra key-hits to get rid of - just a sufficiently visible status
>     line would be perfect. There can be a few reasons why some functions can be
>     annotated while others cannot be.
>    

Alternatively, mark it as unannotatable in the first place (e.g. an 
annotatable symbol is a hyperlink, an unannotatable one is not).  But 
perhaps that's too computationally expensive.

-- 
Do not meddle in the internals of kernels, for they are subtle and quick to panic.

Re: [PATCH v2 5/5] perf report: Initial TUI using newt

[Arnaldo Carvalho de Melo]

Em Fri, Mar 12, 2010 at 11:55:04AM +0200, Avi Kivity escreveu:
> On 03/12/2010 11:45 AM, Ingo Molnar wrote:
>>
>>   - When i hit enter on a symbol to annotate it, but the annotation fails, the TUI
>>     just does nothing currently. Instead it should print something informative
>>     (and eye-catching) into a status line at the top or the bottom of the
>>     screen, possibly printed in red characters or so. Not a separate window as that
>>     needs extra key-hits to get rid of - just a sufficiently visible status
>>     line would be perfect. There can be a few reasons why some functions can be
>>     annotated while others cannot be.
>
> Alternatively, mark it as unannotatable in the first place (e.g. an  
> annotatable symbol is a hyperlink, an unannotatable one is not).  But  
> perhaps that's too computationally expensive.

I'll come up with some way to do that, changing the color or adding
markings to highlight the top functions, like the stdio one does with
red for te top functions, etc is needed.

- Arnaldo

Re: [PATCH v2 5/5] perf report: Initial TUI using newt

[Arnaldo Carvalho de Melo]

Em Fri, Mar 12, 2010 at 10:45:33AM +0100, Ingo Molnar escreveu:
> 
> * Arnaldo Carvalho de Melo <acme@infradead.org> wrote:
> 
> > From: Arnaldo Carvalho de Melo <acme@redhat.com>
> > 
> > Newt has widespread availability and provides a rather simple API as can be 
> > seen by the size of this patch.
> > 
> > The work needed to support it will benefit other frontends too.
> > 
> > In this initial patch it just checks if the output is a tty, if not it falls 
> > back to the previous behaviour, also if newt-devel/libnewt-dev is not 
> > installed the previous behaviour is maintaned.
> > 
> > Pressing enter on a symbol will annotate it, ESC in the annotation window will
> > return to the report symbol list.
> 
> Very nice!
> 
> a few observations. Firstly, could we perhaps make most of the interface 
> functions GUI/TUI invariant? I.e. things like:
> 
> > +	if (use_browser)
> > +		r = vfprintf(fp, fmt, args);
> > +	else
> > +		r = color_vfprintf(fp, color, fmt, args);
> 
> should be abstracted away into a single method:
> 
> 	r = color_vprintf(fp, color, fmt, args);
> 
> where color_vprintf() knows about which current GUI front-end to use.

Yeah, I'll get to that, its just that I wanted to have something out as
small as possible and that pointed to the places that need refactoring
to properly support multiple UI frontends.
 
> (The old color_printf() should be renamed to ascii_color_printf() or so, and 
> put into a front-end driver structure perhaps - instead of explicit flags.)

Right
 
> There's a similar situation here too:
> 
> > -		ret = vfprintf(stderr, fmt, args);
> > +		if (use_browser)
> > +			ret = browser__show_help(fmt, args);
> > +		else
> > +			ret = vfprintf(stderr, fmt, args);
> 
> Plus a few other observations about the newt TUI itself:
> 
>  - The most important first-impression thing in a TUI is to make it obvious to
>    exit it. I eventually found that Escape would exit - but it would be nice 
>    to map 'Q' and 'Ctrl-C' to it as well. Nothing is more annoying than a TUI 
>    you cannot exit from.

Newt's default is F12, I had to killall perf while developing and
getting to know newtFormAddHotKey to exit, will add the other usual keys to
exit.
 
>  - There's still a 'perf annotate' bug that has been introduced recently, and 
>    it shows up in the TUI too. The bug is due to us passing this to objdump 
>    and grep:
> 
>      18573 execve("/bin/sh", ["sh", "-c", "objdump --start-address=0xffffffff81387b36
>        --stop-address=0xffffffff81387b4f -dS [kernel.kallsyms]|grep -v [kernel.kallsyms]"]
> 
>    Look at how [kernel.kallsyms] goes unquoted to the shell, so globbing will 
>    match it on random file names in the current directory - which will then be 
>    showed by objdump, much to the surprise of the user!

Will fix that, if we don't find a vmlinux file, the kernel symbols will
be marked non annotable in some way.
 
>  - I suspect we should finally make use of the .perfconfig parser and enable people
>    to use a different front-end from Newt? Just in case they prefer ASCII.
> 
>  - When i hit enter on a symbol to annotate it, but the annotation fails, the TUI
>    just does nothing currently. Instead it should print something informative 
>    (and eye-catching) into a status line at the top or the bottom of the 
>    screen, possibly printed in red characters or so. Not a separate window as that
>    needs extra key-hits to get rid of - just a sufficiently visible status 
>    line would be perfect. There can be a few reasons why some functions can be 
>    annotated while others cannot be.

Right.
 
>  - [ call-graph data is not represented yet :-) ]

It will
 
> Anyway, very nice stuff!

Thanks!

- Arnaldo