Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2844883002:
Enable LLD for POSIX LTO builds on Linux. (Closed)
DescriptionEnable LLD for POSIX LTO builds on Linux.
This is a stripped down version of (reverted)
change: https://codereview.chromium.org/2831213006/
That change was reverted because ThinLTO regresses the binary
size too much. LLD was a harmless part of the change,
so it's worth enabling it.
If everything is calm, I will enable LLD for regular (non-LTO)
builds on Linux in the next CL.
BUG=607968
Patch Set 1 #
Messages
Total messages: 28 (13 generated)
The CQ bit was checked by krasin@chromium.org to run a CQ dry run
krasin@chromium.org changed reviewers: + dpranke@google.com
Dry run: CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
dpranke@chromium.org changed reviewers: + dpranke@chromium.org
lgtm
On 2017/04/26 23:11:26, Dirk Pranke wrote: > lgtm Do you have any perf numbers how link time compares? (Nice to have for this setup, definitely needed for doing this in regular builds) Also, please watch bot cycle time a bit, the lld bots on Windows are for some reason much slower than the link.exe ones, even though all individual links are much faster locally. Lgtm
Link time are roughly the same. Sometimes LLD is faster, but not by a high margin. At some point I have seen a little degradation but that is claimed to be fixed (and I didn't see it either). I would expect no material changes. As for Windows bots, it's my intent to only change this default on Linux. Supposedly, Windows bots will not be affected. But sure, will keep an eye. Will submit tomorrow morning. On Apr 26, 2017 4:46 PM, <thakis@chromium.org> wrote: > On 2017/04/26 23:11:26, Dirk Pranke wrote: > > lgtm > > Do you have any perf numbers how link time compares? (Nice to have for this > setup, definitely needed for doing this in regular builds) > > Also, please watch bot cycle time a bit, the lld bots on Windows are for > some > reason much slower than the link.exe ones, even though all individual > links are > much faster locally. > > Lgtm > > https://codereview.chromium.org/2844883002/ > -- 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 CQ bit was unchecked by commit-bot@chromium.org
Dry run: Try jobs failed on following builders: linux_android_rel_ng on master.tryserver.chromium.android (JOB_FAILED, https://build.chromium.org/p/tryserver.chromium.android/builders/linux_androi...)
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 commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
So, I slept on this and decided to make a gold-vs-lld comparison before submitting this CL, just in case I have tested on the official + chrome branded configuration with LTO and CFI (the config we ship to the Linux users) Link time for 'chrome' target: Gold + LLVM Gold plugin: 1704 LLD: 5537 (3.2x slower) I have no explanation to this drastic difference. It contradicts to my experience with LLD + Chrome. Possibly, some recent regression? FWIW, these are some numbers from December 2016: https://crbug.com/672158 (but that bug compares cc_unittests, not chrome). I will profile LLD. I feel it's some obscure O(n^2) hiding there. A bug is filed: https://crbug/716209
Message was sent while issue was closed.
Bug url is https://bugs.chromium.org/p/chromium/issues/detail?id=716209
Message was sent while issue was closed.
Description was changed from ========== Enable LLD for POSIX LTO builds on Linux. This is a stripped down version of (reverted) change: https://codereview.chromium.org/2831213006/ That change was reverted because ThinLTO regresses the binary size too much. LLD was a harmless part of the change, so it's worth enabling it. If everything is calm, I will enable LLD for regular (non-LTO) builds on Linux in the next CL. BUG=607968 ========== to ========== Enable LLD for POSIX LTO builds on Linux. This is a stripped down version of (reverted) change: https://codereview.chromium.org/2831213006/ That change was reverted because ThinLTO regresses the binary size too much. LLD was a harmless part of the change, so it's worth enabling it. If everything is calm, I will enable LLD for regular (non-LTO) builds on Linux in the next CL. BUG=607968 ==========
Now we understand what has happened (see the bug: https://bugs.chromium.org/p/chromium/issues/detail?id=716209). Once it's fixed, I will check again and submit this CL. TL;DR: very recent regression (2 days ago) where -Map, function/data sections, LTO and LLD collided.
I intend to flip the commit bit on this once https://codereview.chromium.org/2848113002/ lands.
The CQ bit was checked by pcc@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 commit-bot@chromium.org
The author krasin@chromium.org has not signed Google Contributor License Agreement. Please visit https://cla.developers.google.com to sign and manage CLA.
On 2017/05/12 20:57:37, commit-bot: I haz the power wrote: > The author mailto:krasin@chromium.org has not signed Google Contributor License > Agreement. Please visit https://cla.developers.google.com to sign and manage > CLA. I'll create a new CL unless there is some way to bypass this.
On 2017/05/12 21:04:47, pcc1 wrote: > On 2017/05/12 20:57:37, commit-bot: I haz the power wrote: > > The author mailto:krasin@chromium.org has not signed Google Contributor > License > > Agreement. Please visit https://cla.developers.google.com to sign and manage > > CLA. > > I'll create a new CL unless there is some way to bypass this. We probably need to add krasin to the googlers@ auth group; I'm not sure why he's not already in it. Can someone file a ticket w/ go/bug-a-trooper?
On 2017/05/12 21:11:20, Dirk Pranke wrote: > On 2017/05/12 21:04:47, pcc1 wrote: > > On 2017/05/12 20:57:37, commit-bot: I haz the power wrote: > > > The author mailto:krasin@chromium.org has not signed Google Contributor > > License > > > Agreement. Please visit https://cla.developers.google.com to sign and manage > > > CLA. > > > > I'll create a new CL unless there is some way to bypass this. > > We probably need to add krasin to the googlers@ auth group; I'm not sure why > he's not already in it. > > Can someone file a ticket w/ go/bug-a-trooper? krasin@ left Google before this change could be submitted.
On 2017/05/12 21:11:57, pcc1 wrote: > On 2017/05/12 21:11:20, Dirk Pranke wrote: > > On 2017/05/12 21:04:47, pcc1 wrote: > > > On 2017/05/12 20:57:37, commit-bot: I haz the power wrote: > > > > The author mailto:krasin@chromium.org has not signed Google Contributor > > > License > > > > Agreement. Please visit https://cla.developers.google.com to sign and > manage > > > > CLA. > > > > > > I'll create a new CL unless there is some way to bypass this. > > > > We probably need to add krasin to the googlers@ auth group; I'm not sure why > > he's not already in it. > > > > Can someone file a ticket w/ go/bug-a-trooper? > > krasin@ left Google before this change could be submitted. Oh :( I didn't realize he had left. In that case, the easiest thing to do is re-upload it.
On 2017/05/12 21:13:23, Dirk Pranke (slow) wrote: > On 2017/05/12 21:11:57, pcc1 wrote: > > On 2017/05/12 21:11:20, Dirk Pranke wrote: > > > On 2017/05/12 21:04:47, pcc1 wrote: > > > > On 2017/05/12 20:57:37, commit-bot: I haz the power wrote: > > > > > The author mailto:krasin@chromium.org has not signed Google Contributor > > > > License > > > > > Agreement. Please visit https://cla.developers.google.com to sign and > > manage > > > > > CLA. > > > > > > > > I'll create a new CL unless there is some way to bypass this. > > > > > > We probably need to add krasin to the googlers@ auth group; I'm not sure why > > > he's not already in it. > > > > > > Can someone file a ticket w/ go/bug-a-trooper? > > > > krasin@ left Google before this change could be submitted. > > Oh :( I didn't realize he had left. In that case, the easiest thing to do is > re-upload it. Done: https://codereview.chromium.org/2877913003 |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
