2012-02-23 16:18:52

by Pekka Enberg

[permalink] [raw]
Subject: [PATCH] perf report: Add a simple GTK2-based 'perf report' browser

This patch adds a simple GTK2-based browser to 'perf report' that's based on
the TTY-based browser in builtin-report.c.

Please not that you need to use

make WERROR=0

to build perf on Fedora 15 (and possibly other distributions) because GTK
headers do not compile cleanly:

CC util/gtk/browser.o
In file included from /usr/include/gtk-2.0/gtk/gtk.h:234:0,
from util/gtk/browser.c:7:
/usr/include/gtk-2.0/gtk/gtkitemfactory.h:47:1: warning: function declaration isn’t a prototype [-Wstrict-prototypes]

To launch "perf report" using the new GTK interface just type:

./perf report --gtk

The interface is somewhat limited in features at the moment:

- No callgraph support

- No KVM guest profiling support

- No color coding for percentages

- No sorting from the UI

- ..and many, many more!

That said, I think this patch a reasonable start to build future features on.

Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Signed-off-by: Pekka Enberg <[email protected]>
---
I've uploaded a screenshot of the new UI here:

http://i.imgur.com/S93cu.png

tools/perf/Documentation/perf-report.txt | 2 +
tools/perf/Makefile | 14 +++
tools/perf/builtin-report.c | 24 +++-
tools/perf/config/feature-tests.mak | 13 ++
tools/perf/util/cache.h | 12 ++
tools/perf/util/gtk/browser.c | 189 ++++++++++++++++++++++++++++++
tools/perf/util/hist.h | 17 +++
7 files changed, 265 insertions(+), 6 deletions(-)
create mode 100644 tools/perf/util/gtk/browser.c

diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
index 9b430e9..9654f27 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -110,6 +110,8 @@ OPTIONS
requires a tty, if one is not present, as when piping to other
commands, the stdio interface is used.

+--gtk:: Use the GTK2 interface.
+
-k::
--vmlinux=<file>::
vmlinux pathname
diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index e011b50..371f114 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -485,6 +485,20 @@ else
endif
endif

+ifdef NO_GTK2
+ BASIC_CFLAGS += -DNO_GTK2
+else
+ FLAGS_GTK2=$(ALL_CFLAGS) $(ALL_LDFLAGS) $(EXTLIBS) $(shell pkg-config --libs --cflags gtk+-2.0)
+ ifneq ($(call try-cc,$(SOURCE_GTK2),$(FLAGS_GTK2)),y)
+ msg := $(warning GTK2 not found, disables GTK2 support. Please install gtk2-devel or libgtk2.0-dev);
+ BASIC_CFLAGS += -DNO_GTK2_SUPPORT
+ else
+ BASIC_CFLAGS += $(shell pkg-config --cflags gtk+-2.0)
+ EXTLIBS += $(shell pkg-config --libs gtk+-2.0)
+ LIB_OBJS += $(OUTPUT)util/gtk/browser.o
+ endif
+endif
+
ifdef NO_LIBPERL
BASIC_CFLAGS += -DNO_LIBPERL
else
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 25d34d4..e0a5d17 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -40,7 +40,7 @@ struct perf_report {
struct perf_tool tool;
struct perf_session *session;
char const *input_name;
- bool force, use_tui, use_stdio;
+ bool force, use_tui, use_gtk, use_stdio;
bool hide_unresolved;
bool dont_use_callchains;
bool show_full_info;
@@ -224,6 +224,7 @@ static int perf_evlist__tty_browse_hists(struct perf_evlist *evlist,

hists__fprintf_nr_sample_events(hists, evname, stdout);
hists__fprintf(hists, NULL, false, true, 0, 0, stdout);
+
fprintf(stdout, "\n\n");
}

@@ -326,8 +327,13 @@ static int __cmd_report(struct perf_report *rep)
}

if (use_browser > 0) {
- perf_evlist__tui_browse_hists(session->evlist, help,
- NULL, NULL, 0);
+ if (use_browser == 1) {
+ perf_evlist__tui_browse_hists(session->evlist, help,
+ NULL, NULL, 0);
+ } else if (use_browser == 2) {
+ perf_evlist__gtk_browse_hists(session->evlist, help,
+ NULL, NULL, 0);
+ }
} else
perf_evlist__tty_browse_hists(session->evlist, rep, help);

@@ -474,6 +480,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __used)
OPT_STRING(0, "pretty", &report.pretty_printing_style, "key",
"pretty printing style key: normal raw"),
OPT_BOOLEAN(0, "tui", &report.use_tui, "Use the TUI interface"),
+ OPT_BOOLEAN(0, "gtk", &report.use_gtk, "Use the GTK2 interface"),
OPT_BOOLEAN(0, "stdio", &report.use_stdio,
"Use the stdio interface"),
OPT_STRING('s', "sort", &sort_order, "key[,key2...]",
@@ -526,6 +533,8 @@ int cmd_report(int argc, const char **argv, const char *prefix __used)
use_browser = 0;
else if (report.use_tui)
use_browser = 1;
+ else if (report.use_gtk)
+ use_browser = 2;

if (report.inverted_callchain)
callchain_param.order = ORDER_CALLER;
@@ -537,9 +546,12 @@ int cmd_report(int argc, const char **argv, const char *prefix __used)
report.input_name = "perf.data";
}

- if (strcmp(report.input_name, "-") != 0)
- setup_browser(true);
- else
+ if (strcmp(report.input_name, "-") != 0) {
+ if (report.use_tui)
+ setup_browser(true);
+ else if (report.use_gtk)
+ perf_gtk_setup_browser(argc, argv, true);
+ } else
use_browser = 0;

/*
diff --git a/tools/perf/config/feature-tests.mak b/tools/perf/config/feature-tests.mak
index 6170fd2..f5b551c 100644
--- a/tools/perf/config/feature-tests.mak
+++ b/tools/perf/config/feature-tests.mak
@@ -65,6 +65,19 @@ int main(void)
endef
endif

+ifndef NO_GTK2
+define SOURCE_GTK2
+#include <gtk/gtk.h>
+
+int main(int argc, char *argv[])
+{
+ gtk_init(&argc, &argv);
+
+ return 0;
+}
+endef
+endif
+
ifndef NO_LIBPERL
define SOURCE_PERL_EMBED
#include <EXTERN.h>
diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
index fc5e5a0..8dd224d 100644
--- a/tools/perf/util/cache.h
+++ b/tools/perf/util/cache.h
@@ -45,6 +45,18 @@ void setup_browser(bool fallback_to_pager);
void exit_browser(bool wait_for_ok);
#endif

+#ifdef NO_GTK2_SUPPORT
+static inline void perf_gtk_setup_browser(int argc __used, const char *argv[] __used, bool fallback_to_pager)
+{
+ if (fallback_to_pager)
+ setup_pager();
+}
+static inline void perf_gtk_exit_browser(bool wait_for_ok __used) {}
+#else
+void perf_gtk_setup_browser(int argc, const char *argv[], bool fallback_to_pager);
+void perf_gtk_exit_browser(bool wait_for_ok);
+#endif
+
char *alias_lookup(const char *alias);
int split_cmdline(char *cmdline, const char ***argv);

diff --git a/tools/perf/util/gtk/browser.c b/tools/perf/util/gtk/browser.c
new file mode 100644
index 0000000..4878279
--- /dev/null
+++ b/tools/perf/util/gtk/browser.c
@@ -0,0 +1,189 @@
+#include "../evlist.h"
+#include "../cache.h"
+#include "../evsel.h"
+#include "../sort.h"
+#include "../hist.h"
+
+#include <gtk/gtk.h>
+#include <signal.h>
+
+#define MAX_COLUMNS 32
+
+void perf_gtk_setup_browser(int argc, const char *argv[],
+ bool fallback_to_pager __used)
+{
+ gtk_init(&argc, (char ***)&argv);
+}
+
+void perf_gtk_exit_browser(bool wait_for_ok __used)
+{
+ gtk_main_quit();
+}
+
+static void perf_gtk_signal(int sig)
+{
+ psignal(sig, "perf");
+ gtk_main_quit();
+}
+
+static void perf_gtk_resize_window(GtkWidget *window)
+{
+ GdkRectangle rect;
+ GdkScreen *screen;
+ int monitor;
+ int height;
+ int width;
+
+ screen = gtk_widget_get_screen(window);
+
+ monitor = gdk_screen_get_monitor_at_window(screen, window->window);
+
+ gdk_screen_get_monitor_geometry(screen, monitor, &rect);
+
+ width = rect.width * 3 / 4;
+ height = rect.height * 3 / 4;
+
+ gtk_window_resize(GTK_WINDOW(window), width, height);
+}
+
+static void perf_gtk_show_hists(GtkWidget *window, struct hists *hists)
+{
+ GType col_types[MAX_COLUMNS];
+ GtkCellRenderer *renderer;
+ struct sort_entry *se;
+ GtkListStore *store;
+ struct rb_node *nd;
+ u64 total_period;
+ GtkWidget *view;
+ int col_idx;
+ int nr_cols;
+
+ nr_cols = 0;
+
+ /* The percentage column */
+ col_types[nr_cols++] = G_TYPE_STRING;
+
+ list_for_each_entry(se, &hist_entry__sort_list, list) {
+ if (se->elide)
+ continue;
+
+ col_types[nr_cols++] = G_TYPE_STRING;
+ }
+
+ store = gtk_list_store_newv(nr_cols, col_types);
+
+ view = gtk_tree_view_new();
+
+ renderer = gtk_cell_renderer_text_new();
+
+ col_idx = 0;
+
+ /* The percentage column */
+ gtk_tree_view_insert_column_with_attributes(GTK_TREE_VIEW(view),
+ -1, "Overhead (%)",
+ renderer, "text",
+ col_idx++, NULL);
+
+ list_for_each_entry(se, &hist_entry__sort_list, list) {
+ if (se->elide)
+ continue;
+
+ gtk_tree_view_insert_column_with_attributes(GTK_TREE_VIEW(view),
+ -1, se->se_header,
+ renderer, "text",
+ col_idx++, NULL);
+ }
+
+ gtk_tree_view_set_model(GTK_TREE_VIEW(view), GTK_TREE_MODEL(store));
+
+ g_object_unref(GTK_TREE_MODEL(store));
+
+ total_period = hists->stats.total_period;
+
+ for (nd = rb_first(&hists->entries); nd; nd = rb_next(nd)) {
+ struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
+ GtkTreeIter iter;
+ double percent;
+ char s[512];
+
+ if (h->filtered)
+ continue;
+
+ gtk_list_store_append(store, &iter);
+
+ col_idx = 0;
+
+ percent = (h->period * 100.0) / total_period;
+
+ snprintf(s, ARRAY_SIZE(s), "%.2f", percent);
+
+ gtk_list_store_set(store, &iter, col_idx++, s, -1);
+
+ list_for_each_entry(se, &hist_entry__sort_list, list) {
+ if (se->elide)
+ continue;
+
+ se->se_snprintf(h, s, ARRAY_SIZE(s),
+ hists__col_len(hists, se->se_width_idx));
+
+ gtk_list_store_set(store, &iter, col_idx++, s, -1);
+ }
+ }
+
+ gtk_container_add(GTK_CONTAINER(window), view);
+}
+
+int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
+ const char *help __used,
+ void (*timer) (void *arg)__used,
+ void *arg __used, int delay_secs __used)
+{
+ struct perf_evsel *pos;
+ GtkWidget *notebook;
+ GtkWidget *window;
+
+ signal(SIGSEGV, perf_gtk_signal);
+ signal(SIGFPE, perf_gtk_signal);
+ signal(SIGINT, perf_gtk_signal);
+ signal(SIGQUIT, perf_gtk_signal);
+ signal(SIGTERM, perf_gtk_signal);
+
+ window = gtk_window_new(GTK_WINDOW_TOPLEVEL);
+
+ gtk_window_set_title(GTK_WINDOW(window), "perf report");
+
+ g_signal_connect(window, "delete_event", gtk_main_quit, NULL);
+
+ notebook = gtk_notebook_new();
+
+ list_for_each_entry(pos, &evlist->entries, node) {
+ struct hists *hists = &pos->hists;
+ const char *evname = event_name(pos);
+ GtkWidget *scrolled_window;
+ GtkWidget *tab_label;
+
+ scrolled_window = gtk_scrolled_window_new(NULL, NULL);
+
+ gtk_scrolled_window_set_policy(GTK_SCROLLED_WINDOW(scrolled_window),
+ GTK_POLICY_AUTOMATIC,
+ GTK_POLICY_AUTOMATIC);
+
+ perf_gtk_show_hists(scrolled_window, hists);
+
+ tab_label = gtk_label_new(evname);
+
+ gtk_notebook_append_page(GTK_NOTEBOOK(notebook), scrolled_window, tab_label);
+ }
+
+ gtk_container_add(GTK_CONTAINER(window), notebook);
+
+ gtk_widget_show_all(window);
+
+ perf_gtk_resize_window(window);
+
+ gtk_window_set_position(GTK_WINDOW(window), GTK_WIN_POS_CENTER);
+
+ gtk_main();
+
+ return 0;
+}
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 48e5acd..3be98b2 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -134,6 +134,23 @@ int perf_evlist__tui_browse_hists(struct perf_evlist *evlist, const char *help,
int refresh);
#endif

+#ifdef NO_GTK2_SUPPORT
+static inline
+int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist __used,
+ const char *help __used,
+ void(*timer)(void *arg) __used,
+ void *arg __used,
+ int refresh __used)
+{
+ return 0;
+}
+
+#else
+int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist, const char *help,
+ void(*timer)(void *arg), void *arg,
+ int refresh);
+#endif
+
unsigned int hists__sort_list_width(struct hists *self);

#endif /* __PERF_HIST_H */
--
1.7.6.5


2012-02-23 16:28:59

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf report: Add a simple GTK2-based 'perf report' browser

Em Thu, Feb 23, 2012 at 06:18:42PM +0200, Pekka Enberg escreveu:
> This patch adds a simple GTK2-based browser to 'perf report' that's based on
> the TTY-based browser in builtin-report.c.
>
> Please not that you need to use
>
> make WERROR=0
>
> to build perf on Fedora 15 (and possibly other distributions) because GTK
> headers do not compile cleanly:
>
> CC util/gtk/browser.o
> In file included from /usr/include/gtk-2.0/gtk/gtk.h:234:0,
> from util/gtk/browser.c:7:
> /usr/include/gtk-2.0/gtk/gtkitemfactory.h:47:1: warning: function declaration isn’t a prototype [-Wstrict-prototypes]
>
> To launch "perf report" using the new GTK interface just type:
>
> ./perf report --gtk
>
> The interface is somewhat limited in features at the moment:
>
> - No callgraph support
>
> - No KVM guest profiling support
>
> - No color coding for percentages
>
> - No sorting from the UI
>
> - ..and many, many more!
>
> That said, I think this patch a reasonable start to build future features on.

<SNIP>

> + for (nd = rb_first(&hists->entries); nd; nd = rb_next(nd)) {
> + struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
> + GtkTreeIter iter;
> + double percent;
> + char s[512];
> +
> + if (h->filtered)
> + continue;
> +
> + gtk_list_store_append(store, &iter);
> +
> + col_idx = 0;
> +
> + percent = (h->period * 100.0) / total_period;
> +
> + snprintf(s, ARRAY_SIZE(s), "%.2f", percent);
> +
> + gtk_list_store_set(store, &iter, col_idx++, s, -1);
> +
> + list_for_each_entry(se, &hist_entry__sort_list, list) {
> + if (se->elide)
> + continue;
> +
> + se->se_snprintf(h, s, ARRAY_SIZE(s),
> + hists__col_len(hists, se->se_width_idx));
> +
> + gtk_list_store_set(store, &iter, col_idx++, s, -1);
> + }
> + }

This is what I avoided by writing a tree widget for the TUI, I really
didn't want to traverse _all_ the hists just to show the ones that
appear in the screen.

Isn't there a way to ask GTK to ask a callback to provide a
representation for the Nth hist entry, i.e. just the ones it _needs_ to
render the screen?

Anyway, great to see this work!

- Arnaldo

2012-02-23 16:32:43

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH] perf report: Add a simple GTK2-based 'perf report' browser

On Thu, Feb 23, 2012 at 6:28 PM, Arnaldo Carvalho de Melo
<[email protected]> wrote:
> Em Thu, Feb 23, 2012 at 06:18:42PM +0200, Pekka Enberg escreveu:
>> This patch adds a simple GTK2-based browser to 'perf report' that's based on
>> the TTY-based browser in builtin-report.c.
>>
>> Please not that you need to use
>>
>> ? make WERROR=0
>>
>> to build perf on Fedora 15 (and possibly other distributions) because GTK
>> headers do not compile cleanly:
>>
>> ? ? ? CC util/gtk/browser.o
>> ? In file included from /usr/include/gtk-2.0/gtk/gtk.h:234:0,
>> ? ? ? ? ? ? ? ? ? ?from util/gtk/browser.c:7:
>> ? /usr/include/gtk-2.0/gtk/gtkitemfactory.h:47:1: warning: function declaration isn?t a prototype [-Wstrict-prototypes]
>>
>> To launch "perf report" using the new GTK interface just type:
>>
>> ? ./perf report --gtk
>>
>> The interface is somewhat limited in features at the moment:
>>
>> ? - No callgraph support
>>
>> ? - No KVM guest profiling support
>>
>> ? - No color coding for percentages
>>
>> ? - No sorting from the UI
>>
>> ? - ..and many, many more!
>>
>> That said, I think this patch a reasonable start to build future features on.
>
> <SNIP>
>
>> + ? ? for (nd = rb_first(&hists->entries); nd; nd = rb_next(nd)) {
>> + ? ? ? ? ? ? struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
>> + ? ? ? ? ? ? GtkTreeIter iter;
>> + ? ? ? ? ? ? double percent;
>> + ? ? ? ? ? ? char s[512];
>> +
>> + ? ? ? ? ? ? if (h->filtered)
>> + ? ? ? ? ? ? ? ? ? ? continue;
>> +
>> + ? ? ? ? ? ? gtk_list_store_append(store, &iter);
>> +
>> + ? ? ? ? ? ? col_idx = 0;
>> +
>> + ? ? ? ? ? ? percent = (h->period * 100.0) / total_period;
>> +
>> + ? ? ? ? ? ? snprintf(s, ARRAY_SIZE(s), "%.2f", percent);
>> +
>> + ? ? ? ? ? ? gtk_list_store_set(store, &iter, col_idx++, s, -1);
>> +
>> + ? ? ? ? ? ? list_for_each_entry(se, &hist_entry__sort_list, list) {
>> + ? ? ? ? ? ? ? ? ? ? if (se->elide)
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? continue;
>> +
>> + ? ? ? ? ? ? ? ? ? ? se->se_snprintf(h, s, ARRAY_SIZE(s),
>> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? hists__col_len(hists, se->se_width_idx));
>> +
>> + ? ? ? ? ? ? ? ? ? ? gtk_list_store_set(store, &iter, col_idx++, s, -1);
>> + ? ? ? ? ? ? }
>> + ? ? }
>
> This is what I avoided by writing a tree widget for the TUI, I really
> didn't want to traverse _all_ the hists just to show the ones that
> appear in the screen.
>
> Isn't there a way to ask GTK to ask a callback to provide a
> representation for the Nth hist entry, i.e. just the ones it _needs_ to
> render the screen?

Dunno, probably. Does that matter, though? It's pretty damn fast as-is.

Pekka

2012-02-23 16:39:48

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf report: Add a simple GTK2-based 'perf report' browser

Em Thu, Feb 23, 2012 at 06:32:41PM +0200, Pekka Enberg escreveu:
> On Thu, Feb 23, 2012 at 6:28 PM, Arnaldo Carvalho de Melo <[email protected]> wrote:
> > This is what I avoided by writing a tree widget for the TUI, I really
> > didn't want to traverse _all_ the hists just to show the ones that
> > appear in the screen.
> >
> > Isn't there a way to ask GTK to ask a callback to provide a
> > representation for the Nth hist entry, i.e. just the ones it _needs_ to
> > render the screen?
>
> Dunno, probably. Does that matter, though? It's pretty damn fast as-is.

Well, what kinds of perf.data file are you feeding it?

I'm testing it now with large ones, lets see.

- Arnaldo

2012-02-23 16:45:09

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH] perf report: Add a simple GTK2-based 'perf report' browser

On Thu, 2012-02-23 at 14:39 -0200, Arnaldo Carvalho de Melo wrote:
> > Dunno, probably. Does that matter, though? It's pretty damn fast as-is.
>
> Well, what kinds of perf.data file are you feeding it?

It's a real-world case where I'm profiling JRuby startup under Jato with
perf:

penberg@jaguar:~/src/jato$ ls -lh perf.data
-rw------- 1 penberg penberg 453K 2012-02-23 18:06 perf.data

On Thu, 2012-02-23 at 14:39 -0200, Arnaldo Carvalho de Melo wrote:
> I'm testing it now with large ones, lets see.

How big files are we talking about here?

Pekka

2012-02-23 16:47:47

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf report: Add a simple GTK2-based 'perf report' browser

Em Thu, Feb 23, 2012 at 06:45:04PM +0200, Pekka Enberg escreveu:
> On Thu, 2012-02-23 at 14:39 -0200, Arnaldo Carvalho de Melo wrote:
> > > Dunno, probably. Does that matter, though? It's pretty damn fast as-is.
> >
> > Well, what kinds of perf.data file are you feeding it?
>
> It's a real-world case where I'm profiling JRuby startup under Jato with
> perf:
>
> penberg@jaguar:~/src/jato$ ls -lh perf.data
> -rw------- 1 penberg penberg 453K 2012-02-23 18:06 perf.data
>
> On Thu, 2012-02-23 at 14:39 -0200, Arnaldo Carvalho de Melo wrote:
> > I'm testing it now with large ones, lets see.
>
> How big files are we talking about here?

5 MB ones, say.

But the way you did it is pretty minimalistic, which is good, its just
that I don't really like the idea of having two mirror data structures
representing the report lines.

- Arnaldo

2012-02-23 16:53:11

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf report: Add a simple GTK2-based 'perf report' browser

Em Thu, Feb 23, 2012 at 02:47:39PM -0200, Arnaldo Carvalho de Melo escreveu:
> Em Thu, Feb 23, 2012 at 06:45:04PM +0200, Pekka Enberg escreveu:
> > On Thu, 2012-02-23 at 14:39 -0200, Arnaldo Carvalho de Melo wrote:
> > > > Dunno, probably. Does that matter, though? It's pretty damn fast as-is.
> > >
> > > Well, what kinds of perf.data file are you feeding it?
> >
> > It's a real-world case where I'm profiling JRuby startup under Jato with
> > perf:
> >
> > penberg@jaguar:~/src/jato$ ls -lh perf.data
> > -rw------- 1 penberg penberg 453K 2012-02-23 18:06 perf.data
> >
> > On Thu, 2012-02-23 at 14:39 -0200, Arnaldo Carvalho de Melo wrote:
> > > I'm testing it now with large ones, lets see.
> >
> > How big files are we talking about here?
>
> 5 MB ones, say.

Nah:

[root@felicio linux]# perf record -a -F 10000 sleep 5m
[ perf record: Woken up 166 times to write data ]
[ perf record: Captured and wrote 42.104 MB perf.data (~1839573 samples)
]
[root@felicio linux]#
[root@felicio linux]#
[root@felicio linux]# perf report --gtk

PID USER PR NI VIRT RES SHR S %CPU %MEM TIME+ COMMAND
10618 root 20 0 408m 170m 49m S 0.0 2.2 0:04.71 perf

[root@felicio linux]# perf report --tui

10633 root 20 0 379m 165m 45m S 0.0 2.1 0:01.15 perf

> But the way you did it is pretty minimalistic, which is good, its just
> that I don't really like the idea of having two mirror data structures
> representing the report lines.

- Arnaldo

2012-02-23 17:10:50

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH] perf report: Add a simple GTK2-based 'perf report' browser

On Thu, Feb 23, 2012 at 6:53 PM, Arnaldo Carvalho de Melo
<[email protected]> wrote:
> Em Thu, Feb 23, 2012 at 02:47:39PM -0200, Arnaldo Carvalho de Melo escreveu:
>> Em Thu, Feb 23, 2012 at 06:45:04PM +0200, Pekka Enberg escreveu:
>> > On Thu, 2012-02-23 at 14:39 -0200, Arnaldo Carvalho de Melo wrote:
>> > > > Dunno, probably. Does that matter, though? It's pretty damn fast as-is.
>> > >
>> > > Well, what kinds of perf.data file are you feeding it?
>> >
>> > It's a real-world case where I'm profiling JRuby startup under Jato with
>> > perf:
>> >
>> > penberg@jaguar:~/src/jato$ ls -lh perf.data
>> > -rw------- 1 penberg penberg 453K 2012-02-23 18:06 perf.data
>> >
>> > On Thu, 2012-02-23 at 14:39 -0200, Arnaldo Carvalho de Melo wrote:
>> > > I'm testing it now with large ones, lets see.
>> >
>> > How big files are we talking about here?
>>
>> 5 MB ones, say.
>
> Nah:
>
> [root@felicio linux]# perf record -a -F 10000 sleep 5m
> [ perf record: Woken up 166 times to write data ]
> [ perf record: Captured and wrote 42.104 MB perf.data (~1839573 samples)
> ]
> [root@felicio linux]#
> [root@felicio linux]#
> [root@felicio linux]# perf report --gtk
>
> ?PID USER ? ? ?PR ?NI ?VIRT ?RES ?SHR S %CPU %MEM ? ?TIME+ ?COMMAND
> 10618 root ? ? ?20 ? 0 ?408m 170m ?49m S ?0.0 ?2.2 ? 0:04.71 perf
>
> [root@felicio linux]# perf report --tui
>
> 10633 root ? ? ?20 ? 0 ?379m 165m ?45m S ?0.0 ?2.1 ? 0:01.15 perf

Sorry, I don't understand how to interpret your numbers.

I used the same "perf record" command here which generated a 26 MB
perf.data file. "perf report --gtk" starts up almost instantly here.

Pekka

2012-02-23 17:37:53

by Arnaldo Carvalho de Melo

[permalink] [raw]
Subject: Re: [PATCH] perf report: Add a simple GTK2-based 'perf report' browser

Em Thu, Feb 23, 2012 at 07:10:48PM +0200, Pekka Enberg escreveu:
> On Thu, Feb 23, 2012 at 6:53 PM, Arnaldo Carvalho de Melo <[email protected]> wrote:
> > Em Thu, Feb 23, 2012 at 02:47:39PM -0200, Arnaldo Carvalho de Melo escreveu:
> >> Em Thu, Feb 23, 2012 at 06:45:04PM +0200, Pekka Enberg escreveu:
> >> > How big files are we talking about here?
> >>
> >> 5 MB ones, say.
> >
> > Nah:
> >
> > [root@felicio linux]# perf record -a -F 10000 sleep 5m
> > [ perf record: Woken up 166 times to write data ]
> > [ perf record: Captured and wrote 42.104 MB perf.data (~1839573 samples)
> > ]
> > [root@felicio linux]#
> > [root@felicio linux]#
> > [root@felicio linux]# perf report --gtk
> >
> > ?PID USER ? ? ?PR ?NI ?VIRT ?RES ?SHR S %CPU %MEM ? ?TIME+ ?COMMAND
> > 10618 root ? ? ?20 ? 0 ?408m 170m ?49m S ?0.0 ?2.2 ? 0:04.71 perf
> >
> > [root@felicio linux]# perf report --tui
> >
> > 10633 root ? ? ?20 ? 0 ?379m 165m ?45m S ?0.0 ?2.1 ? 0:01.15 perf
>
> Sorry, I don't understand how to interpret your numbers.

I was just gauging how much overhead --gtk had over --tui, for this
specific file VIRT was "just" 29 MB more, which for todays standards is
almost nothing :-)

So just for really, really big files this will make a difference.

> I used the same "perf record" command here which generated a 26 MB
> perf.data file. "perf report --gtk" starts up almost instantly here.

Yeah, seems to be OK for most cases, for the ones where it may get in
the way, we can use --tui or --stdio or revisit the providing a callback
for GTK to ask for just the lines it wants rendered.

One thing I saw was that it now defaults to --stdio and then doesn't
setup the pager, i.e. just build it with newt-devel, gtk2-devel
installed and don't specify --gtk to see what I mean.

- Arnaldo

2012-02-23 17:52:58

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH] perf report: Add a simple GTK2-based 'perf report' browser

On Thu, 23 Feb 2012, Arnaldo Carvalho de Melo wrote:
> One thing I saw was that it now defaults to --stdio and then doesn't
> setup the pager, i.e. just build it with newt-devel, gtk2-devel
> installed and don't specify --gtk to see what I mean.

Fixed! This version makes sure none of the defaults change and that GTK
interface is started up only if user explicitly asks for it.

Pekka

------------->

>From 3dc75243a43212fe907fad4139087f4033c2bf53 Mon Sep 17 00:00:00 2001
From: Pekka Enberg <[email protected]>
Date: Thu, 23 Feb 2012 17:49:18 +0200
Subject: [PATCH] perf report: Add a simple GTK2-based 'perf report' browser
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

This patch adds a simple GTK2-based browser to 'perf report' that's based on
the TTY-based browser in builtin-report.c.

Please not that you need to use

make WERROR=0

to build perf on Fedora 15 (and possibly other distributions) because GTK
headers do not compile cleanly:

CC util/gtk/browser.o
In file included from /usr/include/gtk-2.0/gtk/gtk.h:234:0,
from util/gtk/browser.c:7:
/usr/include/gtk-2.0/gtk/gtkitemfactory.h:47:1: warning: function declaration isn?t a prototype [-Wstrict-prototypes]

To launch "perf report" using the new GTK interface just type:

./perf report --gtk

The interface is somewhat limited in features at the moment:

- No callgraph support

- No KVM guest profiling support

- No color coding for percentages

- No sorting from the UI

- ..and many, many more!

That said, I think this patch a reasonable start to build future features on.

Cc: Peter Zijlstra <[email protected]>
Cc: Paul Mackerras <[email protected]>
Cc: Ingo Molnar <[email protected]>
Cc: Arnaldo Carvalho de Melo <[email protected]>
Signed-off-by: Pekka Enberg <[email protected]>
---
tools/perf/Documentation/perf-report.txt | 2 +
tools/perf/Makefile | 14 +++
tools/perf/builtin-report.c | 23 +++-
tools/perf/config/feature-tests.mak | 13 ++
tools/perf/util/cache.h | 12 ++
tools/perf/util/gtk/browser.c | 189 ++++++++++++++++++++++++++++++
tools/perf/util/hist.h | 17 +++
7 files changed, 264 insertions(+), 6 deletions(-)
create mode 100644 tools/perf/util/gtk/browser.c

diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
index 9b430e9..9654f27 100644
--- a/tools/perf/Documentation/perf-report.txt
+++ b/tools/perf/Documentation/perf-report.txt
@@ -110,6 +110,8 @@ OPTIONS
requires a tty, if one is not present, as when piping to other
commands, the stdio interface is used.

+--gtk:: Use the GTK2 interface.
+
-k::
--vmlinux=<file>::
vmlinux pathname
diff --git a/tools/perf/Makefile b/tools/perf/Makefile
index e011b50..371f114 100644
--- a/tools/perf/Makefile
+++ b/tools/perf/Makefile
@@ -485,6 +485,20 @@ else
endif
endif

+ifdef NO_GTK2
+ BASIC_CFLAGS += -DNO_GTK2
+else
+ FLAGS_GTK2=$(ALL_CFLAGS) $(ALL_LDFLAGS) $(EXTLIBS) $(shell pkg-config --libs --cflags gtk+-2.0)
+ ifneq ($(call try-cc,$(SOURCE_GTK2),$(FLAGS_GTK2)),y)
+ msg := $(warning GTK2 not found, disables GTK2 support. Please install gtk2-devel or libgtk2.0-dev);
+ BASIC_CFLAGS += -DNO_GTK2_SUPPORT
+ else
+ BASIC_CFLAGS += $(shell pkg-config --cflags gtk+-2.0)
+ EXTLIBS += $(shell pkg-config --libs gtk+-2.0)
+ LIB_OBJS += $(OUTPUT)util/gtk/browser.o
+ endif
+endif
+
ifdef NO_LIBPERL
BASIC_CFLAGS += -DNO_LIBPERL
else
diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
index 25d34d4..d6bf6ec 100644
--- a/tools/perf/builtin-report.c
+++ b/tools/perf/builtin-report.c
@@ -40,7 +40,7 @@ struct perf_report {
struct perf_tool tool;
struct perf_session *session;
char const *input_name;
- bool force, use_tui, use_stdio;
+ bool force, use_tui, use_gtk, use_stdio;
bool hide_unresolved;
bool dont_use_callchains;
bool show_full_info;
@@ -326,8 +326,13 @@ static int __cmd_report(struct perf_report *rep)
}

if (use_browser > 0) {
- perf_evlist__tui_browse_hists(session->evlist, help,
- NULL, NULL, 0);
+ if (use_browser == 1) {
+ perf_evlist__tui_browse_hists(session->evlist, help,
+ NULL, NULL, 0);
+ } else if (use_browser == 2) {
+ perf_evlist__gtk_browse_hists(session->evlist, help,
+ NULL, NULL, 0);
+ }
} else
perf_evlist__tty_browse_hists(session->evlist, rep, help);

@@ -474,6 +479,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __used)
OPT_STRING(0, "pretty", &report.pretty_printing_style, "key",
"pretty printing style key: normal raw"),
OPT_BOOLEAN(0, "tui", &report.use_tui, "Use the TUI interface"),
+ OPT_BOOLEAN(0, "gtk", &report.use_gtk, "Use the GTK2 interface"),
OPT_BOOLEAN(0, "stdio", &report.use_stdio,
"Use the stdio interface"),
OPT_STRING('s', "sort", &sort_order, "key[,key2...]",
@@ -526,6 +532,8 @@ int cmd_report(int argc, const char **argv, const char *prefix __used)
use_browser = 0;
else if (report.use_tui)
use_browser = 1;
+ else if (report.use_gtk)
+ use_browser = 2;

if (report.inverted_callchain)
callchain_param.order = ORDER_CALLER;
@@ -537,9 +545,12 @@ int cmd_report(int argc, const char **argv, const char *prefix __used)
report.input_name = "perf.data";
}

- if (strcmp(report.input_name, "-") != 0)
- setup_browser(true);
- else
+ if (strcmp(report.input_name, "-") != 0) {
+ if (report.use_gtk)
+ perf_gtk_setup_browser(argc, argv, true);
+ else
+ setup_browser(true);
+ } else
use_browser = 0;

/*
diff --git a/tools/perf/config/feature-tests.mak b/tools/perf/config/feature-tests.mak
index 6170fd2..f5b551c 100644
--- a/tools/perf/config/feature-tests.mak
+++ b/tools/perf/config/feature-tests.mak
@@ -65,6 +65,19 @@ int main(void)
endef
endif

+ifndef NO_GTK2
+define SOURCE_GTK2
+#include <gtk/gtk.h>
+
+int main(int argc, char *argv[])
+{
+ gtk_init(&argc, &argv);
+
+ return 0;
+}
+endef
+endif
+
ifndef NO_LIBPERL
define SOURCE_PERL_EMBED
#include <EXTERN.h>
diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
index fc5e5a0..8dd224d 100644
--- a/tools/perf/util/cache.h
+++ b/tools/perf/util/cache.h
@@ -45,6 +45,18 @@ void setup_browser(bool fallback_to_pager);
void exit_browser(bool wait_for_ok);
#endif

+#ifdef NO_GTK2_SUPPORT
+static inline void perf_gtk_setup_browser(int argc __used, const char *argv[] __used, bool fallback_to_pager)
+{
+ if (fallback_to_pager)
+ setup_pager();
+}
+static inline void perf_gtk_exit_browser(bool wait_for_ok __used) {}
+#else
+void perf_gtk_setup_browser(int argc, const char *argv[], bool fallback_to_pager);
+void perf_gtk_exit_browser(bool wait_for_ok);
+#endif
+
char *alias_lookup(const char *alias);
int split_cmdline(char *cmdline, const char ***argv);

diff --git a/tools/perf/util/gtk/browser.c b/tools/perf/util/gtk/browser.c
new file mode 100644
index 0000000..4878279
--- /dev/null
+++ b/tools/perf/util/gtk/browser.c
@@ -0,0 +1,189 @@
+#include "../evlist.h"
+#include "../cache.h"
+#include "../evsel.h"
+#include "../sort.h"
+#include "../hist.h"
+
+#include <gtk/gtk.h>
+#include <signal.h>
+
+#define MAX_COLUMNS 32
+
+void perf_gtk_setup_browser(int argc, const char *argv[],
+ bool fallback_to_pager __used)
+{
+ gtk_init(&argc, (char ***)&argv);
+}
+
+void perf_gtk_exit_browser(bool wait_for_ok __used)
+{
+ gtk_main_quit();
+}
+
+static void perf_gtk_signal(int sig)
+{
+ psignal(sig, "perf");
+ gtk_main_quit();
+}
+
+static void perf_gtk_resize_window(GtkWidget *window)
+{
+ GdkRectangle rect;
+ GdkScreen *screen;
+ int monitor;
+ int height;
+ int width;
+
+ screen = gtk_widget_get_screen(window);
+
+ monitor = gdk_screen_get_monitor_at_window(screen, window->window);
+
+ gdk_screen_get_monitor_geometry(screen, monitor, &rect);
+
+ width = rect.width * 3 / 4;
+ height = rect.height * 3 / 4;
+
+ gtk_window_resize(GTK_WINDOW(window), width, height);
+}
+
+static void perf_gtk_show_hists(GtkWidget *window, struct hists *hists)
+{
+ GType col_types[MAX_COLUMNS];
+ GtkCellRenderer *renderer;
+ struct sort_entry *se;
+ GtkListStore *store;
+ struct rb_node *nd;
+ u64 total_period;
+ GtkWidget *view;
+ int col_idx;
+ int nr_cols;
+
+ nr_cols = 0;
+
+ /* The percentage column */
+ col_types[nr_cols++] = G_TYPE_STRING;
+
+ list_for_each_entry(se, &hist_entry__sort_list, list) {
+ if (se->elide)
+ continue;
+
+ col_types[nr_cols++] = G_TYPE_STRING;
+ }
+
+ store = gtk_list_store_newv(nr_cols, col_types);
+
+ view = gtk_tree_view_new();
+
+ renderer = gtk_cell_renderer_text_new();
+
+ col_idx = 0;
+
+ /* The percentage column */
+ gtk_tree_view_insert_column_with_attributes(GTK_TREE_VIEW(view),
+ -1, "Overhead (%)",
+ renderer, "text",
+ col_idx++, NULL);
+
+ list_for_each_entry(se, &hist_entry__sort_list, list) {
+ if (se->elide)
+ continue;
+
+ gtk_tree_view_insert_column_with_attributes(GTK_TREE_VIEW(view),
+ -1, se->se_header,
+ renderer, "text",
+ col_idx++, NULL);
+ }
+
+ gtk_tree_view_set_model(GTK_TREE_VIEW(view), GTK_TREE_MODEL(store));
+
+ g_object_unref(GTK_TREE_MODEL(store));
+
+ total_period = hists->stats.total_period;
+
+ for (nd = rb_first(&hists->entries); nd; nd = rb_next(nd)) {
+ struct hist_entry *h = rb_entry(nd, struct hist_entry, rb_node);
+ GtkTreeIter iter;
+ double percent;
+ char s[512];
+
+ if (h->filtered)
+ continue;
+
+ gtk_list_store_append(store, &iter);
+
+ col_idx = 0;
+
+ percent = (h->period * 100.0) / total_period;
+
+ snprintf(s, ARRAY_SIZE(s), "%.2f", percent);
+
+ gtk_list_store_set(store, &iter, col_idx++, s, -1);
+
+ list_for_each_entry(se, &hist_entry__sort_list, list) {
+ if (se->elide)
+ continue;
+
+ se->se_snprintf(h, s, ARRAY_SIZE(s),
+ hists__col_len(hists, se->se_width_idx));
+
+ gtk_list_store_set(store, &iter, col_idx++, s, -1);
+ }
+ }
+
+ gtk_container_add(GTK_CONTAINER(window), view);
+}
+
+int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist,
+ const char *help __used,
+ void (*timer) (void *arg)__used,
+ void *arg __used, int delay_secs __used)
+{
+ struct perf_evsel *pos;
+ GtkWidget *notebook;
+ GtkWidget *window;
+
+ signal(SIGSEGV, perf_gtk_signal);
+ signal(SIGFPE, perf_gtk_signal);
+ signal(SIGINT, perf_gtk_signal);
+ signal(SIGQUIT, perf_gtk_signal);
+ signal(SIGTERM, perf_gtk_signal);
+
+ window = gtk_window_new(GTK_WINDOW_TOPLEVEL);
+
+ gtk_window_set_title(GTK_WINDOW(window), "perf report");
+
+ g_signal_connect(window, "delete_event", gtk_main_quit, NULL);
+
+ notebook = gtk_notebook_new();
+
+ list_for_each_entry(pos, &evlist->entries, node) {
+ struct hists *hists = &pos->hists;
+ const char *evname = event_name(pos);
+ GtkWidget *scrolled_window;
+ GtkWidget *tab_label;
+
+ scrolled_window = gtk_scrolled_window_new(NULL, NULL);
+
+ gtk_scrolled_window_set_policy(GTK_SCROLLED_WINDOW(scrolled_window),
+ GTK_POLICY_AUTOMATIC,
+ GTK_POLICY_AUTOMATIC);
+
+ perf_gtk_show_hists(scrolled_window, hists);
+
+ tab_label = gtk_label_new(evname);
+
+ gtk_notebook_append_page(GTK_NOTEBOOK(notebook), scrolled_window, tab_label);
+ }
+
+ gtk_container_add(GTK_CONTAINER(window), notebook);
+
+ gtk_widget_show_all(window);
+
+ perf_gtk_resize_window(window);
+
+ gtk_window_set_position(GTK_WINDOW(window), GTK_WIN_POS_CENTER);
+
+ gtk_main();
+
+ return 0;
+}
diff --git a/tools/perf/util/hist.h b/tools/perf/util/hist.h
index 48e5acd..3be98b2 100644
--- a/tools/perf/util/hist.h
+++ b/tools/perf/util/hist.h
@@ -134,6 +134,23 @@ int perf_evlist__tui_browse_hists(struct perf_evlist *evlist, const char *help,
int refresh);
#endif

+#ifdef NO_GTK2_SUPPORT
+static inline
+int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist __used,
+ const char *help __used,
+ void(*timer)(void *arg) __used,
+ void *arg __used,
+ int refresh __used)
+{
+ return 0;
+}
+
+#else
+int perf_evlist__gtk_browse_hists(struct perf_evlist *evlist, const char *help,
+ void(*timer)(void *arg), void *arg,
+ int refresh);
+#endif
+
unsigned int hists__sort_list_width(struct hists *self);

#endif /* __PERF_HIST_H */
--
1.7.6.5

2012-02-23 19:37:12

by Colin Walters

[permalink] [raw]
Subject: Re: [PATCH] perf report: Add a simple GTK2-based 'perf report' browser

On Thu, 2012-02-23 at 18:18 +0200, Pekka Enberg wrote:
> This patch adds a simple GTK2-based browser to 'perf report' that's based on
> the TTY-based browser in builtin-report.c.
>
> Please not that you need to use
>
> make WERROR=0

You could -Wnoerror=strict-prototypes...

>
> to build perf on Fedora 15 (and possibly other distributions) because GTK
> headers do not compile cleanly:
>
> CC util/gtk/browser.o
> In file included from /usr/include/gtk-2.0/gtk/gtk.h:234:0,
> from util/gtk/browser.c:7:
> /usr/include/gtk-2.0/gtk/gtkitemfactory.h:47:1: warning: function declaration isn’t a prototype [-Wstrict-prototypes]

Unfortunately it was a historical mistake, but it's part of the public
API:

https://bugzilla.gnome.org/show_bug.cgi?id=508760

2012-02-23 20:18:08

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH] perf report: Add a simple GTK2-based 'perf report' browser

On Thu, 2012-02-23 at 18:18 +0200, Pekka Enberg wrote:
>> This patch adds a simple GTK2-based browser to 'perf report' that's based on
>> the TTY-based browser in builtin-report.c.
>>
>> Please not that you need to use
>>
>> ? make WERROR=0

On Thu, Feb 23, 2012 at 9:36 PM, Colin Walters <[email protected]> wrote:
> You could -Wnoerror=strict-prototypes...

Sure. We don't want to do that for all files. Just for the ones that
include <gtk/gtk.h>.

2012-02-23 20:34:27

by Colin Walters

[permalink] [raw]
Subject: Re: [PATCH] perf report: Add a simple GTK2-based 'perf report' browser

On Thu, 2012-02-23 at 22:18 +0200, Pekka Enberg wrote:
> On Thu, 2012-02-23 at 18:18 +0200, Pekka Enberg wrote:
> >> This patch adds a simple GTK2-based browser to 'perf report' that's based on
> >> the TTY-based browser in builtin-report.c.
> >>
> >> Please not that you need to use
> >>
> >> make WERROR=0
>
> On Thu, Feb 23, 2012 at 9:36 PM, Colin Walters <[email protected]> wrote:
> > You could -Wnoerror=strict-prototypes...
>
> Sure. We don't want to do that for all files. Just for the ones that
> include <gtk/gtk.h>.

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wstrict-prototypes"
#include <gtk/gtk.h>
#pragma GCC diagnostic pop

2012-02-23 21:27:46

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH] perf report: Add a simple GTK2-based 'perf report' browser

On Thu, Feb 23, 2012 at 10:33 PM, Colin Walters <[email protected]> wrote:
>> Sure. We don't want to do that for all files. Just for the ones that
>> include <gtk/gtk.h>.
>
> #pragma GCC diagnostic push
> #pragma GCC diagnostic ignored "-Wstrict-prototypes"
> #include <gtk/gtk.h>
> #pragma GCC diagnostic pop

It's cleaner to do it at Makefile level. We should do something like
sparse.git Makefile does where you can optionally specify CFLAGS for
individual source files.

Pekka

2012-02-24 00:58:10

by Namhyung Kim

[permalink] [raw]
Subject: Re: [PATCH] perf report: Add a simple GTK2-based 'perf report' browser

Hi,

2012-02-24 2:52 AM, Pekka Enberg wrote:
> From 3dc75243a43212fe907fad4139087f4033c2bf53 Mon Sep 17 00:00:00 2001
> From: Pekka Enberg<[email protected]>
> Date: Thu, 23 Feb 2012 17:49:18 +0200
> Subject: [PATCH] perf report: Add a simple GTK2-based 'perf report' browser
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> This patch adds a simple GTK2-based browser to 'perf report' that's based on
> the TTY-based browser in builtin-report.c.
>
> Please not that you need to use
>
> make WERROR=0
>
> to build perf on Fedora 15 (and possibly other distributions) because GTK
> headers do not compile cleanly:
>
> CC util/gtk/browser.o
> In file included from /usr/include/gtk-2.0/gtk/gtk.h:234:0,
> from util/gtk/browser.c:7:
> /usr/include/gtk-2.0/gtk/gtkitemfactory.h:47:1: warning: function declaration isn’t a prototype [-Wstrict-prototypes]
>
> To launch "perf report" using the new GTK interface just type:
>
> ./perf report --gtk
>
> The interface is somewhat limited in features at the moment:
>
> - No callgraph support
>
> - No KVM guest profiling support
>
> - No color coding for percentages
>
> - No sorting from the UI
>
> - ..and many, many more!
>
> That said, I think this patch a reasonable start to build future features on.
>

Looks like a nice patch! Few minor nits below.


> Cc: Peter Zijlstra<[email protected]>
> Cc: Paul Mackerras<[email protected]>
> Cc: Ingo Molnar<[email protected]>
> Cc: Arnaldo Carvalho de Melo<[email protected]>
> Signed-off-by: Pekka Enberg<[email protected]>
> ---
> tools/perf/Documentation/perf-report.txt | 2 +
> tools/perf/Makefile | 14 +++
> tools/perf/builtin-report.c | 23 +++-
> tools/perf/config/feature-tests.mak | 13 ++
> tools/perf/util/cache.h | 12 ++
> tools/perf/util/gtk/browser.c | 189 ++++++++++++++++++++++++++++++
> tools/perf/util/hist.h | 17 +++
> 7 files changed, 264 insertions(+), 6 deletions(-)
> create mode 100644 tools/perf/util/gtk/browser.c
>
> diff --git a/tools/perf/Documentation/perf-report.txt b/tools/perf/Documentation/perf-report.txt
> index 9b430e9..9654f27 100644
> --- a/tools/perf/Documentation/perf-report.txt
> +++ b/tools/perf/Documentation/perf-report.txt
> @@ -110,6 +110,8 @@ OPTIONS
> requires a tty, if one is not present, as when piping to other
> commands, the stdio interface is used.
>
> +--gtk:: Use the GTK2 interface.
> +
> -k::
> --vmlinux=<file>::
> vmlinux pathname
> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
> index e011b50..371f114 100644
> --- a/tools/perf/Makefile
> +++ b/tools/perf/Makefile
> @@ -485,6 +485,20 @@ else
> endif
> endif
>
> +ifdef NO_GTK2
> + BASIC_CFLAGS += -DNO_GTK2

s/-DNO_GTK2/-DNO_GTK2_SUPPORT/ ?


> +else
> + FLAGS_GTK2=$(ALL_CFLAGS) $(ALL_LDFLAGS) $(EXTLIBS) $(shell pkg-config --libs --cflags gtk+-2.0)
> + ifneq ($(call try-cc,$(SOURCE_GTK2),$(FLAGS_GTK2)),y)
> + msg := $(warning GTK2 not found, disables GTK2 support. Please install gtk2-devel or libgtk2.0-dev);
> + BASIC_CFLAGS += -DNO_GTK2_SUPPORT
> + else
> + BASIC_CFLAGS += $(shell pkg-config --cflags gtk+-2.0)
> + EXTLIBS += $(shell pkg-config --libs gtk+-2.0)
> + LIB_OBJS += $(OUTPUT)util/gtk/browser.o
> + endif
> +endif
> +
> ifdef NO_LIBPERL
> BASIC_CFLAGS += -DNO_LIBPERL
> else
> diff --git a/tools/perf/builtin-report.c b/tools/perf/builtin-report.c
> index 25d34d4..d6bf6ec 100644
> --- a/tools/perf/builtin-report.c
> +++ b/tools/perf/builtin-report.c
> @@ -40,7 +40,7 @@ struct perf_report {
> struct perf_tool tool;
> struct perf_session *session;
> char const *input_name;
> - bool force, use_tui, use_stdio;
> + bool force, use_tui, use_gtk, use_stdio;
> bool hide_unresolved;
> bool dont_use_callchains;
> bool show_full_info;
> @@ -326,8 +326,13 @@ static int __cmd_report(struct perf_report *rep)
> }
>
> if (use_browser> 0) {
> - perf_evlist__tui_browse_hists(session->evlist, help,
> - NULL, NULL, 0);
> + if (use_browser == 1) {
> + perf_evlist__tui_browse_hists(session->evlist, help,
> + NULL, NULL, 0);
> + } else if (use_browser == 2) {
> + perf_evlist__gtk_browse_hists(session->evlist, help,
> + NULL, NULL, 0);
> + }
> } else
> perf_evlist__tty_browse_hists(session->evlist, rep, help);
>
> @@ -474,6 +479,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __used)
> OPT_STRING(0, "pretty",&report.pretty_printing_style, "key",
> "pretty printing style key: normal raw"),
> OPT_BOOLEAN(0, "tui",&report.use_tui, "Use the TUI interface"),
> + OPT_BOOLEAN(0, "gtk",&report.use_gtk, "Use the GTK2 interface"),
> OPT_BOOLEAN(0, "stdio",&report.use_stdio,
> "Use the stdio interface"),
> OPT_STRING('s', "sort",&sort_order, "key[,key2...]",
> @@ -526,6 +532,8 @@ int cmd_report(int argc, const char **argv, const char *prefix __used)
> use_browser = 0;
> else if (report.use_tui)
> use_browser = 1;
> + else if (report.use_gtk)
> + use_browser = 2;
>
> if (report.inverted_callchain)
> callchain_param.order = ORDER_CALLER;
> @@ -537,9 +545,12 @@ int cmd_report(int argc, const char **argv, const char *prefix __used)
> report.input_name = "perf.data";
> }
>
> - if (strcmp(report.input_name, "-") != 0)
> - setup_browser(true);
> - else
> + if (strcmp(report.input_name, "-") != 0) {
> + if (report.use_gtk)
> + perf_gtk_setup_browser(argc, argv, true);
> + else
> + setup_browser(true);
> + } else

Wouldn't it be better if setup_browser() handled this internally based on the
value of use_browser?


> use_browser = 0;
>
> /*
> diff --git a/tools/perf/config/feature-tests.mak b/tools/perf/config/feature-tests.mak
> index 6170fd2..f5b551c 100644
> --- a/tools/perf/config/feature-tests.mak
> +++ b/tools/perf/config/feature-tests.mak
> @@ -65,6 +65,19 @@ int main(void)
> endef
> endif
>
> +ifndef NO_GTK2
> +define SOURCE_GTK2
> +#include<gtk/gtk.h>
> +
> +int main(int argc, char *argv[])
> +{
> + gtk_init(&argc,&argv);
> +
> + return 0;
> +}
> +endef
> +endif
> +
> ifndef NO_LIBPERL
> define SOURCE_PERL_EMBED
> #include<EXTERN.h>
> diff --git a/tools/perf/util/cache.h b/tools/perf/util/cache.h
> index fc5e5a0..8dd224d 100644
> --- a/tools/perf/util/cache.h
> +++ b/tools/perf/util/cache.h
> @@ -45,6 +45,18 @@ void setup_browser(bool fallback_to_pager);
> void exit_browser(bool wait_for_ok);
> #endif
>
> +#ifdef NO_GTK2_SUPPORT
> +static inline void perf_gtk_setup_browser(int argc __used, const char *argv[] __used, bool fallback_to_pager)
> +{
> + if (fallback_to_pager)
> + setup_pager();
> +}
> +static inline void perf_gtk_exit_browser(bool wait_for_ok __used) {}
> +#else
> +void perf_gtk_setup_browser(int argc, const char *argv[], bool fallback_to_pager);
> +void perf_gtk_exit_browser(bool wait_for_ok);
> +#endif
> +
> char *alias_lookup(const char *alias);
> int split_cmdline(char *cmdline, const char ***argv);
>
> diff --git a/tools/perf/util/gtk/browser.c b/tools/perf/util/gtk/browser.c
> new file mode 100644
> index 0000000..4878279
> --- /dev/null
> +++ b/tools/perf/util/gtk/browser.c

I think it needs to be moved under util/ui. There will be some share point
between tui and gui IMHO. Otherwise, if it is too much depends on TUI, how
about rename util/ui to util/tui?

Thanks,
Namhyung

2012-02-24 06:37:00

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH] perf report: Add a simple GTK2-based 'perf report' browser

On Fri, Feb 24, 2012 at 2:58 AM, Namhyung Kim <[email protected]> wrote:
>> diff --git a/tools/perf/Makefile b/tools/perf/Makefile
>> index e011b50..371f114 100644
>> --- a/tools/perf/Makefile
>> +++ b/tools/perf/Makefile
>> @@ -485,6 +485,20 @@ else
>> ? ? ? ?endif
>> ?endif
>>
>> +ifdef NO_GTK2
>> + ? ? ? BASIC_CFLAGS += -DNO_GTK2
>
>
> s/-DNO_GTK2/-DNO_GTK2_SUPPORT/ ?

Fixed.

>> + ? ? ? if (strcmp(report.input_name, "-") != 0) {
>> + ? ? ? ? ? ? ? if (report.use_gtk)
>> + ? ? ? ? ? ? ? ? ? ? ? perf_gtk_setup_browser(argc, argv, true);
>> + ? ? ? ? ? ? ? else
>> + ? ? ? ? ? ? ? ? ? ? ? setup_browser(true);
>> + ? ? ? } else
>
>
> Wouldn't it be better if setup_browser() handled this internally based on
> the value of use_browser?

Well yes, but that requires some more code shuffling so I'd like to
leave it like that for now.

>> diff --git a/tools/perf/util/gtk/browser.c b/tools/perf/util/gtk/browser.c
>> new file mode 100644
>> index 0000000..4878279
>> --- /dev/null
>> +++ b/tools/perf/util/gtk/browser.c
>
>
> I think it needs to be moved under util/ui. There will be some share point
> between tui and gui IMHO. Otherwise, if it is too much depends on TUI, how
> about rename util/ui to util/tui?

What's with you perf guys and deeply nested directory hierarchies? :-)

I think we should keep tools/ui but move the TUI specific bits out of
there. But as I said earlier, the code needs some work to support GTK
and TUI cleanly.

Pekka

2012-02-24 09:48:22

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] perf report: Add a simple GTK2-based 'perf report' browser


* Pekka Enberg <[email protected]> wrote:

> On Thu, Feb 23, 2012 at 10:33 PM, Colin Walters <[email protected]> wrote:
> >> Sure. We don't want to do that for all files. Just for the ones that
> >> include <gtk/gtk.h>.
> >
> > #pragma GCC diagnostic push
> > #pragma GCC diagnostic ignored "-Wstrict-prototypes"
> > #include <gtk/gtk.h>
> > #pragma GCC diagnostic pop
>
> It's cleaner to do it at Makefile level. We should do
> something like sparse.git Makefile does where you can
> optionally specify CFLAGS for individual source files.

I actually like the #pragma hack because it only turns off the
check for that broken header and keeps our checks in place for
the rest of the .c file.

Could be turned into a util/gtk.h file that is included instead
of <gtk/gtk.h>, so that we don't have to see the #pragma
workaround all the time?

Thanks,

Ingo

2012-02-24 10:05:58

by Pekka Enberg

[permalink] [raw]
Subject: Re: [PATCH] perf report: Add a simple GTK2-based 'perf report' browser

On Fri, Feb 24, 2012 at 11:48 AM, Ingo Molnar <[email protected]> wrote:
> * Pekka Enberg <[email protected]> wrote:
>
>> On Thu, Feb 23, 2012 at 10:33 PM, Colin Walters <[email protected]> wrote:
>> >> Sure. We don't want to do that for all files. Just for the ones that
>> >> include <gtk/gtk.h>.
>> >
>> > #pragma GCC diagnostic push
>> > #pragma GCC diagnostic ignored "-Wstrict-prototypes"
>> > #include <gtk/gtk.h>
>> > #pragma GCC diagnostic pop
>>
>> It's cleaner to do it at Makefile level. We should do
>> something like sparse.git Makefile does where you can
>> optionally specify CFLAGS for individual source files.
>
> I actually like the #pragma hack because it only turns off the
> check for that broken header and keeps our checks in place for
> the rest of the .c file.
>
> Could be turned into a util/gtk.h file that is included instead
> of <gtk/gtk.h>, so that we don't have to see the #pragma
> workaround all the time?

Sure, makes sense.