perf report: Add a simple GTK2-based 'perf report' browser
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.
---
I've uploaded a screenshot of the new UI here:
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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
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,
fromutil/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 --gtkThe 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
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,
fromutil/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)) {
+ structhist_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
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
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
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.dataQuoted 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
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
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.dataQuoted 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
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
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
3dc75243a43212fe907fad4139087f4033c2bf53Mon 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.
--- 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
Hi,
Quoted 33 lines · Pekka Enberg
From
3dc75243a43212fe907fad4139087f4033c2bf53Mon 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: 8bitThis 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,
fromutil/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 --gtkThe 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]>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.ctools/perf/Documentation/perf-report.txt +2
-0@@ -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 pathnametools/perf/Makefile +0
-0
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 elsetools/perf/builtin-report.c +11
-3@@ -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
-0use_browser =0 ;/* tools/perf/config/feature-tests.mak +13
-0@@ -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@@ -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
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
Quoted 8 lines · Namhyung Kim
Quoted 8 lines
tools/perf/Makefile +0
-0@@ -485,6 +485,20 @@ elseendif
endif
Quoted 5 lines
Quoted 2 lines
+ifdef
NO_GTK2
+BASIC_CFLAGS+= -DNO_GTK2s/-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);
+ } elseWouldn't it be better if
setup_browser()handled this internally based on the value ofuse_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
-0I 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
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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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 */
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,
fromutil/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:
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>.
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
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
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 popIt's cleaner to do it at
Makefilelevel. We should do something like sparse.gitMakefiledoes 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
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 popIt's cleaner to do it at
Makefilelevel. We should do something like sparse.gitMakefiledoes 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.hfile 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.
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 popIt's cleaner to do it at
Makefilelevel. We should do something like sparse.gitMakefiledoes 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.hfile 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
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 popIt's cleaner to do it at
Makefilelevel. We should do something like sparse.gitMakefiledoes 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.hfile 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
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 popIt's cleaner to do it at
Makefilelevel. We should do something like sparse.gitMakefiledoes 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.hfile 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!