build: Tweak linker memory usage for android static build

build: Tweak linker memory usage for android static build

Manually measured using top that linker can use ~25G of memory linking
unit_tests_apk.

BUG=617429
Committed: https://crrev.com/3fbd5a693a788cf963ac8d009ef120239f56e9c2
Cr-Commit-Position: refs/heads/master@{#415072}

[boliu, 2016-08-26 17:24:20]

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

[boliu, 2016-08-26 17:24:20]

ptal

finally the original change for android

[Dirk Pranke, 2016-08-26 17:50:46]

https://codereview.chromium.org/2282943002/diff/1/build/toolchain/concurrent_...
File build/toolchain/concurrent_links.gni (right):

https://codereview.chromium.org/2282943002/diff/1/build/toolchain/concurrent_...
build/toolchain/concurrent_links.gni:32: _args = [ "--mem_per_link_gb=25" ]
is this number correct for both debug and release? it's a big change, so if we
want to land this as-is you need to make sure we make sure cycle time goes down,
not up :).

[boliu, 2016-08-26 17:58:16]

https://codereview.chromium.org/2282943002/diff/1/build/toolchain/concurrent_...
File build/toolchain/concurrent_links.gni (right):

https://codereview.chromium.org/2282943002/diff/1/build/toolchain/concurrent_...
build/toolchain/concurrent_links.gni:32: _args = [ "--mem_per_link_gb=25" ]
On 2016/08/26 17:50:45, Dirk Pranke wrote:
> is this number correct for both debug and release? it's a big change, so if we
> want to land this as-is you need to make sure we make sure cycle time goes
down,
> not up :).

this was measured when I copied the config from the official release bots, so
definitely not debug.

I thought debug+static build isn't a thing anymore though?

I'm guessing the next config that matters most is symbol_level, and 25G is
measured at symbol_level=2. I could measure 1 and 0 if you want?

[Dirk Pranke, 2016-08-26 17:59:45]

Description was changed from

==========
build: Tweak linker memory usage for android static build

Manually measured using top that linker can use ~25G of memory linking
unit_tests_apk.

BUG=617429
==========

to

==========
build: Tweak linker memory usage for android static build

Manually measured using top that linker can use ~25G of memory linking
unit_tests_apk.

BUG=617429
==========

[Dirk Pranke, 2016-08-26 18:01:12]

debug static is definitely still a thing, particularly on android.

We should definitely check the impact on both debug and release builds as well
as official, since this likely cuts the number of parallel links in half or two
thirds.

[boliu, 2016-08-27 00:19:53]

On 2016/08/26 18:01:12, Dirk Pranke wrote:
> debug static is definitely still a thing, particularly on android.
> 
> We should definitely check the impact on both debug and release builds as well
> as official, since this likely cuts the number of parallel links in half or
two
> thirds.

Re-did testing on static builds, using the resident set column from top
official: 0.025t
release: 0.025t
debug: 0.026t

So roughly the same across the board.

[boliu, 2016-08-27 00:27:35]

On 2016/08/27 00:19:53, boliu wrote:
> On 2016/08/26 18:01:12, Dirk Pranke wrote:
> > debug static is definitely still a thing, particularly on android.
> > 
> > We should definitely check the impact on both debug and release builds as
well
> > as official, since this likely cuts the number of parallel links in half or
> two
> > thirds.
> 
> Re-did testing on static builds, using the resident set column from top
> official: 0.025t
> release: 0.025t
> debug: 0.026t
> 
> So roughly the same across the board.

Oh, that was linking for unit_tests_apk, one of the largest targets.

[boliu, 2016-08-27 01:01:33]

On 2016/08/27 00:27:35, boliu wrote:
> On 2016/08/27 00:19:53, boliu wrote:
> > On 2016/08/26 18:01:12, Dirk Pranke wrote:
> > > debug static is definitely still a thing, particularly on android.
> > > 
> > > We should definitely check the impact on both debug and release builds as
> well
> > > as official, since this likely cuts the number of parallel links in half
or
> > two
> > > thirds.
> > 
> > Re-did testing on static builds, using the resident set column from top
> > official: 0.025t
> > release: 0.025t
> > debug: 0.026t
> > 
> > So roughly the same across the board.
> 
> Oh, that was linking for unit_tests_apk, one of the largest targets.

Tried debug+static+symbol_level=1 only uses 5.8G, so I'm limiting this to
symbol_level==2

[Dirk Pranke, 2016-08-28 17:59:45]

ok, lgtm.

[boliu, 2016-08-29 13:39:56]

The CQ bit was checked by boliu@chromium.org

[commit-bot: I haz the power, 2016-08-29 13:40:16]

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-08-29 13:46:18]

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

[commit-bot: I haz the power, 2016-08-29 13:46:19]

Exceeded global retry quota

[boliu, 2016-08-29 15:56:31]

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

[commit-bot: I haz the power, 2016-08-29 15:57:47]

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

[boliu, 2016-08-29 16:07:48]

Dirk can you look again? I added import for symbol_level, which apparently is
not included from the analyze step.

I'm not sure if the import is preferred over checking if symbol_level is
defined?

[Dirk Pranke, 2016-08-29 16:17:10]

lgtm.

import() is definitely correct.

defined() wouldn't even really do the right thing (I'm not sure if it would ever
return true w/o the import; I think it used to if you set symbol_level
explicitly in args.gn, but I thought we changed that because it was a source of
bugs like this).

[boliu, 2016-08-29 16:18:29]

The CQ bit was unchecked by boliu@chromium.org

[boliu, 2016-08-29 16:18:33]

The CQ bit was checked by boliu@chromium.org

[commit-bot: I haz the power, 2016-08-29 16:22:10]

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

[dsansome, 2016-08-30 01:46:12]

The CQ bit was unchecked by dsansome@chromium.org

[dsansome, 2016-08-30 03:54:34]

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

[dsansome, 2016-08-30 03:54:35]

(I unset the commit bit on this change to try fix a stuck CQ -
https://bugs.chromium.org/p/chromium/issues/detail?id=642077.  The issue is
fixed now so I'm sending this to CQ again for you)

[dsansome, 2016-08-30 03:55:31]

The CQ bit was checked by dsansome@chromium.org

[commit-bot: I haz the power, 2016-08-30 03:57:33]

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-08-30 04:06:01]

Description was changed from

==========
build: Tweak linker memory usage for android static build

Manually measured using top that linker can use ~25G of memory linking
unit_tests_apk.

BUG=617429
==========

to

==========
build: Tweak linker memory usage for android static build

Manually measured using top that linker can use ~25G of memory linking
unit_tests_apk.

BUG=617429
==========

[commit-bot: I haz the power, 2016-08-30 04:06:10]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-08-30 04:09:26]

Description was changed from

==========
build: Tweak linker memory usage for android static build

Manually measured using top that linker can use ~25G of memory linking
unit_tests_apk.

BUG=617429
==========

to

==========
build: Tweak linker memory usage for android static build

Manually measured using top that linker can use ~25G of memory linking
unit_tests_apk.

BUG=617429

Committed: https://crrev.com/3fbd5a693a788cf963ac8d009ef120239f56e9c2
Cr-Commit-Position: refs/heads/master@{#415072}
==========

[commit-bot: I haz the power, 2016-08-30 04:09:27]

Patchset 3 (id:??) landed as
https://crrev.com/3fbd5a693a788cf963ac8d009ef120239f56e9c2
Cr-Commit-Position: refs/heads/master@{#415072}