Deploy ThinLTO on Linux x86-64 |
|||||||||
Issue descriptionCurrently, the official Chrome on Linux x86-64 is powered by Full LTO + WholeProgramDevirt + CFI. That results in very high link times (> 1 hour for some targets). ThinLTO (http://blog.llvm.org/2016/06/thinlto-scalable-and-incremental-lto.html) should dramatically speed this up, and Peter Collingbourne is currently working on CFI+ThinLTO and WholeProgramDevirt+ThinLTO on LLVM side. This issue is to track ThinLTO deployment in the Chrome land. The current status is that we have 'ThinLTO Linux ToT' bot: https://build.chromium.org/p/chromium.fyi/builders/ThinLTO Linux ToT It used to be green (with webkit_unit_tests disabled, see https://crbug.com/650718), but didn't have all the speedups, because we don't currently enable threads in LLVM Gold plugin, and we don't want to as running Gold + LLVM Gold plugin in the multithreaded mode under ThreadSanitizer found many race conditions. As making fixes to binutils is known to be painful, we would rather start using LLD for that.
,
Dec 6 2016
,
Dec 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a170f5836a8db250bdd60bb44c89d170f819090d commit a170f5836a8db250bdd60bb44c89d170f819090d Author: krasin <krasin@chromium.org> Date: Wed Dec 07 01:01:07 2016 Limit the number of thinlto jobs to 8; also support limits in lld. BUG= 660216 Review-Url: https://codereview.chromium.org/2559563002 Cr-Commit-Position: refs/heads/master@{#436805} [modify] https://crrev.com/a170f5836a8db250bdd60bb44c89d170f819090d/build/config/compiler/BUILD.gn
,
Dec 12 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/540eb54150917e0ffa28559b7c77fad2e4783eb0 commit 540eb54150917e0ffa28559b7c77fad2e4783eb0 Author: krasin <krasin@chromium.org> Date: Mon Dec 12 23:11:53 2016 Use LLD on ThinLTO ToT bot. BUG= 607968 , 660216 Review-Url: https://codereview.chromium.org/2568873003 Cr-Commit-Position: refs/heads/master@{#437944} [modify] https://crrev.com/540eb54150917e0ffa28559b7c77fad2e4783eb0/tools/mb/mb_config.pyl
,
Jan 19 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bf6c6a31bafed2f7bc5ed6083e32b21956152221 commit bf6c6a31bafed2f7bc5ed6083e32b21956152221 Author: krasin <krasin@chromium.org> Date: Thu Jan 19 22:34:10 2017 Build with full debug info on ThinLTO Linux ToT bot. This will be a better proxy for the official builds, which we are mostly worried about in the context of ThinLTO builds. BUG= 682773 , 660216 Review-Url: https://codereview.chromium.org/2644803003 Cr-Commit-Position: refs/heads/master@{#444866} [modify] https://crrev.com/bf6c6a31bafed2f7bc5ed6083e32b21956152221/tools/mb/mb_config.pyl
,
Mar 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/tools/build/+/973870a62af1f77ac5356fbabd9a471a4fdf6c91 commit 973870a62af1f77ac5356fbabd9a471a4fdf6c91 Author: Ivan Krasin <krasin@chromium.org> Date: Fri Mar 17 15:58:32 2017 Add 'CFI ThinLTO Linux ToT' buildbot (recipes part). See the src part for more details: https://codereview.chromium.org/2759443002/ BUG= 660216 Change-Id: If1922c68bd2da060a84a5da2623387d83acdde62 Reviewed-on: https://chromium-review.googlesource.com/456616 Reviewed-by: Dirk Pranke <dpranke@chromium.org> Commit-Queue: Ivan Krasin <krasin@chromium.org> [add] https://crrev.com/973870a62af1f77ac5356fbabd9a471a4fdf6c91/scripts/slave/recipes/chromium.expected/full_chromium_fyi_CFI_ThinLTO_Linux_ToT.json [modify] https://crrev.com/973870a62af1f77ac5356fbabd9a471a4fdf6c91/masters/master.chromium.fyi/master.cfg [modify] https://crrev.com/973870a62af1f77ac5356fbabd9a471a4fdf6c91/scripts/slave/recipe_modules/chromium_tests/chromium_fyi.py [modify] https://crrev.com/973870a62af1f77ac5356fbabd9a471a4fdf6c91/masters/master.chromium.fyi/slaves.cfg
,
Mar 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/bad9089fbe4617afcdf29fb481058d9cf412da58 commit bad9089fbe4617afcdf29fb481058d9cf412da58 Author: krasin <krasin@chromium.org> Date: Fri Mar 17 16:02:02 2017 Add 'CFI ThinLTO Linux ToT' buildbot (src part). We're close to replace FullLTO and CFI with FullLTO with their ThinLTO counterparts. This should reduce linking time by 2x-3x, and open a road for even better gains. This particular change sets up a bot that tests for CFI+ThinLTO. Eventually, when we switch the desktop build to ThinLTO, we'll retire all ThinLTO bots and make CFI/LTO bots to imply ThinLTO. BUG= 660216 Review-Url: https://codereview.chromium.org/2759443002 Cr-Commit-Position: refs/heads/master@{#457782} [modify] https://crrev.com/bad9089fbe4617afcdf29fb481058d9cf412da58/testing/buildbot/chromium.fyi.json [modify] https://crrev.com/bad9089fbe4617afcdf29fb481058d9cf412da58/tools/mb/mb_config.pyl
,
Mar 27 2017
Clang roll happened. We still need to wait some time (like, a day) to decide if it sticks or not, but assuming it's, the next steps are: 1. Start using llvm-ar instead of /usr/bin/ar when building with Clang: https://codereview.chromium.org/2766333002/ (approved and dry run succeeded) 2. Turn on LLD + ThinLTO on a few stable bots (LTO Linux and LTO Linux Perf, CFI Linux Full): https://codereview.chromium.org/2776243002/ After that, hopefully, we can enable ThinLTO and LLD everywhere. One potential blocker is https://crbug.com/686980 , but maybe we can workaround that.
,
Mar 27 2017
With "everywhere", you mean "everywhere that does LTO" (i.e. not for regular non-official linux builds), right? I kind of prefer toggling all currently-LTO'ing builds to ThinLTO at once. What's the advantage of switching only a few bots?
,
Mar 27 2017
yes, everywhere with LTO. The regular builds shall not be made any slower. If ThinLTO gets to the point it's as fast as regular Gold, then one might consider that.
,
Mar 27 2017
> What's the advantage of switching only a few bots? I would like to preview issues before messing with the official desktop builders and Perf waterfall. It's easy and only delay us for a day. My experience with launching FullLTO, where I had to revert so many times, taught me to not try to switch all bots at once.
,
Mar 27 2017
Ok, if it's just for one day that's fine. (My motivation is that we should try to keep the number of supported configurations low. Every distinct configuration can break in its own interesting way.)
,
Mar 27 2017
>Every distinct configuration can break in its own interesting way Tell me about that. :) I am very eager to kill FullLTO and Gold, and that's the top priority for me now.
,
Mar 28 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/8968cd5170f4a22b1c508ffa3bc240b346c44b1b commit 8968cd5170f4a22b1c508ffa3bc240b346c44b1b Author: krasin <krasin@chromium.org> Date: Tue Mar 28 16:46:15 2017 Use llvm-ar when building with Clang. llvm-ar is faster and is capable of handling LLVM bitcode files without a need for a Gold plugin. BUG= 660216 , 607968 Review-Url: https://codereview.chromium.org/2766333002 Cr-Commit-Position: refs/heads/master@{#460129} [modify] https://crrev.com/8968cd5170f4a22b1c508ffa3bc240b346c44b1b/build/config/compiler/BUILD.gn [modify] https://crrev.com/8968cd5170f4a22b1c508ffa3bc240b346c44b1b/build/config/posix/BUILD.gn [modify] https://crrev.com/8968cd5170f4a22b1c508ffa3bc240b346c44b1b/build/toolchain/gcc_toolchain.gni
,
Mar 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/82460c48b74229f08ddb8b19982a6a5076787466 commit 82460c48b74229f08ddb8b19982a6a5076787466 Author: krasin <krasin@chromium.org> Date: Wed Mar 29 05:43:12 2017 Make 'LTO Linux' and 'LTO Linux Perf' bots to use LLD and ThinLTO. Soon, we'll switch all LTO bots to LLD / ThinLTO, but we need a few brave souls to go first. BUG= 660216 , 607968 Review-Url: https://codereview.chromium.org/2776243002 Cr-Commit-Position: refs/heads/master@{#460291} [modify] https://crrev.com/82460c48b74229f08ddb8b19982a6a5076787466/tools/mb/mb_config.pyl
,
Mar 29 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/95e4983f85e5afb97b37df59b5666b2f3d2949a1 commit 95e4983f85e5afb97b37df59b5666b2f3d2949a1 Author: krasin <krasin@chromium.org> Date: Wed Mar 29 16:37:40 2017 (Really) disable gdb-index for LTO. This is a follow up to https://codereview.chromium.org/2782483002, where only official LTO builds had got gdb-index disabled. BUG= 660216 Review-Url: https://codereview.chromium.org/2786603003 Cr-Commit-Position: refs/heads/master@{#460421} [modify] https://crrev.com/95e4983f85e5afb97b37df59b5666b2f3d2949a1/build/config/compiler/BUILD.gn
,
Apr 20 2017
Current state: we are waiting for a Clang roll, and then there's no currently known issues; we will try to enable ThinLTO on for Linux LTO builds after the new toolchain is declared stable.
,
Apr 21 2017
This will soon be enabled on Linux x86-64. CL: https://codereview.chromium.org/2831213006/ I plan to submit that CL on Monday.
,
Apr 24 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7997bbe88aef7eeca37111dcb8426f0430347fb5 commit 7997bbe88aef7eeca37111dcb8426f0430347fb5 Author: krasin <krasin@chromium.org> Date: Mon Apr 24 20:19:18 2017 Enable ThinLTO and LLD for POSIX LTO by default on Linux. Eventually, we will enable LLD on Linux even for regular builds, but it's natural to make incremental steps here. ThinLTO brings multi-threaded linking for LinkTimeOptimization builds, which allows to speed up codegen considerably (up to 4x). ThinLTO also manages a cache inside out/<gn-config>/thinlto-cache directory, that improves incremental linking as well. There's a cache pruning policy that will prevent the cache from growing indefinitely. The policy is not yet finalized, and complaints / suggestions are welcome. BUG= 660216 , 607968 Review-Url: https://codereview.chromium.org/2831213006 Cr-Commit-Position: refs/heads/master@{#466732} [modify] https://crrev.com/7997bbe88aef7eeca37111dcb8426f0430347fb5/build/toolchain/toolchain.gni
,
Apr 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6aee4d67dcc67850c8ec7b9fc181cb7d91b55734 commit 6aee4d67dcc67850c8ec7b9fc181cb7d91b55734 Author: krasin <krasin@chromium.org> Date: Tue Apr 25 00:42:01 2017 Revert of Enable ThinLTO and LLD for POSIX LTO by default on Linux. (patchset #4 id:60001 of https://codereview.chromium.org/2831213006/ ) Reason for revert: Code size increase: https://chromeperf.appspot.com/report?sid=7826328365affe1842778994c934cbe08c7f9b78ef5d8895ec85d3094bb8972d&start_rev=466555&end_rev=466782 Postmortem is coming. TL;DR: worse dead code elimination in ThinLTO Original issue's description: > Enable ThinLTO and LLD for POSIX LTO by default on Linux. > > Eventually, we will enable LLD on Linux even for regular > builds, but it's natural to make incremental steps here. > > ThinLTO brings multi-threaded linking for LinkTimeOptimization > builds, which allows to speed up codegen considerably (up to 4x). > ThinLTO also manages a cache inside out/<gn-config>/thinlto-cache > directory, that improves incremental linking as well. > > There's a cache pruning policy that will prevent the cache from > growing indefinitely. The policy is not yet finalized, and > complaints / suggestions are welcome. > > BUG= 660216 , 607968 > > Review-Url: https://codereview.chromium.org/2831213006 > Cr-Commit-Position: refs/heads/master@{#466732} > Committed: https://chromium.googlesource.com/chromium/src/+/7997bbe88aef7eeca37111dcb8426f0430347fb5 TBR=dpranke@chromium.org,thakis@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 660216 , 607968 Review-Url: https://codereview.chromium.org/2842683002 Cr-Commit-Position: refs/heads/master@{#466842} [modify] https://crrev.com/6aee4d67dcc67850c8ec7b9fc181cb7d91b55734/build/toolchain/toolchain.gni
,
Apr 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/deda22b114bc8308ad1ae888727a66ceda9189aa commit deda22b114bc8308ad1ae888727a66ceda9189aa Author: pcc <pcc@chromium.org> Date: Tue Apr 25 21:54:04 2017 build: Enable function sections and data sections during LTO. This allows the linker to apply --gc-sections to object files created during LTO. This yields a >8MB reduction in stripped binary size for official binaries built with ThinLTO. This change increases the size of an official binary built with regular LTO by 8192 bytes, but this probably isn't worth worrying about. BUG= 660216 , 607968 R=thakis@chromium.org,krasin@chromium.org Review-Url: https://codereview.chromium.org/2840723003 Cr-Commit-Position: refs/heads/master@{#467129} [modify] https://crrev.com/deda22b114bc8308ad1ae888727a66ceda9189aa/build/config/compiler/BUILD.gn
,
Apr 26 2017
I have taken a quick look at the performance graphs. They look quite good: https://chromeperf.appspot.com/report?sid=aaf069b56e5c177e4ba526cf71ef4a3f1944658c548d70c466584d4b3b76bbef&start_rev=464902&end_rev=467226 Many microbenchmarks improved by ~10%. I have not found any regressions yet (keep looking)
,
Apr 26 2017
The revision ThinLTO was enabled is 466732, reverted at 466842.
,
Apr 26 2017
Even more improvements across different test suites: https://chromeperf.appspot.com/report?sid=c3fd4e1a1a20cd382ce92b13fba839bdfbb4e4928b4e5da6574a6cc50eadd32c&start_rev=464902&end_rev=467226 I have observed benchmarks which are not affected by the change, but I am yet to find a benchmark that degraded. +Elliott Sprehn: can you please look at the graphs above and tell us if you're happy to see improvements like these. We tried to launch ThinLTO (it was enabled between r466732 and r466842), but it was reverted due to the large code size regression. Peter (pcc@) is working on fixing this regression, so it's a preview of what's coming soon.
,
May 4 2017
Re-assigning to myself, as Ivan is no longer working on this. I have been working on a prototype of improved dead code stripping support in the linker. Unfortunately this doesn't seem to have helped as much as I had hoped: with my prototype, the binary size increases from 118445376 bytes (current official build) to 150310912 bytes (official build + ThinLTO). So around 27%, which is less than the delta during the previous launch attempt (37%) but still quite substantial. The next thing I tried was to measure the code size/perf impact of -Os together with ThinLTO, on the hypothesis that the perf gains of ThinLTO would help to overcome the losses of -Os. Binary size looks reasonable (109850624 bytes), but on my local workstation the perf delta versus current official builds seems inconclusive (I am seeing some gains and some losses in smoothness.top_25_smooth, blink_perf.layout, blink_perf.svg and blink_perf.css, but the results don't seem statistically significant). I'd imagine that a 27% binary size regression isn't tolerable. If that's the case, the next step I'd like to take is to attempt to enable ThinLTO together with -Os in official builds while monitoring the perf dashboard.
,
May 4 2017
What's the increase from no thinlto + -Os to thinlto + -Os? I'd kind of like if we'd use uniform optimization settings everywhere. Android is -Os, Windows is mostly -Os, Linux and Mac use -O2. Using -Os everywhere sounds nice, if feasible.
,
May 4 2017
Have we done any work on tweaking ThinLTO's opt pipeline? I only have a vague understanding of this, but if we're running mostly the same opt pipeline twice, be probably want the inliner a lot more conservative in the LTO step since there will be many more opportunities, not all of which are a good idea.
,
May 4 2017
> What's the increase from no thinlto + -Os to thinlto + -Os? I'll measure that. > I'd kind of like if we'd use uniform optimization settings everywhere. Android is -Os, Windows is mostly -Os, Linux and Mac use -O2. Using -Os everywhere sounds nice, if feasible. I think I agree. > Have we done any work on tweaking ThinLTO's opt pipeline? > > I only have a vague understanding of this, but if we're running mostly the same opt pipeline twice, be probably want the inliner a lot more conservative in the LTO step since there will be many more opportunities, not all of which are a good idea. I know that people have been working on the ThinLTO opt pipeline, but I'm not sure how much of that work has been focused on binary size as opposed to performance. Teresa probably has the best picture of how the pipeline compares to regular builds; adding her to cc.
,
May 4 2017
You're right, we haven't done much tuning for binary size. And we don't reduce aggressiveness of optimizations like inlining in the compile step, the idea being that we want the code going into the thin link to be decently well optimized so that we don't have to import as much. However, we bail out of the optimization pipeline early during the compile step, before doing things like loop optimizations (which may expand the code size and negatively affect importing decisions). See uses of "PrepareForThinLTO" in the PassManagerBuilder. It might be interesting to play with inlining thresholds during the compile step. There's a tradeoff, because we have per-function instruction limits on importing, and inlining can increase function sizes, but the instruction threshold decays as the call chain is traversed during the thin link, so there is some benefit to collapsing the call chain as well. Additionally, after inlining and things like constant prop we may end up with more compact code. In the ThinLTO backends, the optimization pipeline is very similar to that of a normal -O2 build. Note this is more aggressive than the regular LTO backend pipeline, which has to be limited since there is a much higher compilation overhead due to the monolithic nature of regular LTO.
,
May 5 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a367e903ff405e49e4a22b0512b8b989be08838a commit a367e903ff405e49e4a22b0512b8b989be08838a Author: pcc <pcc@chromium.org> Date: Fri May 05 00:45:13 2017 build: Fix optimize_for_size build on Linux by adding an is_nacl guard. The PNaCl compiler does not understand -Os. BUG= 660216 R=thakis@chromium.org Review-Url: https://codereview.chromium.org/2865573002 Cr-Commit-Position: refs/heads/master@{#469552} [modify] https://crrev.com/a367e903ff405e49e4a22b0512b8b989be08838a/build/config/compiler/BUILD.gn
,
May 5 2017
> It might be interesting to play with inlining thresholds during the compile step. Yes, that may be worth playing with. Though I would have thought that functions compiled with -Os would be subject to the lower inlining threshold for optsize functions in the backend. So I guess at -Os we are importing functions, but not using the imported copies as much. It isn't clear what impact that would have on code size, but I don't think it would help. I guess we could fix that by including the optsize/minsize attributes in the function summary and using them to control the importing threshold. One thing that the regular inliner does is to apply a very high threshold to functions that are called by one function but not otherwise referenced: http://llvm-cs.pcc.me.uk/lib/Analysis/InlineCost.cpp#1290 I think we should be able to do something similar at the summary level, but only when the caller is not exported (so we can drop the copy of the callee that would otherwise be used by importers of the caller).
,
May 5 2017
> I'll measure that. If I just enable -Os in an official build (so full LTO + CFI + whole-program devirt, no other cross-module opt) the size is 97789216 bytes. So ThinLTO costs us about 12% at -Os.
,
May 5 2017
,
May 6 2017
The imported copy should be dropped since it is available_externally, but this can result in promotion of many needlessly exported locals which prevents their removal if they are fully inlined in the original module. But hopefully then linker gc would remove them. Although as you point out later, they would potentially have gotten a bonus for inlining if they were still locals (last call to static bonus), which they wouldn't if promoted. Yes. I haven't looked at -Os at all with ThinLTO so there is probably a good amount of tuning to be done there... Yeah this is something Mehdi and I have talked about, but never had a chance to implement. It should be pretty easy to do some analysis of the summary to see how many functions are called a single time, from another module, to see how much opportunity there might be.
,
May 10 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c1269ce7fec8568a1789e07b2b5bb3b630a67f5f commit c1269ce7fec8568a1789e07b2b5bb3b630a67f5f Author: pcc <pcc@chromium.org> Date: Wed May 10 16:41:31 2017 build: Enable optimize_for_size unconditionally. This change causes us to favor size over speed on Linux and Mac, which aligns the build config for those platforms with that of the other supported platforms, and should reduce the binary size impact of enabling ThinLTO. This change is expected to reduce binary size for Linux official builds by about 15%. There may be unacceptable perf regressions associated with this change, but the perf bots should at least let us know where those regressions are. I plan to monitor the Linux and Mac perf bots once it lands. BUG= 660216 R=thakis@chromium.org Review-Url: https://codereview.chromium.org/2864383003 Cr-Commit-Position: refs/heads/master@{#470606} [modify] https://crrev.com/c1269ce7fec8568a1789e07b2b5bb3b630a67f5f/build/config/compiler/BUILD.gn
,
May 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2a6f440fa84a584d916254f9a4c4e4d83c53435f commit 2a6f440fa84a584d916254f9a4c4e4d83c53435f Author: yhirano <yhirano@chromium.org> Date: Thu May 11 03:23:19 2017 Revert of build: Enable optimize_for_size unconditionally. (patchset #1 id:1 of https://codereview.chromium.org/2864383003/ ) Reason for revert: Causes failures on a MSAN bot. https://uberchromegw.corp.google.com/i/chromium.memory/builders/Linux%20ChromiumOS%20MSan%20Tests/builds/440 Original issue's description: > build: Enable optimize_for_size unconditionally. > > This change causes us to favor size over speed on Linux and Mac, > which aligns the build config for those platforms with that of the > other supported platforms, and should reduce the binary size impact > of enabling ThinLTO. This change is expected to reduce binary size > for Linux official builds by about 15%. > > There may be unacceptable perf regressions associated with this > change, but the perf bots should at least let us know where those > regressions are. I plan to monitor the Linux and Mac perf bots once > it lands. > > BUG= 660216 > R=thakis@chromium.org > > Review-Url: https://codereview.chromium.org/2864383003 > Cr-Commit-Position: refs/heads/master@{#470606} > Committed: https://chromium.googlesource.com/chromium/src/+/c1269ce7fec8568a1789e07b2b5bb3b630a67f5f TBR=thakis@chromium.org,pcc@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 660216 Review-Url: https://codereview.chromium.org/2881503002 Cr-Commit-Position: refs/heads/master@{#470793} [modify] https://crrev.com/2a6f440fa84a584d916254f9a4c4e4d83c53435f/build/config/compiler/BUILD.gn
,
May 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/97b99808511f8918d00add266a8555ba5e21813c commit 97b99808511f8918d00add266a8555ba5e21813c Author: pcc <pcc@chromium.org> Date: Thu May 11 19:35:26 2017 mesa: Disable support for loading libtxc_dxtn.so if msan is enabled. The libtxc_dxtn.so library supports loading textures in S3TC format. This change disables code in mesa that loads libtxc_dxtn.so if msan is enabled, as the library will be loaded from a system directory, which may cause spurious msan reports because msan expects most libraries in the process to be compiled with msan. For some reason, msan only appears to break when compiling with -Os, as revealed by a recent attempt [0] to enable -Os on Linux and Mac. [0] https://codereview.chromium.org/2864383003/ BUG= 660216 R=senorblanco@chromium.org,thakis@chromium.org Review-Url: https://codereview.chromium.org/2876693003 Cr-Commit-Position: refs/heads/master@{#471045} [modify] https://crrev.com/97b99808511f8918d00add266a8555ba5e21813c/third_party/mesa/BUILD.gn
,
May 11 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/ea5fad44dd0b0a74a40dbe2c05598f43861cd75a commit ea5fad44dd0b0a74a40dbe2c05598f43861cd75a Author: pcc <pcc@chromium.org> Date: Thu May 11 19:42:44 2017 Reland of build: Enable optimize_for_size unconditionally. (patchset #1 id:1 of https://codereview.chromium.org/2881503002/ ) Reason for revert: Relanding now that the msan issue is fixed: https://codereview.chromium.org/2876693003 Original issue's description: > Revert of build: Enable optimize_for_size unconditionally. (patchset #1 id:1 of https://codereview.chromium.org/2864383003/ ) > > Reason for revert: > Causes failures on a MSAN bot. > https://uberchromegw.corp.google.com/i/chromium.memory/builders/Linux%20ChromiumOS%20MSan%20Tests/builds/440 > > > Original issue's description: > > build: Enable optimize_for_size unconditionally. > > > > This change causes us to favor size over speed on Linux and Mac, > > which aligns the build config for those platforms with that of the > > other supported platforms, and should reduce the binary size impact > > of enabling ThinLTO. This change is expected to reduce binary size > > for Linux official builds by about 15%. > > > > There may be unacceptable perf regressions associated with this > > change, but the perf bots should at least let us know where those > > regressions are. I plan to monitor the Linux and Mac perf bots once > > it lands. > > > > BUG= 660216 > > R=thakis@chromium.org > > > > Review-Url: https://codereview.chromium.org/2864383003 > > Cr-Commit-Position: refs/heads/master@{#470606} > > Committed: https://chromium.googlesource.com/chromium/src/+/c1269ce7fec8568a1789e07b2b5bb3b630a67f5f > > TBR=thakis@chromium.org,pcc@chromium.org > # Skipping CQ checks because original CL landed less than 1 days ago. > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > BUG= 660216 > > Review-Url: https://codereview.chromium.org/2881503002 > Cr-Commit-Position: refs/heads/master@{#470793} > Committed: https://chromium.googlesource.com/chromium/src/+/2a6f440fa84a584d916254f9a4c4e4d83c53435f TBR=thakis@chromium.org,yhirano@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 660216 Review-Url: https://codereview.chromium.org/2870393005 Cr-Commit-Position: refs/heads/master@{#471046} [modify] https://crrev.com/ea5fad44dd0b0a74a40dbe2c05598f43861cd75a/build/config/compiler/BUILD.gn
,
May 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/22667180463b25a9059df17350b495b037e5a300 commit 22667180463b25a9059df17350b495b037e5a300 Author: pcc <pcc@chromium.org> Date: Tue May 16 03:58:11 2017 Revert of build: Enable optimize_for_size unconditionally. (patchset #1 id:1 of https://codereview.chromium.org/2870393005/ ) Reason for revert: Large number of perf regressions on mac. BUG= 722473 Original issue's description: > Reland of build: Enable optimize_for_size unconditionally. (patchset #1 id:1 of https://codereview.chromium.org/2881503002/ ) > > Reason for revert: > Relanding now that the msan issue is fixed: https://codereview.chromium.org/2876693003 > > Original issue's description: > > Revert of build: Enable optimize_for_size unconditionally. (patchset #1 id:1 of https://codereview.chromium.org/2864383003/ ) > > > > Reason for revert: > > Causes failures on a MSAN bot. > > https://uberchromegw.corp.google.com/i/chromium.memory/builders/Linux%20ChromiumOS%20MSan%20Tests/builds/440 > > > > > > Original issue's description: > > > build: Enable optimize_for_size unconditionally. > > > > > > This change causes us to favor size over speed on Linux and Mac, > > > which aligns the build config for those platforms with that of the > > > other supported platforms, and should reduce the binary size impact > > > of enabling ThinLTO. This change is expected to reduce binary size > > > for Linux official builds by about 15%. > > > > > > There may be unacceptable perf regressions associated with this > > > change, but the perf bots should at least let us know where those > > > regressions are. I plan to monitor the Linux and Mac perf bots once > > > it lands. > > > > > > BUG= 660216 > > > R=thakis@chromium.org > > > > > > Review-Url: https://codereview.chromium.org/2864383003 > > > Cr-Commit-Position: refs/heads/master@{#470606} > > > Committed: https://chromium.googlesource.com/chromium/src/+/c1269ce7fec8568a1789e07b2b5bb3b630a67f5f > > > > TBR=thakis@chromium.org,pcc@chromium.org > > # Skipping CQ checks because original CL landed less than 1 days ago. > > NOPRESUBMIT=true > > NOTREECHECKS=true > > NOTRY=true > > BUG= 660216 > > > > Review-Url: https://codereview.chromium.org/2881503002 > > Cr-Commit-Position: refs/heads/master@{#470793} > > Committed: https://chromium.googlesource.com/chromium/src/+/2a6f440fa84a584d916254f9a4c4e4d83c53435f > > TBR=thakis@chromium.org,yhirano@chromium.org > # Skipping CQ checks because original CL landed less than 1 days ago. > NOPRESUBMIT=true > NOTREECHECKS=true > NOTRY=true > BUG= 660216 > > Review-Url: https://codereview.chromium.org/2870393005 > Cr-Commit-Position: refs/heads/master@{#471046} > Committed: https://chromium.googlesource.com/chromium/src/+/ea5fad44dd0b0a74a40dbe2c05598f43861cd75a TBR=thakis@chromium.org,yhirano@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG= 660216 Review-Url: https://codereview.chromium.org/2883113002 Cr-Commit-Position: refs/heads/master@{#472010} [modify] https://crrev.com/22667180463b25a9059df17350b495b037e5a300/build/config/compiler/BUILD.gn
,
May 17 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a2adba27e3ef69283f5060c04c4e64b02bb6ea4d commit a2adba27e3ef69283f5060c04c4e64b02bb6ea4d Author: pcc <pcc@chromium.org> Date: Wed May 17 03:35:49 2017 build: Enable -fwhole-program-vtables under ThinLTO. This feature has been supported under ThinLTO for some time now. BUG= 660216 R=thakis@chromium.org Review-Url: https://codereview.chromium.org/2890573002 Cr-Commit-Position: refs/heads/master@{#472304} [modify] https://crrev.com/a2adba27e3ef69283f5060c04c4e64b02bb6ea4d/build/config/compiler/BUILD.gn
,
Jun 1 2017
I am experimenting thinlto on ChromeOS.
Currently thinlto or lto is disabled on ChromeOS
allow_posix_link_time_opt =
is_clang && target_os == "linux" && !is_chromeos && target_cpu == "x64" &&
is_official_build
This allow_posix_link_time_opt enables thinlto or lto based on whether use_thin_lto is set or not.
Does it make sense to have another flag 'use_lto' to control whether we want
to use lto or not?
The reason for the new flag is that, I want to make ChromeOS can support link_time_opt build but I do not want to enable either of the build (thinlto and lto) now.
,
Jun 1 2017
> Does it make sense to have another flag 'use_lto' to control whether we want to use lto or not? I think that is exactly what the allow_posix_link_time_opt flag is for. The flag is only disabled by default on Chrome OS, you should be able to enable it by adding "allow_posix_link_time_opt = true" to your args.gn.
,
Jun 15 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/da072d13b2a1cc8459c29aa4e52584a4491a4ad6 commit da072d13b2a1cc8459c29aa4e52584a4491a4ad6 Author: pcc <pcc@chromium.org> Date: Thu Jun 15 22:38:38 2017 Disable ThinLTO optimizations on Linux with LLD. Unfortunately: - Binaries built with ThinLTO at -O2/--lto-O2 (which is what you currently get with use_thin_lto=true) are too large. - Binaries built with -Os introduce significant performance regressions. This issue most likely affects ThinLTO as well, at least according to local benchmarking. So that leaves -O2/--lto-O0, which is pretty much what we have now with full LTO. (With full LTO, --lto-O1 basically just means apply CFI, whole-program devirtualization, dead code elimination and CFG simplification.) Efforts have been made on the LLVM side to make ThinLTO at --lto-O0 generate binaries with as good quality as full LTO with --lto-O1 (simplifying the code generated for CFI checks (r304921) and allowing for more dead code elimination (r305482)). Now that those are complete, we are ready to start using --lto-O0 with ThinLTO. BUG= 660216 R=thakis@chromium.org Review-Url: https://codereview.chromium.org/2939923004 Cr-Commit-Position: refs/heads/master@{#479875} [modify] https://crrev.com/da072d13b2a1cc8459c29aa4e52584a4491a4ad6/build/config/compiler/BUILD.gn
,
Jun 16 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5cb9983f12f300422c16ba812c2d5cdeb278cb74 commit 5cb9983f12f300422c16ba812c2d5cdeb278cb74 Author: pcc <pcc@chromium.org> Date: Fri Jun 16 20:13:24 2017 Enable ThinLTO for POSIX LTO by default on Linux. This is a reland of the remaining part of https://codereview.chromium.org/2831213006 . The binary size regression has been addressed by disabling ThinLTO whole-program optimizations and improving the quality of unoptimized code. BUG= 660216 R=thakis@chromium.org,hans@chromium.org Review-Url: https://codereview.chromium.org/2939373002 Cr-Commit-Position: refs/heads/master@{#480144} [modify] https://crrev.com/5cb9983f12f300422c16ba812c2d5cdeb278cb74/build/toolchain/toolchain.gni
,
Jun 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2be84dd0d41f8247d22bdc8690b5520d6c483a8d commit 2be84dd0d41f8247d22bdc8690b5520d6c483a8d Author: pcc <pcc@chromium.org> Date: Fri Jun 23 00:46:58 2017 Revert of Enable ThinLTO for POSIX LTO by default on Linux. (patchset #2 id:20001 of https://codereview.chromium.org/2939373002/ ) Reason for revert: Breaks chromium.perf builder. BUG= 735651 Original issue's description: > Enable ThinLTO for POSIX LTO by default on Linux. > > This is a reland of the remaining part of > https://codereview.chromium.org/2831213006 . > > The binary size regression has been addressed by disabling ThinLTO > whole-program optimizations and improving the quality of unoptimized > code. > > BUG= 660216 > R=thakis@chromium.org,hans@chromium.org > > Review-Url: https://codereview.chromium.org/2939373002 > Cr-Commit-Position: refs/heads/master@{#480144} > Committed: https://chromium.googlesource.com/chromium/src/+/5cb9983f12f300422c16ba812c2d5cdeb278cb74 TBR=hans@chromium.org,thakis@chromium.org # Not skipping CQ checks because original CL landed more than 1 days ago. BUG= 660216 Review-Url: https://codereview.chromium.org/2953943002 Cr-Commit-Position: refs/heads/master@{#481748} [modify] https://crrev.com/2be84dd0d41f8247d22bdc8690b5520d6c483a8d/build/toolchain/toolchain.gni
,
Jun 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/c4e41121343e0c49038ef2e4ab719004955cfa0f commit c4e41121343e0c49038ef2e4ab719004955cfa0f Author: Peter Collingbourne <pcc@chromium.org> Date: Fri Jun 23 02:55:10 2017 Update packaging deps after r481748. TBR=thomasanderson@chromium.org NOTREECHECKS=true NOTRY=true BUG= 660216 Review-Url: https://codereview.chromium.org/2956553003 . Cr-Commit-Position: refs/heads/master@{#481792} [modify] https://crrev.com/c4e41121343e0c49038ef2e4ab719004955cfa0f/chrome/installer/linux/debian/expected_deps_x64_jessie
,
Jun 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9a6cc80b32bb5522fe262e1f60a9995dabf9dcbb commit 9a6cc80b32bb5522fe262e1f60a9995dabf9dcbb Author: pcc <pcc@chromium.org> Date: Fri Jun 23 17:35:17 2017 Enable ThinLTO for POSIX LTO by default on Linux, take 2. Previous attempt: https://codereview.chromium.org/2939373002 It was reverted because it caused large thinlto-cache directories to be created on the perf bots, which were also being unnecessarily included in package archives. This has been addressed in two ways: 1) the bots have been taught to exclude thinlto-cache from package archives (see https://chromium-review.googlesource.com/c/544530/). 2) this change limits the size of the cache to 10% of available disk space. BUG= 660216 , 735651 R=dpranke@chromium.org,thakis@chromium.org TBR=thomasanderson@chromium.org Review-Url: https://codereview.chromium.org/2957533002 Cr-Commit-Position: refs/heads/master@{#481944} [modify] https://crrev.com/9a6cc80b32bb5522fe262e1f60a9995dabf9dcbb/build/config/compiler/BUILD.gn [modify] https://crrev.com/9a6cc80b32bb5522fe262e1f60a9995dabf9dcbb/build/toolchain/toolchain.gni [modify] https://crrev.com/9a6cc80b32bb5522fe262e1f60a9995dabf9dcbb/chrome/installer/linux/debian/expected_deps_x64_jessie
,
Jun 23 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6640da843b8266e576b0399b10698dc2673c3fee commit 6640da843b8266e576b0399b10698dc2673c3fee Author: Peter Collingbourne <pcc@chromium.org> Date: Fri Jun 23 21:02:44 2017 Update packaging deps after r481748. TBR=thomasanderson@chromium.org NOTREECHECKS=true NOTRY=true BUG= 660216 Review-Url: https://codereview.chromium.org/2956553003 . Cr-Original-Commit-Position: refs/heads/master@{#481792} Review-Url: https://codereview.chromium.org/2952403004 . Cr-Commit-Position: refs/branch-heads/3139@{#9} Cr-Branched-From: 2dd0ea84298d45e0962c595c591529bc84dd0ebd-refs/heads/master@{#481757} [modify] https://crrev.com/6640da843b8266e576b0399b10698dc2673c3fee/chrome/installer/linux/debian/expected_deps_x64_jessie
,
Jul 13 2017
,
Jul 14 2017
I collected some performance numbers by looking at the ninja logs from https://luci-milo.appspot.com/buildbot/chromium.chrome/Google%20Chrome%20Linux%20x64/ Taking the median chrome link time from before and after https://codereview.chromium.org/2957533002 landed (3 revisions each side), looks like it went down from 1697s to 315s. Same thing for https://chromium-review.googlesource.com/566280 (which rolled clang past a revision that improved symbol table performance) it went down from 297s to 282s. ThinLTO also allows for incremental builds. On my local machine a full build takes 360s and no-op incremental takes about 140s, which is odd because it used to take something like 25s before, though I bump the size of the cache from 10% of disk to 75% it takes like 60s. Probably worth 1) making the cache larger (should be possible now that https://reviews.llvm.org/D34547 is landed and rolled in) and 2) doing some profiling to figure out why that's regressed.
,
Jul 14 2017
(s/build/link/ in last para)
,
Mar 6 2018
For the record: https://luci-milo.appspot.com/buildbot/chromium.chrome/Google%20Chrome%20Linux%20x64/18613 was the build where my change landed. Looking at the median of 5 builds either side of that, total build time decreased from 1h17m to 37m48s. |
|||||||||
►
Sign in to add a comment |
|||||||||
Comment 1 by krasin@chromium.org
, Dec 5 2016