Pragma once

[Nico Weber]

Hi,

dpranke suggested that external include guards might speed up
compilation on windows quite a bit. External include guard means that
instead of including a file like this

#include "base/string16.h"

you include it like this:

#ifndef BASE_STRING16_H_
#include "base/string16.h"
#endif

(assuming that BASE_STRING16_H_ is the include guard used in string16.h).

The idea is that if string16.h has been included already, the compiler
doesn't even need to start string16.h a second time.

Another approach to achieve more or less the same is to add #pragma
once to string16.h. This is nicer in that including files don't need
to be littered with #ifndef/#endif noise. The downside is that not all
compilers support #pragma once (but both gcc and msvc do, and other
compilers just ignore it, so it doesn't really hurt).

gcc doesn't need external include guards and does this optimization
itself, but msvc doesn't seem to do this.
http://www.gamearchitect.net/Articles/ExperimentsWithIncludes.html
suggests that as well.

I tried doing clean builds of chrome on my windows box (z600). I did
this only for base, chrome, gfx for testing, but that should be good
enough to get an idea. Results:

Normal clean build: 40 min (I'm apparently not using /mp?)
Clean build w/ #pragma once to all headers in chrome, base, and gfx: 36 min
Clean build w / external include guards: 35 min

This suggests we should add "#pragma once" to all our header files
(except for the _messages_internal headers used for IPC, cause that
breaks stuff). Is anyone opposed to this idea?

Nico

ps:
pragma once: http://codepad.org/AXRjx93w
external guards: http://codepad.org/jxdtJDc8

[Dirk Pranke]

Is it easy to do the same test on Mac and Linux and verify that we're
not seeing any benefit w/ gcc or clang? (and that they still work
correctly)?

-- Dirk

> --
> Chromium Developers mailing list: chromi...@chromium.org
> View archives, change email options, or unsubscribe:
>    http://groups.google.com/a/chromium.org/group/chromium-dev
>

[Nico Weber]

Try! :-)

[Nico Weber]

I just tested on Mac with, and as expected build times didn't change.

I retested on windows /mp, and build times went from 13 to 11 minutes.

http://src.chromium.org/viewvc/chrome/trunk/src/tools/pragmaonce/pragmaonce.py?view=markup&pathrev=53271
does now contain the script I intend to run on our source some time
next week (if I hear no objections until then).

Nico

On Wed, Jul 21, 2010 at 3:25 PM, Dirk Pranke <dpr...@chromium.org> wrote:

[Peter Kasting]

On Wed, Jul 21, 2010 at 4:53 PM, Nico Weber <tha...@chromium.org> wrote:

I just tested on Mac with, and as expected build times didn't change.

I retested on windows /mp, and build times went from 13 to 11 minutes.

http://src.chromium.org/viewvc/chrome/trunk/src/tools/pragmaonce/pragmaonce.py?view=markup&pathrev=53271
does now contain the script I intend to run on our source some time
next week (if I hear no objections until then).

When you do that, make sure to list in the change description what files are being skipped and why.  (I assume before you do that you'll fix anything that's just flat broken.)

PK 

[Nico Weber]

I added #pragma once to ~2700 header files yesterday. Judging from

http://build.chromium.org/buildbot/waterfall/stats/Chromium%20XP
http://build.chromium.org/buildbot/waterfall/stats/Chromium%20Builder
http://build.chromium.org/buildbot/waterfall/stats/Chromium%20Builder%20(dbg)

, build times on the bots didn't go down at all (how often are these
charts updated? They're not labeled).

On Wed, Jul 21, 2010 at 3:10 PM, Elliot Glaysher <e...@google.com> wrote:
> Feel free to send the review to me when you do this.

[Nico Weber]

Sounds like the Right Thing is to revert the #pragma once patch and to
blame my local 10% speedup on disk cache effects or something? (Though
I would've thought linking chrome evicts every cache in this world.)

On Tue, Jul 27, 2010 at 9:25 AM, Thomas Van Lenten <thom...@google.com> wrote:
> I believe they update with every run.
> TVL

[Albert J. Wong (王重傑)]

Before reverting maybe someone else should try to see if there's a local speedup?  If it helps local devs, it could still be worth it, and the change doesn't really hurt does it?

-Albert

[Nicolas Sylvain]

On Tue, Jul 27, 2010 at 9:21 AM, Nico Weber <tha...@chromium.org> wrote:

I added #pragma once to ~2700 header files yesterday. Judging from

http://build.chromium.org/buildbot/waterfall/stats/Chromium%20XP
http://build.chromium.org/buildbot/waterfall/stats/Chromium%20Builder
http://build.chromium.org/buildbot/waterfall/stats/Chromium%20Builder%20(dbg)

, build times on the bots didn't go down at all (how often are these
charts updated? They're not labeled).

The charts are live. Always up to date.  Maybe it just does not help with incredibuild?

Nicolas

[Darin Fisher]

It adds maintenance burden.  Everyone will need to learn to maintain this and add #pragma once to new files.  That is not free.

-Darin

[Jeremy Orlow]

Sure, but if it makes everyone's build faster (even by 10%), it seems worth it.

[Peter Kasting]

On Tue, Jul 27, 2010 at 11:32 PM, Darin Fisher <da...@chromium.org> wrote:

It adds maintenance burden.  Everyone will need to learn to maintain this and add #pragma once to new files.  That is not free.

I create new files by starting with a partial or complete copy of an old file.  I would imagine most others do too, in order to get e.g. the license text correct.  In that sense, I don't see this as adding any maintenance burden, any more than the license does.

PK 

[Darin Fisher]

Maybe I'm the odd one, but I always start fresh when creating a new file so that I don't have anything residual left over from an existing file that I didn't intend.  I just open an existing file and copy/paste the 3 lines of license into the new file.

Anyways, while we could take a poll and optimize around that, I don't think that's the point.  We're talking about something that would need to go into the style guide and be enforced at review time.  Perhaps a presubmit check could be written.  If this is really worth it, then we should probably implement that.

-Darin

[Robert Sesek]

On Thu, Jul 29, 2010 at 12:26 AM, Darin Fisher <da...@chromium.org> wrote:

Perhaps a presubmit check could be written.  If this is really worth it, then we should probably implement that.

I agree that a presubmit check is the best way to enforce this.

http://codereview.chromium.org/3038036.

rsesek / @chromium.org

[Matthew Harris]

If this is part of the new Chromium style, could you please add it to
the style guide? <http://dev.chromium.org/developers/coding-style>

I'm speaking as a Google readability reviewer attempting to ensure
compliance with the style guide. The Google C++ Style Guide forbids
#pragma once <http://google-styleguide.googlecode.com/svn/trunk/
cppguide.xml?showone=Windows_Code#Windows_Code> and the Chromium
additions don't currently mention it.


Matthew

[Dirk Pranke]

I think this ended up being unclear if it was a win or not, so we're
not currently requiring it (or forbidding it).

-- Dirk

[Peter Kasting]

On Thu, Sep 9, 2010 at 6:39 PM, Dirk Pranke <dpr...@chromium.org> wrote:

I think this ended up being unclear if it was a win or not, so we're
not currently requiring it (or forbidding it).

I added it to the style guide.

AFAIK we're going with it.  It's easy for us to reverse course and back it out, and change the style guide accordingly, but unless and until that happens, I don't think the files should all be one way and the style guide the other.

PK