Skip to content

Commit

Permalink
Rework how MB and GN handle concurrent links.
Browse files Browse the repository at this point in the history
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}
  • Loading branch information
dpranke authored and Commit bot committed Jun 7, 2016
1 parent 7005a6e commit ec07926
Show file tree
Hide file tree
Showing 8 changed files with 102 additions and 20 deletions.
2 changes: 1 addition & 1 deletion .gn
Original file line number Diff line number Diff line change
Expand Up @@ -241,7 +241,7 @@ exec_script_whitelist = [
"//build/config/win/visual_studio_version.gni",
"//build/gn_helpers.py",
"//build/gypi_to_gn.py",
"//build/toolchain/gcc_toolchain.gni",
"//build/toolchain/concurrent_links.gni",
"//build/toolchain/mac/BUILD.gn",
"//build/toolchain/nacl/BUILD.gn",
"//build/toolchain/win/BUILD.gn",
Expand Down
31 changes: 31 additions & 0 deletions build/toolchain/concurrent_links.gni
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Copyright 2016 The Chromium Authors. All rights reserved.
# Use of this source code is governed by a BSD-style license that can be
# found in the LICENSE file.

# This file should only be imported from files that define toolchains.
# There's no way to enforce this exactly, but all toolchains are processed
# in the context of the default_toolchain, so we can at least check for that.
assert(current_toolchain == default_toolchain)

import("//build/config/sanitizers/sanitizers.gni")
import("//build/toolchain/toolchain.gni")

declare_args() {
# Limit the number of concurrent links; we often want to run fewer
# links at once than we do compiles, because linking is memory-intensive.
# The default to use varies by platform and by the amount of memory
# available, so we call out to a script to get the right value.
concurrent_links = -1
}

if (concurrent_links == -1) {
if (allow_posix_link_time_opt || is_cfi) {
_args = [ "--lto" ]
} else {
_args = []
}

# TODO(crbug.com/617429) Pass more build configuration info to the script
# so that we can compute better values.
concurrent_links = exec_script("get_concurrent_links.py", _args, "value")
}
13 changes: 2 additions & 11 deletions build/toolchain/gcc_toolchain.gni
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,7 @@ import("//build/config/sanitizers/sanitizers.gni")
import("//build/toolchain/cc_wrapper.gni")
import("//build/toolchain/goma.gni")
import("//build/toolchain/toolchain.gni")

# "concurrent_links" is a toolchain variable. By computing it here rather than
# inside the toolchain, the exec_script will only get run once rather than
# each time the toolchain template is invoked.
if (allow_posix_link_time_opt || is_cfi) {
concurrent_links_ =
exec_script("get_concurrent_links.py", [ "--lto" ], "value")
} else {
concurrent_links_ = exec_script("get_concurrent_links.py", [], "value")
}
import("//build/toolchain/concurrent_links.gni")

# This template defines a toolchain for something that works like gcc
# (including clang).
Expand Down Expand Up @@ -118,7 +109,7 @@ template("gcc_toolchain") {
assert(defined(invoker.toolchain_os),
"gcc_toolchain() must specify a \"toolchain_os\"")

concurrent_links = concurrent_links_
# concurrent_links is picked up from the declare_arg().

if (defined(invoker.cc_wrapper)) {
cc_wrapper = invoker.cc_wrapper
Expand Down
3 changes: 3 additions & 0 deletions build/toolchain/mac/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ assert(host_os == "mac")

import("//build/toolchain/goma.gni")
import("//build/toolchain/toolchain.gni")
import("//build/toolchain/concurrent_links.gni")

if (use_goma) {
goma_prefix = "$goma_dir/gomacc "
Expand All @@ -40,6 +41,8 @@ template("mac_toolchain") {
assert(defined(invoker.toolchain_os),
"mac_toolchain() must specify a \"toolchain_os\"")

# concurrent_links is picked up from the declare_arg().

# We can't do string interpolation ($ in strings) on things with dots in
# them. To allow us to use $cc below, for example, we create copies of
# these values in our scope.
Expand Down
5 changes: 2 additions & 3 deletions build/toolchain/win/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import("//build/config/sanitizers/sanitizers.gni")
import("//build/config/win/visual_studio_version.gni")
import("//build/toolchain/goma.gni")
import("//build/toolchain/toolchain.gni")
import("//build/toolchain/concurrent_links.gni")

# Should only be running on Windows.
assert(is_win)
Expand All @@ -24,9 +25,6 @@ if (use_goma) {
goma_prefix = ""
}

# This value will be inherited in the toolchain below.
concurrent_links = exec_script("../get_concurrent_links.py", [], "value")

# Copy the VS runtime DLL for the default toolchain to the root build directory
# so things will run.
if (current_toolchain == default_toolchain) {
Expand All @@ -50,6 +48,7 @@ if (current_toolchain == default_toolchain) {
# environment: File name of environment file.
template("msvc_toolchain") {
if (defined(invoker.concurrent_links)) {
# concurrent_links is picked up from the declare_arg() otherwise.
concurrent_links = invoker.concurrent_links
}

Expand Down
16 changes: 14 additions & 2 deletions tools/mb/mb.py
Original file line number Diff line number Diff line change
Expand Up @@ -1127,9 +1127,19 @@ def GYPCmd(self, output_dir, vals):
# to get rid of the arg and add the old var in, instead.
# See crbug.com/582737 for more on this. This can hopefully all
# go away with GYP.
if 'llvm_force_head_revision=1' in gyp_defines:
m = re.search('llvm_force_head_revision=1\s*', gyp_defines)
if m:
env['LLVM_FORCE_HEAD_REVISION'] = '1'
gyp_defines = gyp_defines.replace('llvm_force_head_revision=1', '')
gyp_defines = gyp_defines.replace(m.group(0), '')

# This is another terrible hack to work around the fact that
# GYP sets the link concurrency to use via the GYP_LINK_CONCURRENCY
# environment variable, and not via a proper GYP_DEFINE. See
# crbug.com/611491 for more on this.
m = re.search('gyp_link_concurrency=(\d+)(\s*)', gyp_defines)
if m:
env['GYP_LINK_CONCURRENCY'] = m.group(1)
gyp_defines = gyp_defines.replace(m.group(0), '')

env['GYP_GENERATORS'] = 'ninja'
if 'GYP_CHROMIUM_NO_ACTION' in env:
Expand Down Expand Up @@ -1323,6 +1333,8 @@ def print_env(var):

print_env('GYP_CROSSCOMPILE')
print_env('GYP_DEFINES')
print_env('GYP_LINK_CONCURRENCY')
print_env('LLVM_FORCE_HEAD_REVISION')

if cmd[0] == self.executable:
cmd = ['python'] + cmd[1:]
Expand Down
17 changes: 14 additions & 3 deletions tools/mb/mb_config.pyl
Original file line number Diff line number Diff line change
Expand Up @@ -640,9 +640,9 @@
'precise64 beta': 'gn_official',
'precise64 stable': 'gn_official',
'precise64 trunk': 'gn_official',
'win beta': 'gyp_official',
'win stable': 'gyp_official',
'win trunk': 'gyp_official',
'win beta': 'gyp_official_six_concurrent_links',
'win stable': 'gyp_official_six_concurrent_links',
'win trunk': 'gyp_official_six_concurrent_links',
},

'tryserver.blink': {
Expand Down Expand Up @@ -1328,6 +1328,10 @@
'gyp', 'official', 'syzyasan',
],

'gyp_official_six_concurrent_links': [
'gyp', 'official', 'six_concurrent_links',
],

# TODO(crbug.com/595947) - figure out how to handle PGO, which needs
# to invoke GYP/GN twice, with two different sets of flags, apparently.
'gyp_official_winpgo': [
Expand Down Expand Up @@ -2233,6 +2237,13 @@
'mixins': ['shared', 'release', 'goma']
},

'six_concurrent_links': {
# TODO(crbug.com/611491) Adjust the get_concurrent_links script
# to be more conservative so that we don't need this.
'gn_args': 'concurrent_links=6',
'gyp_defines': 'gyp_link_concurrency=6',
},

'static': {
'gn_args': 'is_component_build=false',
'gyp_defines': 'component=static_library',
Expand Down
35 changes: 35 additions & 0 deletions tools/mb/mb_unittest.py
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,31 @@ def close(self):
}
"""


GYP_HACKS_CONFIG = """\
{
'masters': {
'chromium': {},
'fake_master': {
'fake_builder': 'fake_config',
},
},
'configs': {
'fake_config': ['fake_mixin'],
},
'mixins': {
'fake_mixin': {
'type': 'gyp',
'gn_args': '',
'gyp_defines':
('foo=bar llvm_force_head_revision=1 '
'gyp_link_concurrency=1 baz=1'),
},
},
}
"""


class UnitTest(unittest.TestCase):
def fake_mbw(self, files=None, win32=False):
mbw = FakeMBW(win32=win32)
Expand Down Expand Up @@ -473,6 +498,16 @@ def test_bad_validate(self):
mbw.files[mbw.default_config] = TEST_BAD_CONFIG
self.check(['validate'], mbw=mbw, ret=1)

def test_gyp_env_hacks(self):
mbw = self.fake_mbw()
mbw.files[mbw.default_config] = GYP_HACKS_CONFIG
self.check(['lookup', '-c', 'fake_config'], mbw=mbw,
ret=0,
out=("GYP_DEFINES='foo=bar baz=1'\n"
"GYP_LINK_CONCURRENCY=1\n"
"LLVM_FORCE_HEAD_REVISION=1\n"
"python build/gyp_chromium -G output_dir=_path_\n"))


if __name__ == '__main__':
unittest.main()

0 comments on commit ec07926

Please sign in to comment.