Commit cf6352bf authored by Moe Ahmadi's avatar Moe Ahmadi Committed by Commit Bot

Revert "build: In LTO links, run just one link at once, but give it all cores"

This reverts commit 174d9faa.

Reason for revert: Breaking https://ci.chromium.org/p/chromium/builders/ci/chromeos-amd64-generic-cfi-thin-lto-rel

See:
https://ci.chromium.org/p/chromium/builders/ci/chromeos-amd64-generic-cfi-thin-lto-rel/24710

Original change's description:
> build: In LTO links, run just one link at once, but give it all cores
>
> Normally, we allow running several links in parallel if there's enough
> memory, since links tend to be memory- and io-bound, not compute bound.
> However, in LTO builds the linker does codegen for every linked
> translation unit, which needs a lot of CPU.
>
> Previously, we used to hardcode 8 threads per link for thinlto builds,
> which makes builds needlessly slow on machines with many cores.
>
> Instead, automatically use all cores for a thinlto link, but only run
> one link in parallel at once.
>
> chromeos-chrome-9999.ebuild in the Chrome OS build and link_jobs_32 in
> mb_config.pyl currently manually set the number of threads per lto link
> to the number of cores. Once this change here is in, we should stop
> explicitly setting the arg there and then we can turn this from a gn arg
> to a regular variable that's not settable in args.gn
>
> Bug: 1132930
> Change-Id: If2fda3e38b2b1827beb2da844f024f52211210db
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2464782
> Reviewed-by: Hans Wennborg <hans@chromium.org>
> Commit-Queue: Nico Weber <thakis@chromium.org>
> Auto-Submit: Nico Weber <thakis@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#816129}

TBR=thakis@chromium.org,hans@chromium.org

Change-Id: Ic1d381681e00dbebd576ec44119d426518f4b778
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 1132930
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2466261Reviewed-by: default avatarMoe Ahmadi <mahmadi@chromium.org>
Commit-Queue: Moe Ahmadi <mahmadi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#816208}
parent b647ada1
......@@ -621,7 +621,7 @@ config("compiler") {
# files on Windows (https://crbug.com/871962).
ldflags += [
"/opt:lldlto=" + lto_opt_level,
"/opt:lldltojobs=all",
"/opt:lldltojobs=" + max_jobs_per_link,
# Experimentally determined to yield a reasonable trade-off between
# build time, run-time performance, and binary size.
......@@ -637,11 +637,10 @@ config("compiler") {
# usage in crbug.com/1038040. Note this will increase build time in
# Chrome OS.
# In ThinLTO builds, we run at most one link process at a time,
# and let it use all cores.
# TODO(thakis): Check if '=0' (that is, number of cores, instead
# of "all" which means number of hardware threads) is faster.
ldflags += [ "-Wl,--thinlto-jobs=all" ]
# Limit the parallelism to avoid too aggressive competition between
# linker jobs. This is still suboptimal to a potential dynamic
# resource allocation scheme, but should be good enough.
ldflags += [ "-Wl,--thinlto-jobs=" + max_jobs_per_link ]
# Limit the size of the ThinLTO cache to the lesser of 10% of
# available disk space, 10GB and 100000 files.
......
......@@ -73,8 +73,8 @@ declare_args() {
# If true, use Goma for ThinLTO code generation where applicable.
use_goma_thin_lto = false
# Ignored, only exists temporarily for backwards compat.
# TODO(thakis): Remove this once bots no longer set it.
# Limit the number of jobs (threads/processes) the linker is allowed
# to use (for linkers that support this).
max_jobs_per_link = 8
# Whether we're using a sample profile collected on an architecture different
......
......@@ -76,24 +76,16 @@ if (concurrent_links == -1) {
# so that we can compute better values.
_command_dict = exec_script("get_concurrent_links.py", _args, "scope")
if (use_thin_lto) {
concurrent_links = 1
concurrent_links_logs =
[ "thinlto -- only one link at once but every link can use all cores" ]
} else {
concurrent_links = _command_dict.primary_pool_size
concurrent_links_logs = _command_dict.explanation
if (_command_dict.secondary_pool_size >= concurrent_links) {
# Have R8 / Lint share the link pool unless we would safely get more
# concurrency out of using a separate one.
# On low-RAM machines, this allows an apk's native library to link at the
# same time as its java is optimized with R8.
java_cmd_pool_size = _command_dict.secondary_pool_size
}
concurrent_links = _command_dict.primary_pool_size
concurrent_links_logs = _command_dict.explanation
if (_command_dict.secondary_pool_size >= concurrent_links) {
# Have R8 / Lint share the link pool unless we would safely get more
# concurrency out of using a separate one.
# On low-RAM machines, this allows an apk's native library to link at the
# same time as its java is optimized with R8.
java_cmd_pool_size = _command_dict.secondary_pool_size
}
} else {
assert(!use_thin_lto, "can't explicitly set concurrent_links with thinlto")
concurrent_links_logs =
[ "concurrent_links set by GN arg (value=$concurrent_links)" ]
}
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment