Skip to contentSkip to conversation

perf report: Add a simple GTK2-based 'perf report' browser

PATCH In review Pekka Enberg 5 participants 22 messages Feb 23, 2012 – Mar 27, 2012 perf events

lore
Share & tools
lore
#0 lore raw

PATCH 1/1 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]>

---
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
tools/perf/Documentation/perf-report.txt +2 -0
index 9b430e9..9654f27 100644
@@ -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
tools/perf/Makefile +14 -0
index e011b50..371f114 100644
@@ -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
tools/perf/builtin-report.c +18 -6
index 25d34d4..e0a5d17 100644
@@ -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;
 
 	/*
tools/perf/config/feature-tests.mak +13 -0
index 6170fd2..f5b551c 100644
@@ -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>
tools/perf/util/cache.h +12 -0
index fc5e5a0..8dd224d 100644
@@ -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);
 
tools/perf/util/gtk/browser.c new file +189 -0

189 lines not on this page · raw message

tools/perf/util/hist.h +17 -0
index 48e5acd..3be98b2 100644
@@ -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 */
Signature · 1 line
1.7.6.5
#1 lore raw

Em Thu, Feb 23, 2012 at 06:18:42PM +0200, Pekka Enberg escreveu:

Quoted 32 lines

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>

Quoted 19 lines
tools/perf/util/gtk/browser.c new file +19 -0
+	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);
+

tools/perf/util/gtk/browser.c line 131

Quoted 10 lines
tools/perf/util/gtk/browser.c new file +10 -0
+		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

#2 lore raw
Quoted 70 lines · Arnaldo Carvalho de Melo

Em Thu, Feb 23, 2012 at 06:18:42PM +0200, Pekka Enberg escreveu:

Quoted 32 lines

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>

Quoted 19 lines

+     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];

tools/perf/util/gtk/browser.c new file +14 -0
+
+             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);
+

tools/perf/util/gtk/browser.c line 131

Quoted 10 lines
tools/perf/util/gtk/browser.c new file +10 -0
+             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.

Quoted 3 lines

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

#3 lore raw

Em Thu, Feb 23, 2012 at 06:32:41PM +0200, Pekka Enberg escreveu:

Quoted 10 lines · Arnaldo Carvalho de Melo
Quoted 7 lines

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

#4 lore raw
Quoted 3 lines · Arnaldo Carvalho de Melo
Quoted 1 line

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
Quoted 1 line · Arnaldo Carvalho de Melo

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

How big files are we talking about here?

Pekka

#5 lore raw

Em Thu, Feb 23, 2012 at 06:45:04PM +0200, Pekka Enberg escreveu:

Quoted 8 lines · Arnaldo Carvalho de Melo
Quoted 3 lines
Quoted 1 line

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:

Quoted 7 lines
penberg@jaguar:~/src/jato$ ls -lh perf.data 
-rw------- 1 penberg penberg 453K 2012-02-23 18:06 perf.data
Quoted 1 line · Arnaldo Carvalho de Melo

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

#6 lore raw

Em Thu, Feb 23, 2012 at 02:47:39PM -0200, Arnaldo Carvalho de Melo escreveu:

Quoted 12 lines

Em Thu, Feb 23, 2012 at 06:45:04PM +0200, Pekka Enberg escreveu:

Quoted 11 lines · Arnaldo Carvalho de Melo
Quoted 3 lines
Quoted 1 line

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
Quoted 6 lines
Quoted 4 lines · Arnaldo Carvalho de Melo
Quoted 1 line

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

Quoted 3 lines

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

#7 lore raw
Quoted 30 lines · Arnaldo Carvalho de Melo

Em Thu, Feb 23, 2012 at 02:47:39PM -0200, Arnaldo Carvalho de Melo escreveu:

Quoted 12 lines

Em Thu, Feb 23, 2012 at 06:45:04PM +0200, Pekka Enberg escreveu:

Quoted 11 lines · Arnaldo Carvalho de Melo
Quoted 3 lines
Quoted 1 line

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
Quoted 6 lines
Quoted 4 lines · Arnaldo Carvalho de Melo
Quoted 1 line

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

Quoted 6 lines

 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

#8 lore raw

Em Thu, Feb 23, 2012 at 07:10:48PM +0200, Pekka Enberg escreveu:

Quoted 17 lines · Arnaldo Carvalho de Melo
Quoted 16 lines

Em Thu, Feb 23, 2012 at 02:47:39PM -0200, Arnaldo Carvalho de Melo escreveu:

Quoted 4 lines

Em Thu, Feb 23, 2012 at 06:45:04PM +0200, Pekka Enberg escreveu:

Quoted 1 line

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

Quoted 8 lines
Quoted 6 lines

 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.

Quoted 2 lines

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

#9 lore raw
Quoted 3 lines · Arnaldo Carvalho de Melo

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

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

Quoted 1 line

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]>
---
 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
tools/perf/Documentation/perf-report.txt +2 -0
index 9b430e9..9654f27 100644
@@ -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
tools/perf/Makefile +14 -0
index e011b50..371f114 100644
@@ -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
tools/perf/builtin-report.c +17 -6
index 25d34d4..d6bf6ec 100644
@@ -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;
 
 	/*
tools/perf/config/feature-tests.mak +13 -0
index 6170fd2..f5b551c 100644
@@ -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>
tools/perf/util/cache.h +12 -0
index fc5e5a0..8dd224d 100644
@@ -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);
 
tools/perf/util/gtk/browser.c new file +189 -0

189 lines not on this page · raw message

tools/perf/util/hist.h +17 -0
index 48e5acd..3be98b2 100644
@@ -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 */
Signature · 1 line
1.7.6.5
#10 lore raw

Hi,

Quoted 33 lines · Pekka Enberg

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

Quoted 8 lines

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

Quoted 34 lines
Cc Peter Zijlstra<[email protected]>
Cc Paul Mackerras<[email protected]>
Cc Ingo Molnar<[email protected]>
Cc Arnaldo Carvalho de Melo<[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

tools/perf/Documentation/perf-report.txt +2 -0
index 9b430e9..9654f27 100644
@@ -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
tools/perf/Makefile +0 -0
index e011b50..371f114 100644

tools/perf/Makefile else · line 489

Quoted 5 lines
tools/perf/Makefile +2 -0
@@ -485,6 +485,20 @@ else
  	endif
  endif
 
+ifdef NO_GTK2
+	BASIC_CFLAGS += -DNO_GTK2

s/-DNO_GTK2/-DNO_GTK2_SUPPORT/ ?

Quoted 64 lines
tools/perf/Makefile +12 -0
+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
tools/perf/builtin-report.c +11 -3
index 25d34d4..d6bf6ec 100644
@@ -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";
  	}

tools/perf/builtin-report.c int cmd_report(int argc, const char **argv, const char *prefix __used) · line 553

Quoted 10 lines
tools/perf/builtin-report.c +6 -3
 
-	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?

tools/perf/util/gtk/browser.c

Quoted 55 lines
tools/perf/builtin-report.c +0 -0
  		use_browser = 0;
 
  	/*
tools/perf/config/feature-tests.mak +13 -0
index 6170fd2..f5b551c 100644
@@ -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>
tools/perf/util/cache.h +12 -0
index fc5e5a0..8dd224d 100644
@@ -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);
 
tools/perf/util/gtk/browser.c new file +0 -0
new file mode 100644
index 0000000..4878279

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

#11 lore raw
Quoted 8 lines · Namhyung Kim
Quoted 8 lines
tools/perf/Makefile +0 -0
index e011b50..371f114 100644
@@ -485,6 +485,20 @@ else

       endif
 endif

Quoted 5 lines
Quoted 2 lines

+ifdef NO_GTK2
+       BASIC_CFLAGS += -DNO_GTK2

s/-DNO_GTK2/-DNO_GTK2_SUPPORT/ ?

Fixed.

Quoted 10 lines
Quoted 6 lines

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

Quoted 10 lines

tools/perf/util/gtk/browser.c

Quoted 5 lines
tools/perf/util/gtk/browser.c new file +0 -0
new file mode 100644
index 0000000..4878279

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

#12 lore raw
In reply to Pekka Enberg
Commit-ID:  c31a94570552dcaa517c4f7a043ffd28835016be
Gitweb:     http://git.kernel.org/tip/c31a94570552dcaa517c4f7a043ffd28835016be
Author:     Pekka Enberg <[email protected]>
AuthorDate: Mon, 19 Mar 2012 15:13:29 -0300
Committer:  Arnaldo Carvalho de Melo <[email protected]>
CommitDate: Mon, 19 Mar 2012 15:13:29 -0300

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.

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.

[ committer note: Added #pragma to make gtk no strict prototype problem go
away as suggested by Colin Walters modulo avoiding push/pop ]

---
 tools/perf/Documentation/perf-report.txt |    2 +
 tools/perf/Makefile                      |   14 +++
 tools/perf/builtin-report.c              |   19 +++-
 tools/perf/config/feature-tests.mak      |   15 +++
 tools/perf/util/cache.h                  |   12 ++
 tools/perf/util/gtk/browser.c            |  189 ++++++++++++++++++++++++++++++
 tools/perf/util/gtk/gtk.h                |    8 ++
 tools/perf/util/hist.h                   |   17 +++
 8 files changed, 272 insertions(+), 4 deletions(-)
tools/perf/Documentation/perf-report.txt +2 -0
index 68bbf9c..2d89f02 100644
@@ -113,6 +113,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
tools/perf/Makefile +14 -0
index e07fbd8..cc27148 100644
@@ -507,6 +507,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
tools/perf/builtin-report.c +15 -4
index c005458..2e31743 100644
@@ -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;
@@ -415,8 +415,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);
 
@@ -573,6 +578,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...]",
@@ -630,6 +636,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;
@@ -666,7 +674,10 @@ int cmd_report(int argc, const char **argv, const char *prefix __used)
 	}
 
 	if (strcmp(report.input_name, "-") != 0) {
-		setup_browser(true);
+		if (report.use_gtk)
+			perf_gtk_setup_browser(argc, argv, true);
+		else
+			setup_browser(true);
 	} else {
 		use_browser = 0;
 	}
tools/perf/config/feature-tests.mak +15 -0
index 6170fd2..d9084e0 100644
@@ -65,6 +65,21 @@ int main(void)
 endef
 endif
 
+ifndef NO_GTK2
+define SOURCE_GTK2
+#pragma GCC diagnostic ignored \"-Wstrict-prototypes\"
+#include <gtk/gtk.h>
+#pragma GCC diagnostic error \"-Wstrict-prototypes\"
+
+int main(int argc, char *argv[])
+{
+        gtk_init(&argc, &argv);
+
+        return 0;
+}
+endef
+endif
+
 ifndef NO_LIBPERL
 define SOURCE_PERL_EMBED
 #include <EXTERN.h>
tools/perf/util/cache.h +12 -0
index fc5e5a0..8dd224d 100644
@@ -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);
 
tools/perf/util/gtk/browser.c new file +189 -0

189 lines not on this page · raw message

tools/perf/util/gtk/gtk.h new file +8 -0
new file mode 100644
index 0000000..75177ee
@@ -0,0 +1,8 @@
+#ifndef _PERF_GTK_H_
+#define _PERF_GTK_H_ 1
+
+#pragma GCC diagnostic ignored "-Wstrict-prototypes"
+#include <gtk/gtk.h>
+#pragma GCC diagnostic error "-Wstrict-prototypes"
+
+#endif /* _PERF_GTK_H_ */
tools/perf/util/hist.h +17 -0
index 10343c0..2cae9df 100644
@@ -147,6 +147,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 */
#13 lore raw
In reply to Pekka Enberg
Quoted 6 lines · Pekka Enberg

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

Quoted 8 lines

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

#14 lore raw
Quoted 6 lines · Pekka Enberg
Quoted 6 lines

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

Quoted 1 line · Colin Walters

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

#15 lore raw
Quoted 6 lines · Pekka Enberg
Quoted 5 lines · Pekka Enberg
Quoted 5 lines

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

Quoted 7 lines
Quoted 1 line
Quoted 1 line

make WERROR=0

Quoted 1 line · Colin Walters

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

#16 lore raw
Quoted 7 lines · Colin Walters
Quoted 2 lines

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

#17 lore raw
Quoted 4 lines · Pekka Enberg
Quoted 3 lines · Colin Walters
Quoted 2 lines

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

Quoted 8 lines
Quoted 4 lines

#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

#18 lore raw
Quoted 15 lines · Ingo Molnar
Quoted 4 lines · Pekka Enberg
Quoted 3 lines · Colin Walters
Quoted 2 lines

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

Quoted 8 lines
Quoted 4 lines

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

Quoted 7 lines

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.

#19 lore raw

Em Fri, Feb 24, 2012 at 12:05:56PM +0200, Pekka Enberg escreveu:

Quoted 20 lines · Ingo Molnar
Quoted 19 lines · Pekka Enberg
Quoted 13 lines · Colin Walters
Quoted 7 lines
Quoted 2 lines

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.

Quoted 5 lines
Quoted 3 lines

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.

Hey,

Was there any new development here? I was going to apply, but
requiring that WERROR=0 is too ugly :-)

- Arnaldo

#20 lore raw
In reply to Pekka Enberg

Em Fri, Feb 24, 2012 at 12:05:56PM +0200, Pekka Enberg escreveu:

Quoted 20 lines · Ingo Molnar
Quoted 19 lines · Pekka Enberg
Quoted 13 lines · Colin Walters
Quoted 7 lines
Quoted 2 lines

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.

Quoted 5 lines
Quoted 3 lines

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.

Using just:

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

Since push/pop was introduced in gcc 4.6, and here at the test machine using RHEL6.2 I have gcc 4.4.6.

- Arnaldo

#21 lore raw
Quoted 27 lines · Arnaldo Carvalho de Melo

Em Fri, Feb 24, 2012 at 12:05:56PM +0200, Pekka Enberg escreveu:

Quoted 20 lines · Ingo Molnar
Quoted 19 lines · Pekka Enberg
Quoted 13 lines · Colin Walters
Quoted 7 lines
Quoted 2 lines

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.

Quoted 5 lines
Quoted 3 lines

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.

Quoted 8 lines

Using just:

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

Since push/pop was introduced in gcc 4.6, and here at the test machine using RHEL6.2 I have gcc 4.4.6.

Thanks for taking care of this, Arnaldo and Ingo!