Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2831213006:
Enable ThinLTO and LLD for POSIX LTO by default on Linux. (Closed)
DescriptionEnable 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
Patch Set 1 #Patch Set 2 : fmt #Patch Set 3 : Update a comment #Patch Set 4 : Update another comment #Messages
Total messages: 34 (18 generated)
The CQ bit was checked by krasin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by krasin@chromium.org
The CQ bit was checked by krasin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
krasin@chromium.org changed reviewers: + dpranke@chromium.org
The CQ bit was checked by krasin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was checked by krasin@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
thakis@chromium.org changed reviewers: + thakis@chromium.org
lgtm, but please say "64-bit" in CL description. What's the story for 32-bit?
(also, please watch bot cycle time on bots that do full builds, since parallel links might now compete for resources a lot. also, different linker.)
On 2017/04/21 18:07:04, Nico wrote: > lgtm, but please say "64-bit" in CL description. What's the story for 32-bit? It's 64-bit Linux at the moment. And we don't ship 32-bit Linux, do we? But yes, I will update the description to include 'Linux'.
Description was changed from ========== Enable ThinLTO and LLD for POSIX LTO by default. 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 ========== to ========== 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 ==========
The CQ bit was unchecked by krasin@chromium.org
Ah right, we don't ship 32-bit linux, you're right. (We do have a 32-bit chromeos build, so we're supporting 32-bit linux that way.) On Fri, Apr 21, 2017 at 2:08 PM, <krasin@chromium.org> wrote: > On 2017/04/21 18:07:04, Nico wrote: > > lgtm, but please say "64-bit" in CL description. What's the story for > 32-bit? > > It's 64-bit Linux at the moment. And we don't ship 32-bit Linux, do we? > But yes, I will update the description to include 'Linux'. > > https://codereview.chromium.org/2831213006/ > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
On 2017/04/21 18:07:36, Nico wrote: > (also, please watch bot cycle time on bots that do full builds, since parallel > links might now compete for resources a lot. also, different linker.) Yes. I also plan to submit this on Monday, not today. Just to make sure that the Clang roll from yesterday sticks.
..so what's the plan for arm? :-) My point is "why do we need the explicit check for 64-bit" I suppose. On Fri, Apr 21, 2017 at 2:10 PM, Nico Weber <thakis@chromium.org> wrote: > Ah right, we don't ship 32-bit linux, you're right. (We do have a 32-bit > chromeos build, so we're supporting 32-bit linux that way.) > > On Fri, Apr 21, 2017 at 2:08 PM, <krasin@chromium.org> wrote: > >> On 2017/04/21 18:07:04, Nico wrote: >> > lgtm, but please say "64-bit" in CL description. What's the story for >> 32-bit? >> >> It's 64-bit Linux at the moment. And we don't ship 32-bit Linux, do we? >> But yes, I will update the description to include 'Linux'. >> >> https://codereview.chromium.org/2831213006/ >> > > -- You received this message because you are subscribed to the Google Groups "Chromium-reviews" group. To unsubscribe from this group and stop receiving emails from it, send an email to chromium-reviews+unsubscribe@chromium.org.
The plan is to win on Linux x86-64, and then expand to more platforms and OSes. With FullLTO it was impossible, with ThinLTO - may be, we hope!
lgtm
The CQ bit was checked by krasin@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by thakis@chromium.org
On 2017/04/24 16:15:24, commit-bot: I haz the power wrote: > CQ is trying da patch. > > Follow status at: > https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or... Looks like the roll broke asan on Android, search crbug for "aarya Hans". Let's wait and see what we do about that before landing a dep on the roll.
On 2017/04/24 16:39:14, Nico wrote: > On 2017/04/24 16:15:24, commit-bot: I haz the power wrote: > > CQ is trying da patch. > > > > Follow status at: > > > https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or... > > Looks like the roll broke asan on Android, search crbug for "aarya Hans". Let's > wait and see what we do about that before landing a dep on the roll. I didn't know that. Thank you for saving it.
given the new information (that it has been broken for a month now), I think it's safe to proceed with this CL.
The CQ bit was checked by krasin@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 60001, "attempt_start_ts": 1493064640343500,
"parent_rev": "abcc18157be26d6e43b461f5073d1235a4a21507", "commit_rev":
"7997bbe88aef7eeca37111dcb8426f0430347fb5"}
Message was sent while issue was closed.
Description was changed from ========== 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 ========== to ========== 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/+/7997bbe88aef7eeca37111dcb842... ==========
Message was sent while issue was closed.
Committed patchset #4 (id:60001) as https://chromium.googlesource.com/chromium/src/+/7997bbe88aef7eeca37111dcb842...
Message was sent while issue was closed.
A revert of this CL (patchset #4 id:60001) has been created in https://codereview.chromium.org/2842683002/ by krasin@chromium.org. The reason for reverting is: Code size increase: https://chromeperf.appspot.com/report?sid=7826328365affe1842778994c934cbe08c7... Postmortem is coming. TL;DR: worse dead code elimination in ThinLTO. |
