get_concurrent_links.py: give more RAM per job in LTO builds.

get_concurrent_links.py: give more RAM per job in LTO builds.

This is required for launching Control Flow Integrity on Linux x86-64,
as it uses LTO builds, which use significantly more memory during
link phase.

Failing to do that lead me to see this OOM error message on the bot:
https://chromegw.corp.google.com/i/official.desktop/builders/precise64/builds/253

BUG=565162, 464797
Committed: https://crrev.com/8b2f896b9b2083ced0a42879ef6ea427a3cd56d4
Cr-Commit-Position: refs/heads/master@{#363137}

[krasin, 2015-12-03 23:16:29]

krasin@google.com changed reviewers:
+ dpranke@google.com

[krasin, 2015-12-03 23:16:42]

Description was changed from

==========
get_concurrent_links.py: give more RAM per job in LTO builds.

This is required for launching Control Flow Integrity on Linux x86-64,
as it uses LTO builds, which use significantly more memory during
link phase.

Failing to do that lead me to see this OOM error message on the bot:
https://chromegw.corp.google.com/i/official.desktop/builders/precise64/builds...

BUG=565162,464797
==========

to

==========
get_concurrent_links.py: give more RAM per job in LTO builds.

This is required for launching Control Flow Integrity on Linux x86-64,
as it uses LTO builds, which use significantly more memory during
link phase.

Failing to do that lead me to see this OOM error message on the bot:
https://chromegw.corp.google.com/i/official.desktop/builders/precise64/builds...

BUG=565162,464797
==========

[krasin, 2015-12-03 23:18:45]

https://codereview.chromium.org/1492843006/diff/20001/build/toolchain/gcc_too...
File build/toolchain/gcc_toolchain.gni (right):

https://codereview.chromium.org/1492843006/diff/20001/build/toolchain/gcc_too...
build/toolchain/gcc_toolchain.gni:13: concurrent_links =
I am not totally sure, if it's an idiomatic GN code.
I would probably prefer something like

concurrent_links = exec_script("get_concurrent_links.py", ["--lto=" + is_cfi],
"value")

if it's possible. Suggestions are welcome.

[Dirk Pranke, 2015-12-03 23:19:08]

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

[Dirk Pranke, 2015-12-03 23:19:09]

lgtm w/ nits. thanks!

https://codereview.chromium.org/1492843006/diff/20001/build/toolchain/get_con...
File build/toolchain/get_concurrent_links.py (right):

https://codereview.chromium.org/1492843006/diff/20001/build/toolchain/get_con...
build/toolchain/get_concurrent_links.py:14: def
GetDefaultConcurrentLinks(is_lto=False):
nit: don't specify a default argument, since it'll always be passed in. I might
also rename it to _GetDefault to make it a bit clearer that the function is
private to the file.

[Dirk Pranke, 2015-12-03 23:20:22]

On 2015/12/03 23:18:45, krasin wrote:
>
https://codereview.chromium.org/1492843006/diff/20001/build/toolchain/gcc_too...
> File build/toolchain/gcc_toolchain.gni (right):
> 
>
https://codereview.chromium.org/1492843006/diff/20001/build/toolchain/gcc_too...
> build/toolchain/gcc_toolchain.gni:13: concurrent_links =
> I am not totally sure, if it's an idiomatic GN code.
> I would probably prefer something like
> 
> concurrent_links = exec_script("get_concurrent_links.py", ["--lto=" + is_cfi],
> "value")
> 
> if it's possible. Suggestions are welcome.

I'd be surprised if that worked either on the GN or python side :).

What you wrote is totally fine.

[krasin, 2015-12-03 23:22:20]

https://codereview.chromium.org/1492843006/diff/20001/build/toolchain/gcc_too...
File build/toolchain/gcc_toolchain.gni (right):

https://codereview.chromium.org/1492843006/diff/20001/build/toolchain/gcc_too...
build/toolchain/gcc_toolchain.gni:13: concurrent_links =
On 2015/12/03 23:18:45, krasin wrote:
> I am not totally sure, if it's an idiomatic GN code.
> I would probably prefer something like
> 
> concurrent_links = exec_script("get_concurrent_links.py", ["--lto=" + is_cfi],
> "value")
> 
> if it's possible. Suggestions are welcome.

Acknowledged.

https://codereview.chromium.org/1492843006/diff/20001/build/toolchain/get_con...
File build/toolchain/get_concurrent_links.py (right):

https://codereview.chromium.org/1492843006/diff/20001/build/toolchain/get_con...
build/toolchain/get_concurrent_links.py:14: def
GetDefaultConcurrentLinks(is_lto=False):
On 2015/12/03 23:19:08, Dirk Pranke wrote:
> nit: don't specify a default argument, since it'll always be passed in. I
might
> also rename it to _GetDefault to make it a bit clearer that the function is
> private to the file.

Done.

[krasin, 2015-12-03 23:22:31]

The CQ bit was checked by krasin@google.com

[krasin, 2015-12-03 23:22:31]

The patchset sent to the CQ was uploaded after l-g-t-m from dpranke@chromium.org
Link to the patchset: https://codereview.chromium.org/1492843006/#ps40001
(title: "nits")

[commit-bot: I haz the power, 2015-12-03 23:24:34]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1492843006/40001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1492843006/40001

[krasin, 2015-12-03 23:25:04]

The CQ bit was unchecked by krasin@google.com

[krasin, 2015-12-03 23:27:22]

The CQ bit was checked by krasin@google.com

[krasin, 2015-12-03 23:27:22]

The patchset sent to the CQ was uploaded after l-g-t-m from dpranke@chromium.org
Link to the patchset: https://codereview.chromium.org/1492843006/#ps60001
(title: "win")

[commit-bot: I haz the power, 2015-12-03 23:27:50]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1492843006/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1492843006/60001

[krasin, 2015-12-03 23:28:19]

Dirk,

I have also added the same change to the Windows toolchain BUILD.gn. I think
this is what you meant in the comments to the original CL.

[Dirk Pranke, 2015-12-03 23:42:00]

On 2015/12/03 23:28:19, krasin wrote:
> Dirk,
> 
> I have also added the same change to the Windows toolchain BUILD.gn. I think
> this is what you meant in the comments to the original CL.

I'm not sure which comments (or which CL) you're referring to. Do the official 
win builders have the same issue? They aren't using GN yet, so we would need
to make different changes for them (in GYP).

[krasin, 2015-12-03 23:49:51]

On 2015/12/03 23:42:00, Dirk Pranke wrote:
> On 2015/12/03 23:28:19, krasin wrote:
> > Dirk,
> > 
> > I have also added the same change to the Windows toolchain BUILD.gn. I think
> > this is what you meant in the comments to the original CL.
> 
> I'm not sure which comments (or which CL) you're referring to. Do the official

> win builders have the same issue? They aren't using GN yet, so we would need
> to make different changes for them (in GYP).

The Windows toolchain update is not needed right now (for the reasons you
specified). If you feel that we should not have unused code like that, I am fine
to revert the Windows part of the CL.

[Dirk Pranke, 2015-12-03 23:52:02]

I'm fine w/ leaving this in for consistency, since my understanding is that we
will
want CFI on Linux sooner or later.

[Dirk Pranke, 2015-12-03 23:52:20]

On 2015/12/03 23:52:02, Dirk Pranke wrote:
> I'm fine w/ leaving this in for consistency, since my understanding is that we
> will
> want CFI on Linux sooner or later.

argh, make that "will want CFI on Windows sooner or later".

[krasin, 2015-12-03 23:55:43]

On 2015/12/03 23:52:20, Dirk Pranke wrote:
> On 2015/12/03 23:52:02, Dirk Pranke wrote:
> > I'm fine w/ leaving this in for consistency, since my understanding is that
we
> > will
> > want CFI on Linux sooner or later.
> 
> argh, make that "will want CFI on Windows sooner or later".

Thanks!
In fact, CFI on Windows already exists, but we have not yet started to put it
into Chrome.

[commit-bot: I haz the power, 2015-12-04 01:40:23]

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

[commit-bot: I haz the power, 2015-12-04 01:40:26]

Try jobs failed on following builders:
  android_arm64_dbg_recipe on tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  android_chromium_gn_compile_dbg on tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  android_chromium_gn_compile_rel on tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  android_clang_dbg_recipe on tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)
  android_compile_dbg on tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  cast_shell_android on tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  cast_shell_linux on tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  chromeos_amd64-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromeos_daisy_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  chromeos_x86-generic_chromium_compile_only_ng on tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_android_rel_ng on tryserver.chromium.linux (JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_compile_dbg_ng on tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_chromeos_ozone_rel_ng on tryserver.chromium.linux
(JOB_TIMED_OUT, no build URL)
  linux_chromium_compile_dbg_32_ng on tryserver.chromium.linux (JOB_TIMED_OUT,
no build URL)
  linux_chromium_compile_dbg_ng on tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_TIMED_OUT, no
build URL)
  linux_chromium_rel_ng on tryserver.chromium.linux (JOB_TIMED_OUT, no build
URL)

[krasin, 2015-12-04 04:38:40]

The CQ bit was checked by krasin@google.com

[commit-bot: I haz the power, 2015-12-04 04:39:26]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1492843006/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1492843006/60001

[commit-bot: I haz the power, 2015-12-04 04:45:23]

Description was changed from

==========
get_concurrent_links.py: give more RAM per job in LTO builds.

This is required for launching Control Flow Integrity on Linux x86-64,
as it uses LTO builds, which use significantly more memory during
link phase.

Failing to do that lead me to see this OOM error message on the bot:
https://chromegw.corp.google.com/i/official.desktop/builders/precise64/builds...

BUG=565162,464797
==========

to

==========
get_concurrent_links.py: give more RAM per job in LTO builds.

This is required for launching Control Flow Integrity on Linux x86-64,
as it uses LTO builds, which use significantly more memory during
link phase.

Failing to do that lead me to see this OOM error message on the bot:
https://chromegw.corp.google.com/i/official.desktop/builders/precise64/builds...

BUG=565162,464797
==========

[commit-bot: I haz the power, 2015-12-04 04:45:24]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-12-04 04:46:05]

Description was changed from

==========
get_concurrent_links.py: give more RAM per job in LTO builds.

This is required for launching Control Flow Integrity on Linux x86-64,
as it uses LTO builds, which use significantly more memory during
link phase.

Failing to do that lead me to see this OOM error message on the bot:
https://chromegw.corp.google.com/i/official.desktop/builders/precise64/builds...

BUG=565162,464797
==========

to

==========
get_concurrent_links.py: give more RAM per job in LTO builds.

This is required for launching Control Flow Integrity on Linux x86-64,
as it uses LTO builds, which use significantly more memory during
link phase.

Failing to do that lead me to see this OOM error message on the bot:
https://chromegw.corp.google.com/i/official.desktop/builders/precise64/builds...

BUG=565162,464797

Committed: https://crrev.com/8b2f896b9b2083ced0a42879ef6ea427a3cd56d4
Cr-Commit-Position: refs/heads/master@{#363137}
==========

[commit-bot: I haz the power, 2015-12-04 04:46:06]

Patchset 4 (id:??) landed as
https://crrev.com/8b2f896b9b2083ced0a42879ef6ea427a3cd56d4
Cr-Commit-Position: refs/heads/master@{#363137}