Repository navigation
rustbuild: Disable multiple CGUs in stable release builds #45444
Description
#45400 enabled multiple CGUs + ThinLTO in rustc builds to speedup bootstrap, however it made the produced compiler slower (see #45400 (comment) and below).
This is desirable for development builds, where build times are priority, but not for stable release builds that happen rarely (so build times don't matter much) and produce binaries used by majority of Rust users.
Enabling full LTO (#45400 (comment)) in stable release builds is also worth investigating.
Activity
Mark-Simulacrum commentedon Oct 23, 2017
Responding to @michaelwoerister from this comment:
I'm personally not opposed to shipping faster stable/beta artifacts -- we already do this by disabling LLVM assertions for example, so I don't see why disabling ThinLTO is a bad thing here. Seems like a fairly clear win, at least until ThinLTO is no longer a loss.
Not that I know of. Travis is problematic due to running our builds always at least once extra and often more than that, but we may be able to get data from @aidanhs. I'd also be interested in relative performance increases locally (building the compiler itself) but that just needs someone to be patient enough to rebuild the compiler with and without this patch.
michaelwoerister commentedon Oct 23, 2017
That would have been my line of argument too.
alexcrichton commentedon Oct 23, 2017
Yes as @michaelwoerister mentioned the current design with dylibs inhibits full LTO (in general procedural macros throw a wrench into full LTO).
For disabling multiple CGUs/ThinLTO it seems fine to do that on release builds which as mentioned already have other tweaks anyway.
aidanhs commentedon Oct 24, 2017
I might be able to get some stats if you let me know what you'd like to compare - before and after a particular commit I assume? There's a lot of noise in Travis (particularly at the moment) so there may not be very high quality results.
Mark-Simulacrum commentedon Nov 15, 2017
Increasing priority to high as I'd prefer that beta didn't have this, but if it slips that doesn't seem to bad as we'll be fine for one more cycle.
Mark-Simulacrum commentedon Jan 15, 2018
@alexcrichton So if I'm correct at least the last stable release shipped with this? I don't think there's been any movement yet... I can probably make an implementation if we'd like to make beta/stable builds get built without ThinLTO. Is that the consensus?
alexcrichton commentedon Jan 16, 2018
@Mark-Simulacrum hm maybe? I'm not sure myself.
In any case seems fine to draft a patch!
Mark-Simulacrum commentedon Jan 16, 2018
I'll assign myself and try to get a patch up soon.
nikomatsakis commentedon Jan 25, 2018
It's hard for me to get a sense of how big the regression actually is. @alexcrichton suggests pretty small? In any case, I do think it makes sense to make our artifacts as good as they can be.
alexcrichton commentedon Jan 25, 2018
@nikomatsakis IIRC yes it was ~5% at most in some cases
Auto merge of #47834 - Mark-Simulacrum:no-cgu-release, r=<try>
Auto merge of #47834 - Mark-Simulacrum:no-cgu-release, r=<try>
Auto merge of #47834 - Mark-Simulacrum:no-cgu-release, r=alexcrichton