Commit 1cda6e78 authored by Hans Wennborg's avatar Hans Wennborg Committed by Chromium LUCI CQ

[build] Only apply thin_lto_enable_optimizations to select targets

Link-time optimization enables greater performance, but leads to
significantly longer link times. This change turns off optimization
in the link step except for select targets such as the actual browser
executable/shared lib.

Source files are still compiled with the usual optimization flags, and
the ThinLTO mechanism is still used for all links; this only affects
whether optimizations are also performed in the link step or not.

The motivation is to speed up builds with thin_lto_enable_optimizations
so that this can be enabled in official builds on all targets.

Currently only ChromeOS and Android use thin_lto_enable_optimizations,
and in terms of production builds this is a no-op since the optimization
is applied to the shipping binaries as before. However it means that tests
and auxiliary binaries won't be optimized during the link step.


In terms of build speed, this reduces the link time of
v8_context_snapshot_generator in a Linux build with
thin_lto_enable_optimizations from 173 to 39 CPU minutes.

On Windows, the link time of the same target goes from 2m50 to 1m40
(wall-clock time). Linking unit_tests goes from 8m50 to 5m40.


Build timings (best of two) of official "all" builds on my Win
workstation:

- Without ThinLTO:                25m
- With ThinLTO opts:            4h36m
- With ThinLTO opts+this patch: 2h39m

With the ThinLTO cache enabled (requires fixing crbug.com/871962):
- With ThinLTO opts+this patch: 1h52m

Hopefully this patch (esp. if we can also fix the thinlto cache bug)
would make win-official survive enabling thin_lto_enable_optimizations
in official builds.


Build timings of official "all" builds on my Linux workstation:
(ThinLTO without opts is already enabled in official builds due to
CFI, so I didn't measure with it disabled.)

- With ThinLTO opts:              4h00m
- With ThinLTO + this patch:      1h12m


Note that this means test binaries and the shipping browser see
different levels of optimization. We think that is the right trade-off:
the difference should only matter in case of compiler/linker bugs, and
we believe the browser binary gets enough testing (e.g. by perf bots)
that such bugs will still be caught. Also to some extent this is
already the case: most testing is done on non-official builds, and
official builds use different optimizations (notably PGO).

We have more ideas for speeding up linking of the
non-link-time-optimized targets, but we hope this is enough to enable
ThinLTO in official Windows and Linux builds.

Bug: 110124, 1057737
Change-Id: I4130c02d0b7fe7012e52c92e51a0b255b576ad2b
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2628955
Commit-Queue: Hans Wennborg <hans@chromium.org>
Reviewed-by: default avatarNico Weber <thakis@chromium.org>
Reviewed-by: default avatarAndrew Grieve <agrieve@chromium.org>
Cr-Commit-Position: refs/heads/master@{#845618}
parent 88cd329e
...@@ -33,6 +33,10 @@ shared_library("chromium_android_linker") { ...@@ -33,6 +33,10 @@ shared_library("chromium_android_linker") {
# Disable coverage to avoid linker issue. # Disable coverage to avoid linker issue.
configs -= [ "//build/config/coverage:default_coverage" ] configs -= [ "//build/config/coverage:default_coverage" ]
# ThinLTO optimizations save a few bytes of binary size.
configs -= [ "//build/config/compiler:thinlto_optimize_default" ]
configs += [ "//build/config/compiler:thinlto_optimize_max" ]
# Disable orderfile instrumentation. Code in this target is in a different # Disable orderfile instrumentation. Code in this target is in a different
# .so, cannot call unexported instrumentation functions from another one (link # .so, cannot call unexported instrumentation functions from another one (link
# time error). # time error).
......
...@@ -340,6 +340,7 @@ default_compiler_configs = [ ...@@ -340,6 +340,7 @@ default_compiler_configs = [
"//build/config/compiler:no_rtti", "//build/config/compiler:no_rtti",
"//build/config/compiler:runtime_library", "//build/config/compiler:runtime_library",
"//build/config/compiler:thin_archive", "//build/config/compiler:thin_archive",
"//build/config/compiler:thinlto_optimize_default",
"//build/config/compiler:default_init_stack_vars", "//build/config/compiler:default_init_stack_vars",
"//build/config/compiler/pgo:default_pgo_flags", "//build/config/compiler/pgo:default_pgo_flags",
"//build/config/coverage:default_coverage", "//build/config/coverage:default_coverage",
......
...@@ -610,19 +610,12 @@ config("compiler") { ...@@ -610,19 +610,12 @@ config("compiler") {
"-fsplit-lto-unit", "-fsplit-lto-unit",
] ]
if (thin_lto_enable_optimizations) {
lto_opt_level = 2
} else {
lto_opt_level = 0
}
# Limit the size of the ThinLTO cache to the lesser of 10% of # Limit the size of the ThinLTO cache to the lesser of 10% of
# available disk space, 10GB and 100000 files. # available disk space, 10GB and 100000 files.
cache_policy = "cache_size=10%:cache_size_bytes=10g:cache_size_files=100000" cache_policy = "cache_size=10%:cache_size_bytes=10g:cache_size_files=100000"
if (is_win) { if (is_win) {
ldflags += [ ldflags += [
"/opt:lldlto=" + lto_opt_level,
"/opt:lldltojobs=all", "/opt:lldltojobs=all",
# Experimentally determined to yield a reasonable trade-off between # Experimentally determined to yield a reasonable trade-off between
...@@ -652,7 +645,6 @@ config("compiler") { ...@@ -652,7 +645,6 @@ config("compiler") {
"-Wl,--thinlto-cache-policy,$cache_policy", "-Wl,--thinlto-cache-policy,$cache_policy",
] ]
ldflags += [ "-Wl,--lto-O" + lto_opt_level ]
if (thin_lto_enable_optimizations) { if (thin_lto_enable_optimizations) {
# TODO(gbiv): We ideally shouldn't need to specify this; ThinLTO # TODO(gbiv): We ideally shouldn't need to specify this; ThinLTO
# should be able to better manage binary size increases on its own. # should be able to better manage binary size increases on its own.
...@@ -752,6 +744,44 @@ config("compiler") { ...@@ -752,6 +744,44 @@ config("compiler") {
} }
} }
# The BUILDCONFIG file sets this config on targets by default, which means when
# building with ThinLTO, no optimization is performed in the link step.
config("thinlto_optimize_default") {
if (!is_debug && use_thin_lto && is_a_target_toolchain) {
lto_opt_level = 0
if (is_win) {
ldflags = [ "/opt:lldlto=" + lto_opt_level ]
} else {
ldflags = [ "-Wl,--lto-O" + lto_opt_level ]
}
}
}
# Use this to enable optimization in the ThinLTO link step for select targets
# when thin_lto_enable_optimizations is set by doing:
#
# configs -= [ "//build/config/compiler:thinlto_optimize_default" ]
# configs += [ "//build/config/compiler:thinlto_optimize_max" ]
#
# Since it makes linking significantly slower and more resource intensive, only
# use it on important targets such as the main browser executable or dll.
config("thinlto_optimize_max") {
if (!is_debug && use_thin_lto && is_a_target_toolchain) {
if (thin_lto_enable_optimizations) {
lto_opt_level = 2
} else {
lto_opt_level = 0
}
if (is_win) {
ldflags = [ "/opt:lldlto=" + lto_opt_level ]
} else {
ldflags = [ "-Wl,--lto-O" + lto_opt_level ]
}
}
}
# This provides the basic options to select the target CPU and ABI. # This provides the basic options to select the target CPU and ABI.
# It is factored out of "compiler" so that special cases can use this # It is factored out of "compiler" so that special cases can use this
# without using everything that "compiler" brings in. Options that # without using everything that "compiler" brings in. Options that
......
...@@ -129,6 +129,8 @@ if (!is_android && !is_mac) { ...@@ -129,6 +129,8 @@ if (!is_android && !is_mac) {
} }
executable("chrome_initial") { executable("chrome_initial") {
configs -= [ "//build/config/compiler:thinlto_optimize_default" ]
configs += [ "//build/config/compiler:thinlto_optimize_max" ]
if (is_win) { if (is_win) {
output_name = "initialexe/chrome" output_name = "initialexe/chrome"
} else { } else {
...@@ -322,6 +324,8 @@ if (!is_android && !is_mac) { ...@@ -322,6 +324,8 @@ if (!is_android && !is_mac) {
if (is_win) { if (is_win) {
shared_library("chrome_dll") { shared_library("chrome_dll") {
configs += [ "//build/config/compiler:wexit_time_destructors" ] configs += [ "//build/config/compiler:wexit_time_destructors" ]
configs -= [ "//build/config/compiler:thinlto_optimize_default" ]
configs += [ "//build/config/compiler:thinlto_optimize_max" ]
defines = [] defines = []
sources = [ sources = [
...@@ -994,6 +998,8 @@ if (is_win) { ...@@ -994,6 +998,8 @@ if (is_win) {
] ]
configs += [ "//build/config/compiler:wexit_time_destructors" ] configs += [ "//build/config/compiler:wexit_time_destructors" ]
configs -= [ "//build/config/compiler:thinlto_optimize_default" ]
configs += [ "//build/config/compiler:thinlto_optimize_max" ]
} }
mac_framework_bundle("chrome_framework") { mac_framework_bundle("chrome_framework") {
......
...@@ -81,6 +81,11 @@ template("chrome_common_shared_library") { ...@@ -81,6 +81,11 @@ template("chrome_common_shared_library") {
deps += [ "//chrome:chrome_android_core" ] deps += [ "//chrome:chrome_android_core" ]
} }
if (!(defined(invoker.testonly) && invoker.testonly)) {
configs -= [ "//build/config/compiler:thinlto_optimize_default" ]
configs += [ "//build/config/compiler:thinlto_optimize_max" ]
}
configs += [ "//build/config/compiler:chrome_orderfile_config" ] configs += [ "//build/config/compiler:chrome_orderfile_config" ]
# Use a dynamically-generated linker script. # Use a dynamically-generated linker script.
......
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