Blink build system improvements (many minutes faster)

[Daniel Bratell]

For those that remember the "[webkit-dev] Compiling WebKit up to 25%
faster in Windows?" thread in webkit-dev (
https://lists.webkit.org/pipermail/webkit-dev/2013-March/024303.html ),
I've kept working on it when I've had time and I now have something that
seems to work internally at Opera.

The improvements as measured on our build servers is 7 minutes faster
compilation (20% overall; up to 40% faster in WebKit/Blink) in Windows.

The big question is, how can I put these changes on Blink trunk/master so
that everyone benefits from them? It's implemented as a patch on some
scripts and gyp/gypi files and then a script that modifies somewhere
around 4000 .mm, .cpp, .h and other source code files.

Uploading it as just another patch is unlikely to work since it will be
obsolete after just a few hours. It is probably also not easy to review in
a time efficient and meaningful manner.

Technical details:
------------------
It's all about reducing the number of -I arguments to the compiler, and to
do that the source files must themselves tell the compiler where to find
the header files. So instead of a hundred or so include paths for
WebKit/Blink, I've made src/third_party/WebKit/Source an include path and
then changed for instance

#include "ActiveDOMObject.h"

to

#include "WebCore/dom/ActiveDOMObject.h"


To get that also in generated files I've modified various code generation
scripts to give the path to header files. The trickiest part being to map
an IDL name to a path, and there I added a special mapping table created
by a script (not integrated in the build but should be).

Output from git apply --stat:

.../Source/Platform/Platform.gyp/Platform.gyp | 25 -
.../WebKit/Source/WebCore/WebCore.gyp/WebCore.gyp | 148 ----
.../scripts/action_derivedsourcesallinone.py | 2
third_party/WebKit/Source/WebCore/WebCore.gypi | 83 --
.../WebCore/bindings/scripts/CodeGeneratorCPP.pm | 2
.../bindings/scripts/CodeGeneratorGObject.pm | 2
.../WebCore/bindings/scripts/CodeGeneratorJS.pm | 2
.../WebCore/bindings/scripts/CodeGeneratorObjC.pm | 2
.../WebCore/bindings/scripts/CodeGeneratorV8.pm | 306 ++++++---
.../WebCore/bindings/scripts/InFilesCompiler.pm | 10
.../Source/WebCore/bindings/scripts/idltopath.pm | 683
++++++++++++++++++++
third_party/WebKit/Source/WebCore/css/makeprop.pl | 3
.../WebKit/Source/WebCore/css/makevalues.pl | 2
.../Source/WebCore/dom/make_dom_exceptions.pl | 11
.../Source/WebCore/dom/make_event_factory.pl | 6
.../WebKit/Source/WebCore/dom/make_names.pl | 58 +-
.../WebCore/html/parser/create-html-entity-table | 2
.../inspector/CodeGeneratorInspectorStrings.py | 8
.../WebKit/Source/WebCore/page/make_settings.pl | 6
.../WebKit/Source/WebKit/chromium/WebKit.gyp | 2
webkit/compositor_bindings/compositor_bindings.gyp | 3
21 files changed, 964 insertions(+), 402 deletions(-)

And then the actual header file changes:

4295 files changed, 16954 insertions(+), 16954 deletions(-)

----

I realize that there is more work to do, such as compilation testing in
configurations I don't have access to, cleaning up scripts I've changed,
adapting to changes happening on trunk/master, and so on, but also, I
think more people than I need to know about this to avoid making each
others life harder than necessary and I think we need a plan for it.

/Daniel

[John Abd-El-Malek]

I'm really looking forward to this, thank you! On the Z620s that Googlers build with, a Windows build takes 30 minutes. So shaving 6 minutes of that is a really nice improvement.

I wonder if there are similar changes we can do to the non-blink gyp files to reduce other include directories.

[Scott Graham]

This is great, I've wanted to do this for a while too, but it wasn't
really feasible inside WebKit.

As pointed out in a previous thread ("Stoooooopid questions") we
should probably wait until the code moves to src/blink to avoid
changing all the #include lines twice.

It seems like instead of trying to land a many thousand file patch,
it'd be better to work incrementally. Could we deal with a large
percentage of the diff by having a set of patches where each just
removes a single "-I" on the commandline/gyp and patches the relevant
files?

[Dirk Pranke]

On Mon, Apr 15, 2013 at 9:12 AM, Scott Graham <sco...@chromium.org> wrote:

This is great, I've wanted to do this for a while too, but it wasn't
really feasible inside WebKit.

As pointed out in a previous thread ("Stoooooopid questions") we
should probably wait until the code moves to src/blink to avoid
changing all the #include lines twice.

I don't know that I would wait, as I don't think we have a particular ETA for moving to src/blink. 

In theory, moving from unqualified includes ("foo.h") to qualified ("bar/foo.h") is much harder than simply renaming files once they're qualified. I would break the patch apart into a few steps:

1) Ensure that we have some common -I dir that all the files have; we probably have this already. This could be src/, src/third_party/WebKit, src/third_party/WebKit/Source (I'm not sure it matters).

2) Modify the scripts to generate headers based off of that. Use a constant in the scripts so that it's easy to change :).

3) Modify the source files in batches to use the qualified dir; there's probably no reason they need to all land at once, or even in any particular order.

-- Dirk

[Adam Barth]

On Mon, Apr 15, 2013 at 9:27 AM, Dirk Pranke <dpr...@chromium.org> wrote:

On Mon, Apr 15, 2013 at 9:12 AM, Scott Graham <sco...@chromium.org> wrote:

This is great, I've wanted to do this for a while too, but it wasn't
really feasible inside WebKit.

As pointed out in a previous thread ("Stoooooopid questions") we
should probably wait until the code moves to src/blink to avoid
changing all the #include lines twice.

I don't know that I would wait, as I don't think we have a particular ETA for moving to src/blink. 

Yeah, I agree that we shouldn't wait until we move to src/blink.

 

In theory, moving from unqualified includes ("foo.h") to qualified ("bar/foo.h") is much harder than simply renaming files once they're qualified. I would break the patch apart into a few steps:

1) Ensure that we have some common -I dir that all the files have; we probably have this already. This could be src/, src/third_party/WebKit, src/third_party/WebKit/Source (I'm not sure it matters).

I'd recommend using src/third_party/WebKit/Source as the base of the include directory.  Including more directories is just noise at this point.

Before we make this change, I would like to move the following directories:

A) Source/WebCore -> Source/core

B) Source/WTF/wtf -> Source/wtf

After we do that, the include paths should look pretty reasonable.  For example:

#include "wtf/RefPtr.h"

#include "core/dom/Document.h"

Moving those directories around shouldn't be too hard.  I should be able to move those directories by the end of the week, but if someone else is excited about moving code around, feel free to do it before me.  ;)

 

2) Modify the scripts to generate headers based off of that. Use a constant in the scripts so that it's easy to change :).

3) Modify the source files in batches to use the qualified dir; there's probably no reason they need to all land at once, or even in any particular order

Yep.

Adam

[Eric Seidel]

I'm strongly supportive of this proposal. As Adam notes, we should
wait until the core and wtf moves are done. The tree is mostly calmed
down from the big delete-fest.

Thanks for doing the investigation!

[James Robinson]

The script here: http://src.chromium.org/viewvc/chrome/trunk/src/tools/git/move_source_file.py can take care of updating includes and resorting #include blocks for all files in the repo.  I think it would be valuable to get it (or something like it and the other scripts in that directory) working for Blink to help out with changes like this.

- James

[Daniel Bratell]

Den 2013-04-15 18:12:25 skrev Scott Graham <sco...@chromium.org>:

> This is great, I've wanted to do this for a while too, but it wasn't
> really feasible inside WebKit.
>
> As pointed out in a previous thread ("Stoooooopid questions") we
> should probably wait until the code moves to src/blink to avoid
> changing all the #include lines twice.

Makes sense. Though it will be much easier replacing an include once it
has a path since then it will just be search-and-replace compared to today
where it's search-search-search-guess-insert.

> It seems like instead of trying to land a many thousand file patch,
> it'd be better to work incrementally. Could we deal with a large
> percentage of the diff by having a set of patches where each just
> removes a single "-I" on the commandline/gyp and patches the relevant
> files?

I don't think that is a feasible method. It would mean 100+ patches that
all conflicted among each other so it would take a year or so to handle.

Anyway, to show you the current changes to gyp files I uploaded the patch
to
http://pastebin.com/90vx4sep and the script to
http://pastebin.com/WmzGY8zs. Neither have been cleaned up so they are a
bit embarrassing (many debugging tools still in them).

And the patch is based on a pre-blink world so it doesn't apply at all to
current trunk.

/Daniel

[Daniel Bratell]

Den 2013-04-15 18:27:29 skrev Dirk Pranke <dpr...@chromium.org>:

> On Mon, Apr 15, 2013 at 9:12 AM, Scott Graham <sco...@chromium.org>
> wrote:
>
>> This is great, I've wanted to do this for a while too, but it wasn't
>> really feasible inside WebKit.
>>
>> As pointed out in a previous thread ("Stoooooopid questions") we
>> should probably wait until the code moves to src/blink to avoid
>> changing all the #include lines twice.
>>
>>
> I don't know that I would wait, as I don't think we have a particular ETA
> for moving to src/blink.
>
> In theory, moving from unqualified includes ("foo.h") to qualified
> ("bar/foo.h") is much harder than simply renaming files once they're
> qualified. I would break the patch apart into a few steps:
>
> 1) Ensure that we have some common -I dir that all the files have; we
> probably have this already. This could be src/, src/third_party/WebKit,
> src/third_party/WebKit/Source (I'm not sure it matters).

I've assumed src/third_party/WebKit/Source and added it to two targets
that didn't have it.

> 2) Modify the scripts to generate headers based off of that. Use a
> constant
> in the scripts so that it's easy to change :).

Yes, that part is trivial to change in the script I've used (the
include_root variable in the script at http://pastebin.com/WmzGY8zs ). As
I said in the other mails. They are full of debug help lines so they are
not nice to read but I trust nobody will follow that link anyway. :-)

> 3) Modify the source files in batches to use the qualified dir; there's
> probably no reason they need to all land at once, or even in any
> particular
> order.

And all the scripts that generate source files (the pm parts of
http://pastebin.com/90vx4sep )

You forgot the last step that actually is the most important step:

4) Remove all the webcore_include_dirs from the gyp and gypi files.
Without that there is almost no effect at all (a very small one but not
40%).

Basically the gyp part of the same diff: http://pastebin.com/90vx4sep

But a plan like this requires someone that can commit. I cannot.

/Daniel

[Dirk Pranke]

If you can post patches, I'm sure someone (e.g., me) can help you get them landed ...

-- Dirk

[Eric Seidel]

We can fix the you-cannot-commit part.

I think your 4-step proposal sounds great. I think that the wtf and
WebCore -> core moves are blocking you. We should be able to have
those resolved this week and then someone just needs to start posting
patches (committer or otherwise) to start fixing headers to be full
paths.

On Mon, Apr 15, 2013 at 1:05 PM, Daniel Bratell <bra...@opera.com> wrote:

[Daniel Bratell]

Den 2013-04-15 18:46:02 skrev Adam Barth <aba...@chromium.org>:

>> 3) Modify the source files in batches to use the qualified dir; there's
>> probably no reason they need to all land at once, or even in any
>> particular
>> order

I'm a bit concerned about slippage where as long as the -I parts are still
there people add incorrect includes (easily done) that will then also have
to be added. If I were to do it as a non-committer in a different time
zone where patches take a while to handle it could be a serious issue.

/Daniel

[James Robinson]

Let's see if this ends up being a problem before attempting to solve it.  I think it'll be fine.

- James

 

/Daniel

[Adam Barth]

On Mon, Apr 15, 2013 at 9:46 AM, Adam Barth <aba...@chromium.org> wrote:

On Mon, Apr 15, 2013 at 9:27 AM, Dirk Pranke <dpr...@chromium.org> wrote:

On Mon, Apr 15, 2013 at 9:12 AM, Scott Graham <sco...@chromium.org> wrote:

This is great, I've wanted to do this for a while too, but it wasn't
really feasible inside WebKit.

As pointed out in a previous thread ("Stoooooopid questions") we
should probably wait until the code moves to src/blink to avoid
changing all the #include lines twice.

I don't know that I would wait, as I don't think we have a particular ETA for moving to src/blink. 

Yeah, I agree that we shouldn't wait until we move to src/blink.

 

In theory, moving from unqualified includes ("foo.h") to qualified ("bar/foo.h") is much harder than simply renaming files once they're qualified. I would break the patch apart into a few steps:

1) Ensure that we have some common -I dir that all the files have; we probably have this already. This could be src/, src/third_party/WebKit, src/third_party/WebKit/Source (I'm not sure it matters).

I'd recommend using src/third_party/WebKit/Source as the base of the include directory.  Including more directories is just noise at this point.

Before we make this change, I would like to move the following directories:

A) Source/WebCore -> Source/core

B) Source/WTF/wtf -> Source/wtf

After we do that, the include paths should look pretty reasonable.  For example:

#include "wtf/RefPtr.h"

#include "core/dom/Document.h"

Moving those directories around shouldn't be too hard.  I should be able to move those directories by the end of the week, but if someone else is excited about moving code around, feel free to do it before me.  ;)

The WTF move is now complete.  I'm working on moving WebCore.

Daniel, if you like you can start by removing the include directories for wtf and modules now.  We can then remove the include paths for WebCore once the move is done.

Adam

[Thiago Farina]

On Tue, Apr 16, 2013 at 4:50 PM, Adam Barth <aba...@chromium.org> wrote:

On Mon, Apr 15, 2013 at 9:46 AM, Adam Barth <aba...@chromium.org> wrote:

On Mon, Apr 15, 2013 at 9:27 AM, Dirk Pranke <dpr...@chromium.org> wrote:

On Mon, Apr 15, 2013 at 9:12 AM, Scott Graham <sco...@chromium.org> wrote:

This is great, I've wanted to do this for a while too, but it wasn't
really feasible inside WebKit.

As pointed out in a previous thread ("Stoooooopid questions") we
should probably wait until the code moves to src/blink to avoid
changing all the #include lines twice.

I don't know that I would wait, as I don't think we have a particular ETA for moving to src/blink. 

Yeah, I agree that we shouldn't wait until we move to src/blink.

 

In theory, moving from unqualified includes ("foo.h") to qualified ("bar/foo.h") is much harder than simply renaming files once they're qualified. I would break the patch apart into a few steps:

1) Ensure that we have some common -I dir that all the files have; we probably have this already. This could be src/, src/third_party/WebKit, src/third_party/WebKit/Source (I'm not sure it matters).

I'd recommend using src/third_party/WebKit/Source as the base of the include directory.  Including more directories is just noise at this point.

Before we make this change, I would like to move the following directories:

A) Source/WebCore -> Source/core

B) Source/WTF/wtf -> Source/wtf

After we do that, the include paths should look pretty reasonable.  For example:

#include "wtf/RefPtr.h"

#include "core/dom/Document.h"

Moving those directories around shouldn't be too hard.  I should be able to move those directories by the end of the week, but if someone else is excited about moving code around, feel free to do it before me.  ;)

The WTF move is now complete.  I'm working on moving WebCore.

Daniel, if you like you can start by removing the include directories for wtf and modules now.  We can then remove the include paths for WebCore once the move is done.

I'm very happy to see this change moving forward. Full paths makes much easier for new contributors and even long-term contributors to know where things come from and to see how they depend on each other.

--
Thiago

[Adam Barth]

I'm happy to review patches in this area.  I'd recommend starting with headers in wtf and then work up through yarr and modules.

Adam

[Thiago Farina]

On Tue, Apr 16, 2013 at 7:22 PM, Adam Barth <aba...@chromium.org> wrote:
>> I'm very happy to see this change moving forward. Full paths makes much
>> easier for new contributors and even long-term contributors to know where
>> things come from and to see how they depend on each other.
>
>
> I'm happy to review patches in this area. I'd recommend starting with
> headers in wtf and then work up through yarr and modules.

SG. wtf/unicode is done. Eric reviewed it.

--
Thiago

[Adam Barth]

Great!

Adam

[Daniel Bratell]

Cool!

Does this mean all the headers will fix themselves(TM) without me starting
to create patches and chasing reviewers?

I'm not going to let go of this issue since this is important for me (and
Opera) as well, but I have other things to do as well and I'm still not
experienced in the chromium/blink patch world, which is also moving around
quite quickly right now so I'm not working on this every day.

/Daniel

[Thiago Farina]

On Wed, Apr 17, 2013 at 12:57 PM, Daniel Bratell <bra...@opera.com> wrote:
> Den 2013-04-17 05:28:57 skrev Thiago Farina <tfa...@chromium.org>:
>
>> On Tue, Apr 16, 2013 at 7:22 PM, Adam Barth <aba...@chromium.org> wrote:
>>>>
>>>> I'm very happy to see this change moving forward. Full paths makes much
>>>> easier for new contributors and even long-term contributors to know
>>>> where
>>>> things come from and to see how they depend on each other.
>>>
>>>
>>>
>>> I'm happy to review patches in this area. I'd recommend starting with
>>> headers in wtf and then work up through yarr and modules.
>>
>> SG. wtf/unicode is done. Eric reviewed it.
>
> Cool!
>
> Does this mean all the headers will fix themselves(TM) without me starting
> to create patches and chasing reviewers?
>

They won't fix themselves as they aren't AI :)

But devs will fix it while we move forward, and I'm happy to help in
this area as I get more familiar with the code layout of blink. ;)

> I'm not going to let go of this issue since this is important for me (and
> Opera) as well, but I have other things to do as well and I'm still not
> experienced in the chromium/blink patch world, which is also moving around
> quite quickly right now so I'm not working on this every day.
>

I also have limited time to hack on this, but will try to send nightly
patches in this area.

Thanks,

--
Thiago

[Darin Fisher]

Thanks for helping with the conversion, Thiago!  Much appreciated.

[Nico Weber]

I started moving a few includes over (http://crbug.com/234793).

One issue I ran into is includes from generated binding files. We compile lots of idl files into h and cpp files, and these generated files simply say |#include "IdlFilename.h"|. Instead, they should say |#include "path/to/IdlFilename.h"|. To make this work, we need some way to know where the h file corresponding to an idl file is.

Possible approaches:

1) Have a big perl dictionary in the generate-bindings script that maps unqualified header names to their qualified paths ("Storage.h" -> "core/storage/Storage.h" etc). Cons: Requires keeping that dictionary up to date, ugly.

2) Like 1, but keep that dictionary in a data file and create it at build time. Cons: Introduces a bunch of complexity.

3) Assume that the h file is in the same folder as the idl file (if you're processing "core/storage/Storage.idl", use that folder for the #include line). Cons: Currently not true for ScriptProfile.h/idl, ScriptProfileNode.h/idl, JavaScriptCallFrame.h/idl (they're in Source/bindings/v8 and Source/core/inspector respectively at the moment).

Does anyone with bindings expertise have an opinion what the best approach is?

Thanks,

Nico

[Adam Barth]

Approach 3 sounds promising.  Perhaps we can move the IDL files that don't follow the pattern?

Adam

[Nils Barth]

Koji, haraken, any thoughts on qualified paths in generated bindings, per Nico's query?

Per Adam's comment, assuming that header files are in the same directory as the IDL file seems easiest, both for code generation and code organization (keep .h and .idl together).

Is this easy enough to implement?

[Thomas Sepez]

Would this be the first case where generated files don't wind up under the /out directory?

I'm fond of the "when all else fails" cleanup of rm -fr out/Release

Is there a risk of cross-pollenation when I generate the files by doing a debug build, and then run a release build in the same checkout?

[Thomas Sepez]

Also, do you get the majority of the speedup if you make all the other paths "absolute", leave paths to generated files "relative", put then all in a single out/Release/gen directory, and -I out/Release/gen on the compilation command?

In other words, do we have to nail every last one of these?

If you don't like the single out/Debug/gen directory, maybe for these the path is relative to a  /out/Debug root rather than /src

as in:

#include "gen/webkit/bindings/V8Node.h"

[Adam Barth]

On Fri, Apr 26, 2013 at 10:54 AM, Thomas Sepez <tse...@google.com> wrote:

Would this be the first case where generated files don't wind up under the /out directory?

I'm fond of the "when all else fails" cleanup of rm -fr out/Release

Is there a risk of cross-pollenation when I generate the files by doing a debug build, and then run a release build in the same checkout?

All of the generated files will be placed in the "out" directory.  The question is how the generated files will refer back to non-generated headers.  For example, Node.idl generates V8Node.h, which includes Node.h.  Somehow V8Node.h will need to know that Node.h is in the "dom" directory.

As for including generated headers (e.g., V8Node.h) from non-generated sources, we'll need to add the directory where the generated files are stored to the include path in some form because it's different on different platforms.  We might want to place all the generated headers in a subdirectory called "generated" or something so that then can by included as #include "generated/V8Node.h".

Adam

[Elliott Sprehn]

On Fri, Apr 26, 2013 at 2:59 PM, Adam Barth <aba...@chromium.org> wrote:

On Fri, Apr 26, 2013 at 10:54 AM, Thomas Sepez <tse...@google.com> wrote:

Would this be the first case where generated files don't wind up under the /out directory?

I'm fond of the "when all else fails" cleanup of rm -fr out/Release

Is there a risk of cross-pollenation when I generate the files by doing a debug build, and then run a release build in the same checkout?

All of the generated files will be placed in the "out" directory.  The question is how the generated files will refer back to non-generated headers.  For example, Node.idl generates V8Node.h, which includes Node.h.  Somehow V8Node.h will need to know that Node.h is in the "dom" directory.

As for including generated headers (e.g., V8Node.h) from non-generated sources, we'll need to add the directory where the generated files are stored to the include path in some form because it's different on different platforms.  We might want to place all the generated headers in a subdirectory called "generated" or something so that then can by included as #include "generated/V8Node.h".

+1, I like this because it would make it super obvious when a file is generated. We're moving to more and more generated stuff for settings, styles, etc. so it'll be good to know where to find files.

- E 

[Thomas Sepez]

Gotcha.  The second problem seems more interesting to me, which is why I missed the point in the first place.

I don't think that a single /generated is necessarily a good idea.  I do think that having #include "generated/path/to/V8Node.h" is a good idea, though.

[Daniel Bratell]

Den 2013-04-26 02:03:29 skrev Nico Weber <tha...@chromium.org>:

> I started moving a few includes over (http://crbug.com/234793).
>
> One issue I ran into is includes from generated binding files. We compile
> lots of idl files into h and cpp files, and these generated files simply
> say |#include "IdlFilename.h"|. Instead, they should say |#include
> "path/to/IdlFilename.h"|. To make this work, we need some way to know
> where
> the h file corresponding to an idl file is.
>
> Possible approaches:
>
> 1) Have a big perl dictionary in the generate-bindings script that maps
> unqualified header names to their qualified paths ("Storage.h" ->
> "core/storage/Storage.h" etc). Cons: Requires keeping that dictionary up
> to
> date, ugly.
>
> 2) Like 1, but keep that dictionary in a data file and create it at build
> time. Cons: Introduces a bunch of complexity.

It was the approach in 2) combined with the assumption that .idl and .h
goes together I took when I implemented this so it can work. The amount of
complexity is not great, but it's on top of something that is already
overly complex so a less complex solution is clearly favourable. (the #2
approach is in https://codereview.chromium.org/14456006/ )

I never considered moving files around to make the life simpler though,
but instead hardcoded a number of exceptions for strange cases.

/Daniel

[Nico Weber]

This is now done for core/. (Doing the api layer is blocked on it moving to a different folder.)

Here are a few numbers:

18217 #include lines are now qualified, the average path length aded is 13.5 characters. This means the code is now about 250kB of paths bigger. In return, core/ now injects 3 additional include directories into its targets and its direct dependencies instead of ~100 previously, and the build command line for a random translation unit (HTMLButtonElement.cpp) dropped from ~12kB to ~6kB. Generated ninja files are ~3% smaller, and gyp runs ~3% faster.

I did full builds at chromium r196797 and blink each at r149011 (before any of the path qualification CLs landed) and r149309 (after they were all in). I skimmed the git log, no other large reorganizations or additions landed during that time. I compared full build times and incremental build times, on my Mac laptop and my Windows desktop. (I did two full builds at each configuration, to make sure that the build times are consistent.)

Mac:

Before: 25m7.939s, 25m5.304s

After: 24m54.567s, 24m49.789s

~1% faster

Windows:

Before: 14m13s, 13m58.724s

After: 14m43.2s, 14m43.7s

~4% slower

So from a build performance perspective, this seems to not make a big difference. (bratell: Can you maybe run the same experiment on your machine and report results?) It's easier to understand though, and I even found and fixed one case where an include was accidentally resolved to different files in different translation units.

There's a few open ends, for example not all includes in the generated bindings cpp files are qualified. But given that it makes no perf difference and that these files are generated anyhow, I'm not sure if that's worth addressing.

Nico

[Daniel Bratell]

You have a fast Windows machine.

My full recompilation is about 45 minutes so it takes me a day to collect
data so I usually just measure individual projects. I did some of those
the day before yesterday where I saw this:

webcore_dom
Before: 58s
After*: 47s
(with the change I did 1-2 months ago: 38s)

webcore_platform
Before: 59s
After*: 44s
(with the change I did 1-2 months ago: 34s)

After* being the day before yesterday, not today.

I don't know why you don't see any total improvement. Maybe something in
the Google build environment makes you have a different bottleneck
(transfer time of files?) than people outside? Maybe different compiler?
I've measured this with VS2010, but I had consistent results from VS2012
builds as well. I collected the data from 5 runs, making sure all but the
first returned about the same time and took the median (which typically
was about the same as the fastest).

I'll return when I have more data.

/Daniel

[Adam Barth]

Perhaps Nico is using an SSD and you're using a spinning disk?  I can imagine having many include directories to search is causing more seeks, which might be much more expensive with a spinning disk than with an SSD.

Adam

[Daniel Bratell]

No, it's not even about I/O. Same compilation times (more or less) with or
without SSD, since it's all done in file caches and file buffers.
Monitoring the I/O during the compilation of a compilation unit shows
almost instant I/O with a high level of CPU Usage. So whatever the
Microsoft Compiler does, it's not I/O.

It has also been seen and verified by other people here as Opera as well
as by our build servers.

If you have an infinite amount of CPUs I guess this wouldn't matter, but
that is not the case for me. :-)

/Daniel

[Nico Weber]

I don't use any special build environment. I'm doing a regular local build with ninja and MSVS2010. I used the 4 lines described at https://code.google.com/p/chromium/wiki/NinjaBuild#Windows to build.

Are you building with ninja, with MSVS from the command line, or from the MSVS IDE?

[Daniel Bratell]

Den 2013-04-30 08:31:31 skrev Daniel Bratell <bra...@opera.com>:
>
> I'll return when I have more data.

I have more data.

I can verify that the current patch to add paths to generated files (used
by the webcore_derived project) has no measurable effect on compilation
time (somewhere between 0.0 and 0.5% faster which is less than the time
added elsewhere).

It's still not the same compilation speed as what I saw when I originally
posted this discussion which means that there is something else to find. I
will keep looking.

/Daniel

[Daniel Bratell]

I think we can declare this mission accomplished.

Big thanks to abarth, thakis, harakan and others who picked this up and
made something happen.

Proof in the form of some graphs showing big compilation time improvements
in some blink projects:

https://picasaweb.google.com/110337600520532820098/PublicData#5875350723587357794
[1]

webcore_dom -47%
webcore_html -46%
webcore_platform -49%
webcore_rendering -43%

Sadly the numbers will not be the same for everyone. In particular it
seems like Google developers (i.e. most Blink developers) have other
bottlenecks and will not see as big improvements [2].

But from the rest of us, this is very much appreciated!

In the future it can be even better[3] but one battle[4] at a time.

/Daniel

[1] Just checking if picasaweb is a good place to store charts; was not
sure if it was considered ok to include such things in forum posts or not.

[2] My guess is that you're able to parallel compile so much that the
bottleneck becomes those sections where only a single cpu core can be used.

[3] We compile the Presto program corresponding to content_shell in 3-4
minutes on the same hardware that needed 50 minutes to compile chromium
content_shell.

[4] New features will always add more source to compile and at the long
compilation times we already have this is much bigger than a single
change/battle.

/Daniel

[Adam Barth]

That's very interesting!  Do you know what's different about the two that leads to such different compile times?  10x seems like a big speedup.

Adam

[Nico Weber]

On Tue, May 7, 2013 at 2:49 PM, Daniel Bratell <bra...@opera.com> wrote:

Den 2013-05-03 16:24:26 skrev Daniel Bratell <bra...@opera.com>:

Den 2013-04-30 08:31:31 skrev Daniel Bratell <bra...@opera.com>:

I'll return when I have more data.

I have more data.

I can verify that the current patch to add paths to generated files (used by the webcore_derived project) has no measurable effect on compilation time (somewhere between 0.0 and 0.5% faster which is less than the time added elsewhere).

It's still not the same compilation speed as what I saw when I originally posted this discussion which means that there is something else to find. I will keep looking.

I think we can declare this mission accomplished.

Big thanks to abarth, thakis, harakan and others who picked this up and made something happen.

Proof in the form of some graphs showing big compilation time improvements in some blink projects:

https://picasaweb.google.com/110337600520532820098/PublicData#5875350723587357794 [1]

webcore_dom -47%
webcore_html -46%
webcore_platform -49%
webcore_rendering -43%

Sadly the numbers will not be the same for everyone. In particular it seems like Google developers (i.e. most Blink developers) have other bottlenecks and will not see as big improvements [2].

But from the rest of us, this is very much appreciated!

Thans for the data! It looks like the majority of the speedup was before Apr 21 though, while the path absolutification happend Apr 24 – Apr 28. I'm thinking the speedup is mostly from

https://codereview.chromium.org/14323004

https://codereview.chromium.org/13976008

https://codereview.chromium.org/13926016

which had a measurable build time impact for us too.

Nico

[Daniel Bratell]

Den 2013-05-07 23:54:12 skrev Adam Barth <aba...@chromium.org>:

> On Tue, May 7, 2013 at 10:49 PM, Daniel Bratell <bra...@opera.com>
> wrote:
>
>> Den 2013-05-03 16:24:26 skrev Daniel Bratell <bra...@opera.com>:
>>
>> Den 2013-04-30 08:31:31 skrev Daniel Bratell <bra...@opera.com>:
>>>
>>>>
>>>> I'll return when I have more data.
>>>>
>>>
>>> I have more data.
>>>
>>> I can verify that the current patch to add paths to generated files
>>> (used
>>> by the webcore_derived project) has no measurable effect on compilation
>>> time (somewhere between 0.0 and 0.5% faster which is less than the time
>>> added elsewhere).
>>>
>>> It's still not the same compilation speed as what I saw when I
>>> originally
>>> posted this discussion which means that there is something else to
>>> find. I
>>> will keep looking.
>>>
>>
>> I think we can declare this mission accomplished.
>>
>> Big thanks to abarth, thakis, harakan and others who picked this up and
>> made something happen.
>>
>> Proof in the form of some graphs showing big compilation time
>> improvements
>> in some blink projects:
>>

>> https://picasaweb.google.com/**110337600520532820098/**
>> PublicData#5875350723587357794<https://picasaweb.google.com/110337600520532820098/PublicData#5875350723587357794>[1]

>>
>> webcore_dom -47%
>> webcore_html -46%
>> webcore_platform -49%
>> webcore_rendering -43%
>>
>> Sadly the numbers will not be the same for everyone. In particular it
>> seems like Google developers (i.e. most Blink developers) have other
>> bottlenecks and will not see as big improvements [2].
>>
>> But from the rest of us, this is very much appreciated!
>>
>> In the future it can be even better[3] but one battle[4] at a time.
>>
>> /Daniel
>>
>> [1] Just checking if picasaweb is a good place to store charts; was not
>> sure if it was considered ok to include such things in forum posts or
>> not.
>>
>> [2] My guess is that you're able to parallel compile so much that the
>> bottleneck becomes those sections where only a single cpu core can be
>> used.
>>
>> [3] We compile the Presto program corresponding to content_shell in 3-4
>> minutes on the same hardware that needed 50 minutes to compile chromium
>> content_shell.
>>
>
> That's very interesting! Do you know what's different about the two that
> leads to such different compile times? 10x seems like a big speedup.

Without knowing for sure, my guess is a combination of *much* larger
compilation units, less source code, less code generated at compile time
and dramatically fewer VS projects, in approximately that order.

/Daniel

[Daniel Bratell]

On Mon, 15 Apr 2013 13:38:42 +0200, Daniel Bratell <bra...@opera.com>
wrote:

> Technical details:
> ------------------
> It's all about reducing the number of -I arguments to the compiler, and
> to do that the source files must themselves tell the compiler where to
> find the header files. So instead of a hundred or so include paths for
> WebKit/Blink, I've made src/third_party/WebKit/Source an include path
> and then changed for instance
>
> #include "ActiveDOMObject.h"
>
> to
>
> #include "WebCore/dom/ActiveDOMObject.h"

Reviving an old thread to give it a final summary.

I just got a message from Microsoft that says that the performance
problems with many include paths is fixed in VS2013. Not that we care
anymore since we've (mostly) stopped with the ugly habit of letting the
compiler do a file system search for unqualified header files.

/Daniel