build: Drop LTO by default

[cgwalters]

Having lto = true came from the first git commit to this repository. I suspect it was copied from elsewhere.

Having this enabled here makes an incremental (CARGO_INCREMENTAL=1) one-line change build take over a minute.

Disabling it makes that same one-line change take a second.

I think in general we should be deferring decisions like this to "outer" buildsystems; e.g. in Fedora Koji it makes sense to opt-in to full LTO.

[lucab]

Do note that LTO is only enabled for release builds. I think it makes sense to have it on for those builds, as they end up packaged in the OS. It is highly unlikely to have incremental release builds, development loops usually are better/faster through debug builds.

I'm not sure why you picked this one specifically, but we do have the same setup across other projects too.
Though we had to disable it on coreos-installer due to OOM issues: coreos/coreos-installer#728.

[cgwalters]

It is highly unlikely to have incremental release builds, development loops usually are better/faster through debug builds.

Well, the thing is that debug build performance can be really, really bad. I hit this when working on ostree-rs-ext stuff. We've turned lto off in release builds for rpm-ostree as of recently too.

I agree though we should try to be consistent across repositories for this.

[cgwalters]

We also recently did coreos/rpm-ostree#3999

As of recently, there is nicer cargo profile inheritance so we could try to globally add a release-lto across our repositories. That said, there is discussion about enabling lto = "thin" by default which has almost all of the useful effect of the (much more expensive) lto = true.

For reference, https://cs.github.com/?scopeName=All+repos&scope=&q=%22lto+%3D+true%22+org%3Acoreos

So my proposal is basically:

• Across all of our repos, add a release-lto that enables lto = true and use it anywhere we publish release artifacts (RPMs, github release binaries, containers)
• On a per project basis, using lto = "thin" in the release profile (probably worth it for rpm-ostree e.g., I will investigate)

But again debug build performance is too poor in real world cases IMO to suggest using instead of release.

[lucab]

By the way, if it is for one-off local builds I think it should be possible to finely override the LTO setting with CARGO_PROFILE_RELEASE_LTO=false.

[cgwalters]

CARGO_PROFILE_RELEASE_LTO=false.

Interesting. Yes, I could enable that in my dev environments by default. I tested this out with

diff --git a/dot-config/fish/config.fish b/dot-config/fish/config.fish
index dfca858..112fe7b 100644
--- a/dot-config/fish/config.fish
+++ b/dot-config/fish/config.fish
@@ -14,6 +14,8 @@ end
 
 # We want to opt in to this
 export CARGO_INCREMENTAL=1
+# But disable this globally by default since it destroys
+export CARGO_PROFILE_RELEASE_LTO=false
 
 set PATH $PATH /usr/sbin
 for d in $HOME/.local/bin

and indeed it works.

However, the tradeoffs with this are:

• It's something everyone else would also need to learn and enable (OTOH, you already need to do that with CARGO_INCREMENTAL=1...until that gets turned on by default)
• I still think lto = "thin" makes sense by default over lto = true - none of our projects are in a place where the performance difference is relevant

So, adjusted proposal:

• s/lto = true/lto = "thin"/ across all of our repos that enable LTO today

[lucab]

From an out-of-band chat, there is consensus to turn off the currently enabled "fat" LTO across all our CoreOS projects.
Rationale is that it increases build times by a large factor, without much upside as we aren't that performance- or size-sensitive.
Also, if it's not the Rust upstream default, it's more likely to have problems e.g. on non-x86, and LTO problems are annoying to debug.