Question about disabled LTO for cargo-auditable

[zamazan4ik]

Hi!

I found that @Shnatsel disabled LTO for the Dist profile, which is used for prebuilt binaries, published on the GitHub Release page with motivation "Don't use LTO for cargo-dist builds to make them consistent with cargo install etc". Could you please elaborate on this a little bit more - how it affects the project?

I can understand motivation when LTO - even the Thin version - is not enabled for cargo install scenarios. You want to reduce compile time on users' machines and make it faster to install on a machines (especially on a weak machine). However, to be honest, if this is the case - cargo binstall should be mentioned as an alternative installation option at very least. But why disable LTO for prebuilt binaries that are later published as binary artifacts on GitHub Release page? Which issues brings this inconsistency in practice, that are worth of disabling LTO in all cases?

I've tried to find any LTO-related issue with the tool in via a search query but didn't find any issue from enabling LTO for the tool itself. However, I found a commit from 7 years ago with enabling FatLTO with some perfomance wins. So something changed and LTO doesn't bring any performance improvement for the tool anymore? If this is the case, the binary size would be definitely improved anyway even nowadays.

I am just trying to understand the logic behind this decision.

Thank you.

[Shnatsel]

Fat LTO is not perfectly tested even today, and sometimes (admittedly rarely) results in incorrect binaries. I really didn't want to ship binaries built in a completely different configuration from the one we use in development and testing, because if it broke we would never know and the users of those binaries would get really bizarre segfaults that I didn't want to debug.

I've tried fat LTO this a few months ago and it does help binary size a lot for cargo-audit, so perhaps this is a good time to re-enable it if the current maintainers are OK with it.

[zamazan4ik]

Fat LTO is not perfectly tested even today, and sometimes (admittedly rarely) results in incorrect binaries. I really didn't want to ship binaries built in a completely different configuration from the one we use in development and testing, because if it broke we would never know and the users of those binaries would get really bizarre segfaults that I didn't want to debug.

I want to elaborate on this topic a bit more. Here I want to set a difference between several separate things: "LTO as an optimization itself", "LTO for C and C++ project" and "LTO for Rust projects".

LTO as an optimization has very long history in GCC and LLVM compilers. I guess LLVM has FatLTO for at least 15 years, and ThinLTO for ~10 years at least (I can be wrong with exact numbers, it's just my rough estimation as for an external person to these projects). So from history perspective regarding time (how many releases passed since the technology introduction, commits, tests passed, etc) - it's kinda well-tested technology. I don't want to argue about "perfectly tested" since it's kinda hard to define it properly :) But let's talk about actual testing at large scales for actual technologies: C/C++ and Rust.

LTO for C and C++ projects is extensively used by open-source communities at very least. E.g. Ubuntu enables FatLTO (see -ffat-lto-objects flag usage) by default for C and C++ projects for around 5 years already. The same thing is true for other distros like OpenSUSE and Fedora. And this is not only for GCC - FatLTO is tested for Clang too. E.g. Fedora enabled FatLTO by default for all Clang-built packages. So at very least, the LTO state was good enough major Linux distros. However, enabling LTO for C and C++ projects in practice brings additional risks about uncovering hidden before LTO Undefined Beahviors (UB) due to the nature of these languages. Probably the best example gentoo-lto project. But even knowing these things, at least some major distros was okay with that.

Regarding LTO for Rust doesn't have the biggest concern from above about UBs uncovered by LTO since Rust language has much less UBs on average compared to C and C++. I've already introduced FatLTO for many Rust projects and found literally zero issues with FatLTO, when FatLTO was enabled for Rust-only part of Rust applications (I am not talking about cross-lang LTO - it has the same issues as "C and C++ (Fat)LTO" from above). I guess the only exception was LTO + PGO issue that was fixed in 1.87 anyway. But let's be honest - PGO is rare best nowadays anyway :) And there are a lot of guides where people for the most optimized profiles enable exactly FatLTO. E.g. official Ubuntu documentation recommends to use FatLTO - so for them FatLTO state in Rustc is okay to recommend it. Of course, it's always possible to find issues like this one, but it's true for any other part of the compiler: ThinLTO-only build error in Rustc, something ThinLTO-related too, or just any other kind of miscompilation in the compiler.

I am sharing all of these pieces of information just trying to convince you, or any other maintainers, that FatLTO state in Rust is completely stable and safe to use for Rust code. I totally understand your wish to not debug bizarre LTO-introduced issues for the tool - but with FatLTO or ThinLTO specifically for Rust it's not the case. Luckily, we have a privilege to not think about it - compared to C and C++ projects. And enabling LTO at the Cargo level enables LTO only for Rust code. I wouldn't say that it's in "perfectly-tested" state, but nothing is in "perfectly-tested" state anyway, even the Rustc compiler itself :)

I've tried fat LTO this a few months ago and it does help binary size a lot for cargo-audit, so perhaps this is a good time to re-enable it if the current maintainers are OK with it.

Thanks a lot for considering it!

[bjorn3]

I would prefer ThinLTO over fat LTO. ThinLTO generally produces executables that are just as fast and sometimes even faster than fat LTO (like for rustc), while being much faster to compile. The only advantage of fat LTO is that it generally produces slightly smaller executables, which is not really relevant outside of embedded and web.

[Shnatsel]

My concern is less about a specific LTO implementation and more about shipping binaries built in a completely an untested configuration. The LTO miscompilation you linked above was caught by CI; we don't have CI with LTO and don't want to have one because it's going to take a very long time.

In Rust, fat LTO is a toss-up in terms of performance. I've seen it regress it as often as improve it in projects I've actually benchmarked. And even the best case 10% performance difference wouldn't really matter for cargo-audit in practice, it's not performance-critical code anyway.

The only thing we stand to gain is binary size, which in cargo-audit case was significant last time I checked.

It would help your case if you could share specific numbers on what exactly cargo audit stands to gain or lose from LTO - binary size, performance measurements with hyperfine, that sort of thing.

[zamazan4ik]

It would help your case if you could share specific numbers on what exactly cargo audit stands to gain or lose from LTO - binary size, performance measurements with hyperfine, that sort of thing.

I am a bit confused regarding cargo audit mention here since it's located in another repo, and has already enabled FatLTO + CU1 optimizations for the Release profile. I guess in this issue we should concentrate only on cargo auditable.

I don't think that for cargo auditable (not for cargo audit - that's another story) we can get any measurable performance improvements, at least I cannot quickly figure out how to benchmark it properly, since it "just" embeds dependency list into a binary. In theory we can implement some sort of microbenchmarks, but I sure that it's not worth it in practice for that kind of project :) However, if you can provide an easy-to-test way for benchmarking it - I would be happy to run some benches to gather performance data points too.

However, I fully agree with the point about binary size - it definitely can be improved, and here are the numbers. Test environment: AMD Ryzen 9 5900x, Fedora 43, Rust 1.95, cargo auditable from the master branch on ab0d04c5ecc490b06f39f485c2239e65782c99ec commit, build command is cargo build -r --bin cargo-auditable, measurements are done on the idle machine (as much as I can guarantee for a standalone idle personal SSH-ed home machine). I additionally tested enabling CU1 since it's kinda usual practice to enable this option with LTO too, and brings binary size improvements even with enabled LTO (even with FatLTO).

Results:

• Current Release profile: 1.44 Mib, clean build time: 7.7s
• Release + ThinLTO: 1.40 Mib, clean build time: 8s
• Release + ThinLTO + CU1: 1.28 Mib, clean build time: 9.6s
• Release + FatLTO: 1.14 Mib, clean build time: 11s
• Release + FatLTO + CU1: 1.13 Mib, clean build time: 12.4s

So from binary size perspective the most optimized version is FatLTO + CU1, ThinLTO in any form cannot achieve this result. And suggested in another thread by @bjorn3 trick with ICF cannot fully reduce this gap between two LTO modes.

Regarding the build time increase. For particular cargo auditable binary we still stay under good-enough compilation time even for cargo install use cases, IMO. If the maintainers think such increase is not worth binary size reduction for cargo install cases - okay, we can enable FatLTO + CU1 only for the Dist profile. Then for cargo install cases lighter default Release binary will be used, and for prebuilt binaries (which are built on GitHub runners and released not-that-frequently) more aggressive profile will be used. And all prebuilt-binaries users (e.g. cargo binstall users) will get a smaller binary by default. However, for particular cargo auditable I don't see an issue with enabling FatLTO + CU1 just for the Release profiles - seems like it's not an issue at all already for cargo audit tool (which has 1m 40s clean build time for the Release profile - just measured it on the same machine). Build time memory consumption for cargo auditable with FatLTO + CU1 also stays under all reasonable limits and will be completely okay for current CI setup.

That's why I suggest to enable the most aggressive from binary size optimization perspective config for the tool - FatLTO + CU1. I ust don't see a reason, why we shouldn't provide for our users literally free improvement. And to address @Shnatsel concerns with lack of CI with LTO - we can easily enable this config on CI too, so no possible LTO-related issues will be missed on CI too.

I would prefer ThinLTO over fat LTO. ThinLTO generally produces executables that are just as fast and sometimes even faster than fat LTO (like for rustc), while being much faster to compile. The only advantage of fat LTO is that it generally produces slightly smaller executables, which is not really relevant outside of embedded and web.

I don't agree with you - we had the same arguments in another thread :) We cannot measure performance concerns for the tool (at least yet) but binary size is important concern for the ecosystem, not only for embedded and web cases - Rust binaries are really infamous for their sizes. And it becomes an issue, when you bundle a lot of the tools in one place - small size losses from all tools quickly pile up into one big bloat. My personal concern from professional practice is providing minimal images with different tools, that used by other people. With such images, we are billed not only for storage but for network traffic too (cloud environments, yeah). You can say that wins are not that big for particular this tool - 300 Kib - but for us it's just a free improvement anyway! We build the tool (and a lot of others) once, and then store/continuously deploy them everywhere. I want to change flags on my side as less frequently as I can - that's why I'm trying to contribute this change to upstreams - so next time, when I will install the tool in a new environment (including my personal setups without access to our private proprietary binary storages), the optimization will be enabled from the day one. And other users will get this benefit (small but a benefit) too.

Binary size is not only my concern - it's also backed up by official Rust survey (see "Large binary size of compiled artifacts") - and I don't think, that all people who voted there are from web and/or embedded domain (that's just a guess since the survey does not provide a way to separate voting results about this question per domain). You can parry it by "Slow compilation is also a concern - see the first row", but that's why I am trying to highlight the prebuilt artifacts point. With using prebuilt binaries, users don't have a concern about compilation time - it's done on a CI machine once, stored there, and a user just need to download it. In this case, the user doesn't care about build time - it's not done on their machine - they just want the most optimized binary. And I see no reason to not provide it - especially when it doesn't require significant maintenance overhead - just changing two lines in the Cargo.toml file.

I hope I provided enough arguments for enabling FatLTO + CU1 for cargo auditable binary in this comment :)

By the way, cargo audit in addition to FatLTO and CU1 also enables strip = "symbols". Probably it would be okay to enable it for cargo auditable too but I don't have strong opinion here since it can somehow influence debuggability of the cargo auditable binary (however, FMPOV, it's okay to enable it for the tool as well).

[Shnatsel]

Oh, sorry, I though this was about cargo-audit since I used to maintain that as well.

For cargo-audit this is defensible since it results in measurable savings, but for cargo-auditable I'm definitely not complicating the build process to save 300kb of disk space. The whole dist thing already requires more maintenance than I'd like and I'm not going to complicate it any more than I absolutely need to.

[zamazan4ik]

For cargo-audit this is defensible since it results in measurable savings, but for cargo-auditable I'm definitely not complicating the build process to save 300kb of disk space. The whole dist thing already requires more maintenance than I'd like and I'm not going to complicate it any more than I absolutely need to.

I don't understand it - what kind of maintenance will be required from adding the following lines to the Cargo.toml file?

[profile.release]
codegen-units = 1
lto = true

These changes will be automatically applied to both Release and Dist profiles (since the Dist profile already inherits the Release profile). If you don't wanna to add these three lines to the project - I am ready to provide this simple PR, and you will be needed just press the Merge button for PR once, and that's it. What kind of additional maintenance burden the proposed change creates for you in the future? If you are not happy with the dist tool and going later to remove it - okay, it would be fine, the proposed change will stay in the Release profile anyway, and no addition work will be required anyway.