Separate some generated files into their own targets

Separate some generated files into their own targets

In each case you have a trivial action (a python script generating a single file), acting as a big build barrier, because gyp makes the action wait on all dependencies of the library it's part of (aka half of the world) before allowing dependent libraries to start building.
Separating it out removes that barrier, significantly improving parallelism potential and build speed.

BUG=None
TEST=local compile tests (from scratch) + trybots


Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=108434

[bradn, 2011-10-31 16:46:17]

lgtm

[tony, 2011-10-31 17:51:16]

LGTM.

The change as-is probably improves build time for make/ninja, but won't help
XCode (it only parallelizes within a target) and may help MSVS, but will slow
down sln load time since it adds 3 new .vcproj files.

I suspect most of the time is in python startup.  I wonder if we should start
looking into other ways of running scripty actions during the build (maybe
v8_shell would be faster?  not sure how i/o would work).

[piman, 2011-10-31 19:17:45]

On Mon, Oct 31, 2011 at 10:51 AM, <tony@chromium.org> wrote:

> LGTM.
>
> The change as-is probably improves build time for make/ninja, but won't
> help
> XCode (it only parallelizes within a target) and may help MSVS, but will
> slow
> down sln load time since it adds 3 new .vcproj files.
>
> I suspect most of the time is in python startup.  I wonder if we should
> start
> looking into other ways of running scripty actions during the build (maybe
> v8_shell would be faster?  not sure how i/o would work).
>

The gains don't come from parallelizing these actions specifically - they
take no time at all - but they come from the fact that the actions, when
included into the targets, act as build barriers, preventing e.g. all the
.cc files included in the 'browser' target from compiling until all
dependencies of 'browser' (i.e. all of webkit, and a bunch of chrome stuff)
have completely built. This patch makes them buildable in parallel. If
anything it's a limitation of the gyp language, but it can be worked around
nicely this way.

Is the sln load time that dependent on the number of vcprojs - as opposed
to actual files etc.? We're doing the same stuff just organized in targets
a bit differently.
Coupled with http://codereview.chromium.org/8400082/ it can save 1 to 2
minutes on a ninja build (and probably on the make build with an equivalent
patch).


>
http://codereview.chromium.**org/8417017/<http://codereview.chromium.org/8417...
>

[tony, 2011-10-31 20:09:38]

On 2011/10/31 19:17:45, piman wrote:
> The gains don't come from parallelizing these actions specifically - they
> take no time at all - but they come from the fact that the actions, when
> included into the targets, act as build barriers, preventing e.g. all the
> .cc files included in the 'browser' target from compiling until all
> dependencies of 'browser' (i.e. all of webkit, and a bunch of chrome stuff)
> have completely built. This patch makes them buildable in parallel. 

Maybe I misunderstand, but doesn't this just help with the time of the actions
since we still require that the actions run first?  This is my understanding.

Before:
- compile/run dependent targets in parallel
- run browser action
- compile browser cc files in parallel

After:
- compile/run dependent targets in parallel including autofill_regexes
- compile browser cc files in parallel

As in, don't we still have to wait for all the dependencies of browser to be
done before compiling any of the cc files?

> Is the sln load time that dependent on the number of vcprojs - as opposed
> to actual files etc.? We're doing the same stuff just organized in targets
> a bit differently.
> Coupled with http://codereview.chromium.org/8400082/ it can save 1 to 2
> minutes on a ninja build (and probably on the make build with an equivalent
> patch).

I think it depends on both, but more vcproj files definitely slow down gyp time
and sln load time.

[tony, 2011-10-31 20:10:38]

Also, if it wasn't clear, I think this patch is still fine to land.

[commit-bot: I haz the power, 2011-10-31 20:24:39]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/piman@chromium.org/8417017/4001

[piman, 2011-10-31 20:26:45]

On Mon, Oct 31, 2011 at 1:09 PM, <tony@chromium.org> wrote:

> On 2011/10/31 19:17:45, piman wrote:
>
>> The gains don't come from parallelizing these actions specifically - they
>> take no time at all - but they come from the fact that the actions, when
>> included into the targets, act as build barriers, preventing e.g. all the
>> .cc files included in the 'browser' target from compiling until all
>> dependencies of 'browser' (i.e. all of webkit, and a bunch of chrome
>> stuff)
>> have completely built. This patch makes them buildable in parallel.
>>
>
> Maybe I misunderstand, but doesn't this just help with the time of the
> actions
> since we still require that the actions run first?  This is my
> understanding.
>
> Before:
> - compile/run dependent targets in parallel
> - run browser action
> - compile browser cc files in parallel
>
> After:
> - compile/run dependent targets in parallel including autofill_regexes
> - compile browser cc files in parallel
>
> As in, don't we still have to wait for all the dependencies of browser to
> be
> done before compiling any of the cc files?


With http://codereview.chromium.org/8400082/ we don't need to wait on the
compiles in the dependent targets before we can start compiling browser cc
files. So as soon as autofill_regexes executed, we can start them. With
this patch, autofill_regexes doesn't need to wait, so we can start
compiling browser's cc files almost immediately.


>
>  Is the sln load time that dependent on the number of vcprojs - as opposed
>> to actual files etc.? We're doing the same stuff just organized in targets
>> a bit differently.
>> Coupled with
http://codereview.chromium.**org/8400082/<http://codereview.chromium.org/8400...
can save 1 to 2
>> minutes on a ninja build (and probably on the make build with an
>> equivalent
>> patch).
>>
>
> I think it depends on both, but more vcproj files definitely slow down gyp
> time
> and sln load time.
>
>
>
http://codereview.chromium.**org/8417017/<http://codereview.chromium.org/8417...
>

[commit-bot: I haz the power, 2011-10-31 21:22:00]

Try job failure for 8417017-4001 (retry) on linux_rel for step "ui_tests".
It's a second try, previously, step "ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2011-10-31 21:26:23]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/piman@chromium.org/8417017/4001

[commit-bot: I haz the power, 2011-10-31 22:35:42]

Try job failure for 8417017-4001 (retry) on linux_rel for step "ui_tests".
It's a second try, previously, step "ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_rel&...

[commit-bot: I haz the power, 2011-11-02 20:07:20]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/piman@chromium.org/8417017/12001

[commit-bot: I haz the power, 2011-11-02 21:50:02]

Try job failure for 8417017-12001 (retry) on mac_rel for step "ui_tests".
It's a second try, previously, step "ui_tests" failed.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=mac_rel&nu...

[commit-bot: I haz the power, 2011-11-03 06:24:10]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/piman@chromium.org/8417017/12001

[commit-bot: I haz the power, 2011-11-03 07:35:23]

Change committed as 108434

[Ryan Sleevi, 2011-11-03 07:55:18]

http://codereview.chromium.org/8417017/diff/12001/chrome/chrome_browser.gypi
File chrome/chrome_browser.gypi (right):

http://codereview.chromium.org/8417017/diff/12001/chrome/chrome_browser.gypi#...
chrome/chrome_browser.gypi:4014:
'<(SHARED_INTERMEDIATE_DIR)/autofill_regex_constants.cc',
drive by: It's considered bad form to drop files directly into
<(SHARED_INTERMEDIATE_DIR)

http://crbug.com/97185 is for a bug filed on this anti-pattern, and
http://crbug.com/97186 for the related pattern that comes out of people trying
to solve 97185 

See also line 5202

http://codereview.chromium.org/8417017/diff/12001/webkit/glue/webkit_glue.gypi
File webkit/glue/webkit_glue.gypi (right):

http://codereview.chromium.org/8417017/diff/12001/webkit/glue/webkit_glue.gyp...
webkit/glue/webkit_glue.gypi:72: '<(SHARED_INTERMEDIATE_DIR)/webkit_version.h',
here as well