New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 616390 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2016
Cc:
EstimatedDays: ----
NextAction: ----
OS: iOS , Mac
Pri: 2
Type: Bug

Blocking:
issue 431177



Sign in to add a comment

GN: iOS and Mac should use get_concurrent_links.py

Project Member Reported by sdefresne@chromium.org, Jun 1 2016

Issue description

From a comment in https://codereview.chromium.org/2018553003/

> ... which made me just realize that the mac and ios toolchains aren't using
> get_concurrent_links.py either, which is probably bad.

So investigate whether this is bad and fix it if so.
 
Cc: thakis@chromium.org
Yes that's bad. From out/gn/build.ninja:

pool link_pool
  depth = 0

The manual doesn't say what a 0-sized pool does, but the implementation treats this as "don't limit parallelism". So we might run 50 links in parallel on the mac bots until this is fixed.
Blocking: 431177
Should we have the link tools err if the concurrent link is not set?
In gn you mean? Not sure; most people other than chromium probably don't build with -j200 and don't have links that need gigabytes.
Owner: dpranke@chromium.org
Status: Assigned (was: Available)
I should have a patch for this up this morning. I don't think we need to make the links fail.
Project Member

Comment 7 by bugdroid1@chromium.org, Jun 7 2016

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ec079268641d3d284884a5acf55d75af6065f549

commit ec079268641d3d284884a5acf55d75af6065f549
Author: dpranke <dpranke@chromium.org>
Date: Tue Jun 07 02:21:20 2016

Rework how MB and GN handle concurrent links.

The Mac and iOS GN bots don't currently limit the number of concurrent
links. The official continuous win bots aren't on MB because they can't
limit the number of concurrent links through MB.

This CL adds a 'concurrent_links' arg to GN, which calls the
get_concurrent_links script to get the appropriate default value
(making this consistent across platforms), and also adds a hack
to MB to translate a gyp_link_concurrent "GYP_DEFINE" to the
GYP_LINK_CONCURRENCY env var.

The CL also adds a test for this MB hack and the similar, already existing
LLVM_FORCE_HEAD_REVISION hack.

R=scottmg@chromium.org, brettw@chromium.org
BUG=602480, 611491,  616390 

Review-Url: https://codereview.chromium.org/2031233002
Cr-Commit-Position: refs/heads/master@{#398200}

[modify] https://crrev.com/ec079268641d3d284884a5acf55d75af6065f549/.gn
[add] https://crrev.com/ec079268641d3d284884a5acf55d75af6065f549/build/toolchain/concurrent_links.gni
[modify] https://crrev.com/ec079268641d3d284884a5acf55d75af6065f549/build/toolchain/gcc_toolchain.gni
[modify] https://crrev.com/ec079268641d3d284884a5acf55d75af6065f549/build/toolchain/mac/BUILD.gn
[modify] https://crrev.com/ec079268641d3d284884a5acf55d75af6065f549/build/toolchain/win/BUILD.gn
[modify] https://crrev.com/ec079268641d3d284884a5acf55d75af6065f549/tools/mb/mb.py
[modify] https://crrev.com/ec079268641d3d284884a5acf55d75af6065f549/tools/mb/mb_config.pyl
[modify] https://crrev.com/ec079268641d3d284884a5acf55d75af6065f549/tools/mb/mb_unittest.py

Status: Fixed (was: Assigned)

Sign in to add a comment