After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 689810 - Include guard optimization
Include guard optimization
Status: RESOLVED FIXED
Product: atk
Classification: Platform
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: ATK maintainer(s)
ATK maintainer(s)
Depends on:
Blocks:
 
 
Reported: 2012-12-06 21:04 UTC by Peter De Wachter
Modified: 2018-05-16 12:51 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
header edit script (552 bytes, application/x-perl)
2012-12-06 21:04 UTC, Peter De Wachter
  Details
GTK+ 3 patch (152.50 KB, patch)
2012-12-06 21:07 UTC, Peter De Wachter
committed Details | Review
GTK+ 2 patch (121.60 KB, patch)
2012-12-06 21:08 UTC, Peter De Wachter
committed Details | Review
GLib patch (106.88 KB, patch)
2012-12-06 21:08 UTC, Peter De Wachter
committed Details | Review
GDK-pixbuf patch (3.70 KB, patch)
2012-12-06 21:08 UTC, Peter De Wachter
committed Details | Review
ATK patch (15.12 KB, patch)
2012-12-06 21:09 UTC, Peter De Wachter
committed Details | Review

Description Peter De Wachter 2012-12-06 21:04:53 UTC
Created attachment 230918 [details]
header edit script

GCC and other C compilers have optimizations for multiple inclusions of header files that are contained entirely in an #ifndef/#endif pair [1]. Unfortunately, most of GTK+'s header files use the following pattern, with the __GTK_H_INSIDE__ check outside the guards, which disables the optimization:

  #if !defined (__GTK_H_INSIDE__) && !defined (GTK_COMPILATION)
  #error "Only <gtk/gtk.h> can be included directly."
  #endif
 
  #ifndef __GTK_COLOR_SELECTION_DIALOG_H__
  #define __GTK_COLOR_SELECTION_DIALOG_H__

The check can be safely moved inside the guards. My test program contains just the GTK+ include directive:

$ cat test.c
#include <gtk/gtk.h>

Before any changes, compilation (i.e. parsing the GTK+ includes) required about 355ms (best of 10 runs):

before$ time gcc -c `pkg-config --cflags gtk+-3.0` test.c
real    0m0.355s
user    0m0.284s
sys     0m0.060s

After making this change in the GTK+, GLib, ATK and GDK-pixbuf headers, compilation was about 100ms faster:

after$ time gcc -c `pkg-config --cflags gtk+-3.0` test.c
real    0m0.252s
user    0m0.196s
sys     0m0.048s

I've attached the script I've used to make this change. It can be run using the command:

find repo/ -name '*.h' -exec perl fix-guard.pl {} +


[1] http://gcc.gnu.org/onlinedocs/cppinternals/Guard-Macros.html
Comment 1 Peter De Wachter 2012-12-06 21:07:40 UTC
Created attachment 230919 [details] [review]
GTK+ 3 patch
Comment 2 Peter De Wachter 2012-12-06 21:08:04 UTC
Created attachment 230920 [details] [review]
GTK+ 2 patch
Comment 3 Peter De Wachter 2012-12-06 21:08:32 UTC
Created attachment 230921 [details] [review]
GLib patch
Comment 4 Peter De Wachter 2012-12-06 21:08:56 UTC
Created attachment 230922 [details] [review]
GDK-pixbuf patch
Comment 5 Peter De Wachter 2012-12-06 21:09:18 UTC
Created attachment 230923 [details] [review]
ATK patch
Comment 6 Alejandro Piñeiro Iglesias (IRC: infapi00) 2013-03-26 10:58:32 UTC
Review of attachment 230923 [details] [review]:

Looks good. Commit it please.
Comment 7 Daniel Boles 2017-05-13 19:53:40 UTC
Running Peter's script on current ATK also works fine. Maybe we can land this?
Comment 8 Daniel Boles 2017-05-26 15:36:31 UTC
Comment on attachment 230923 [details] [review]
ATK patch

I reran the script to produce an equivalent commit, and pushed it to master after getting an ack from Emmanuele on IRC.
Comment 9 Michael Catanzaro 2018-05-16 12:51:59 UTC
I proposed doing this for WebKit a while back, but Carlos Garcia pointed out that it is simply wrong. Consider that it breaks the single-include guard. Doesn't that now only work the *first* time the header is included?

So:

#include <atk.h>
#include <atk/atksomething.h>

would no longer produce a build failure, right?