Disable compilation symbols on goma builds

Disable compilation symbols on goma builds

Goma builds necessarily don't use PDBs. That means that symbols have to be put in .obj files. This means that symbols get stored redundantly, which makes the job of the linker much harder - greatly increased linker working sets and greatly increased link times.

gyp deals with this by disabling symbol generation in the compile stage for goma builds. gn has not been doing this, which has made gn goma builds *significantly* slower, unless symbol_level is explicitly set to 1.

gyp has an override switch (win_z7) but it appears that nobody uses it so for now I am leaving it unimplemented in gn.

BUG=630074
Committed: https://crrev.com/615601219acf13abf60ac400a5c07333da6fe897
Cr-Commit-Position: refs/heads/master@{#407251}

[brucedawson, 2016-07-22 17:31:19]

The CQ bit was checked by brucedawson@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-07-22 17:32:03]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[brettw, 2016-07-22 17:40:55]

brettw@chromium.org changed reviewers:
+ brettw@chromium.org

[brettw, 2016-07-22 17:40:55]

lgtm

[commit-bot: I haz the power, 2016-07-22 17:42:35]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-07-22 17:42:35]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_FAILED,
https://build.chromium.org/p/tryserver.chromium.android/builders/android_arm6...)

[brucedawson, 2016-07-22 17:43:46]

brucedawson@chromium.org changed reviewers:
+ dpranke@chromium.org

[brucedawson, 2016-07-22 17:43:46]

Adding Dirk because I want to make sure I'm not missing anything.

This is a ridiculously simple change but crucial. This will fix (I believe) the
build-speed regressions on all goma builds. This CL basically adds this logic to
the gn builds so that gyp no longer has an artificial advantage:

      ['OS=="win" and use_goma==1', {
        # goma doesn't support pch yet.
        'chromium_win_pch': 0,
        # goma doesn't support PDB yet, so win_z7=1 or fastbuild=1.
        'conditions': [
          ['win_z7==0 and fastbuild==0', {
            'fastbuild': 1,
          }],
        ],
      }],

I haven't put the win_z7 override in - I don't think anybody uses it.

This could also be done at the config("default_symbols") level instead - any
strong preferences?

[brucedawson, 2016-07-22 17:48:24]

Note that this doesn't affect
https://build.chromium.org/p/chromium/builders/Win/builds/45495 because it
explicitly requests symbol_level = 1.

[brucedawson, 2016-07-22 20:18:54]

Description was changed from

==========
Disable compilation symbols on goma builds

Goma builds necessarily don't use PDBs. That means that symbols have to be put
in .obj files. This means that symbols get stored redundantly, which makes the
job of the linker much harder - greatly increased linker working sets and
greatly increased link times.

gyp deals with this by disabling symbol generation in the compile stage for goma
builds. gn has not been doing this, which has made gn goma builds
*significantly* slower.

gyp has an override switch (win_z7) but it appears that nobody uses it so for
now I am leaving it unimplemented in gn.

Need to tag this with the bug for slow build times also...

BUG=630074
==========

to

==========
Disable compilation symbols on goma builds

Goma builds necessarily don't use PDBs. That means that symbols have to be put
in .obj files. This means that symbols get stored redundantly, which makes the
job of the linker much harder - greatly increased linker working sets and
greatly increased link times.

gyp deals with this by disabling symbol generation in the compile stage for goma
builds. gn has not been doing this, which has made gn goma builds
*significantly* slower, unless symbol_level is explicitly set to 1.

gyp has an override switch (win_z7) but it appears that nobody uses it so for
now I am leaving it unimplemented in gn.

BUG=630074
==========

[brucedawson, 2016-07-22 20:19:01]

The CQ bit was checked by brucedawson@chromium.org

[commit-bot: I haz the power, 2016-07-22 20:19:29]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-07-22 20:33:50]

Description was changed from

==========
Disable compilation symbols on goma builds

Goma builds necessarily don't use PDBs. That means that symbols have to be put
in .obj files. This means that symbols get stored redundantly, which makes the
job of the linker much harder - greatly increased linker working sets and
greatly increased link times.

gyp deals with this by disabling symbol generation in the compile stage for goma
builds. gn has not been doing this, which has made gn goma builds
*significantly* slower, unless symbol_level is explicitly set to 1.

gyp has an override switch (win_z7) but it appears that nobody uses it so for
now I am leaving it unimplemented in gn.

BUG=630074
==========

to

==========
Disable compilation symbols on goma builds

Goma builds necessarily don't use PDBs. That means that symbols have to be put
in .obj files. This means that symbols get stored redundantly, which makes the
job of the linker much harder - greatly increased linker working sets and
greatly increased link times.

gyp deals with this by disabling symbol generation in the compile stage for goma
builds. gn has not been doing this, which has made gn goma builds
*significantly* slower, unless symbol_level is explicitly set to 1.

gyp has an override switch (win_z7) but it appears that nobody uses it so for
now I am leaving it unimplemented in gn.

BUG=630074
==========

[commit-bot: I haz the power, 2016-07-22 20:33:54]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-07-22 20:37:12]

Description was changed from

==========
Disable compilation symbols on goma builds

Goma builds necessarily don't use PDBs. That means that symbols have to be put
in .obj files. This means that symbols get stored redundantly, which makes the
job of the linker much harder - greatly increased linker working sets and
greatly increased link times.

gyp deals with this by disabling symbol generation in the compile stage for goma
builds. gn has not been doing this, which has made gn goma builds
*significantly* slower, unless symbol_level is explicitly set to 1.

gyp has an override switch (win_z7) but it appears that nobody uses it so for
now I am leaving it unimplemented in gn.

BUG=630074
==========

to

==========
Disable compilation symbols on goma builds

Goma builds necessarily don't use PDBs. That means that symbols have to be put
in .obj files. This means that symbols get stored redundantly, which makes the
job of the linker much harder - greatly increased linker working sets and
greatly increased link times.

gyp deals with this by disabling symbol generation in the compile stage for goma
builds. gn has not been doing this, which has made gn goma builds
*significantly* slower, unless symbol_level is explicitly set to 1.

gyp has an override switch (win_z7) but it appears that nobody uses it so for
now I am leaving it unimplemented in gn.

BUG=630074

Committed: https://crrev.com/615601219acf13abf60ac400a5c07333da6fe897
Cr-Commit-Position: refs/heads/master@{#407251}
==========

[commit-bot: I haz the power, 2016-07-22 20:37:13]

Patchset 1 (id:??) landed as
https://crrev.com/615601219acf13abf60ac400a5c07333da6fe897
Cr-Commit-Position: refs/heads/master@{#407251}

[Dirk Pranke, 2016-07-22 21:02:56]

https://codereview.chromium.org/2174873002/diff/1/build/config/compiler/BUILD.gn
File build/config/compiler/BUILD.gn (right):

https://codereview.chromium.org/2174873002/diff/1/build/config/compiler/BUILD...
build/config/compiler/BUILD.gn:1493: cflags = []
The implication of this change is that you can no longer get a full symbol build
w/ goma, right? 

Maybe it would've been better to change it so that if use_goma=true,
symbol_level defaults to 1 ?

That would also make things more consistent w/ what we do on the builders, which
would be nice.

It also looks like we might need /z7 for SyzygyASAN (we'll have to check w/
Sébastien).

[brucedawson, 2016-07-22 21:42:06]

> The implication of this change is that you can no longer get a full symbol
build
> w/ goma, right? 

Correct. My understanding is that this was also true for gyp, unless win_z7 was
used, and I could find no uses of win_z7.

[Dirk Pranke, 2016-07-22 22:03:34]

On 2016/07/22 21:42:06, brucedawson wrote:
> > The implication of this change is that you can no longer get a full symbol
> build
> > w/ goma, right? 
> 
> Correct. My understanding is that this was also true for gyp, unless win_z7
was
> used, and I could find no uses of win_z7.

Right, but you could at least do it manually if you really wanted. At any rate,
that 
by itself isn't that important, but I think I like my suggestion a bit better
than the
way you did things (it at least cleans up a bunch of stuff in the MB configs).

Any objection to me switching things to my approach?

[brucedawson, 2016-07-22 22:42:17]

> Any objection to me switching things to my approach?

No objections. I had considered doing it that way but ended up with a weak
preference for my solution.

I would consider following gyp's lead and adding a win_z7 option to override
use_goma-implies-symbol_level==1

[Dirk Pranke, 2016-07-22 22:45:30]

On 2016/07/22 22:42:17, brucedawson wrote:
> > Any objection to me switching things to my approach?
> 
> No objections. I had considered doing it that way but ended up with a weak
> preference for my solution.
> 
> I would consider following gyp's lead and adding a win_z7 option to override
> use_goma-implies-symbol_level==1

Yup, like I said I think we might need it for SyzygyASAN anyway.