Disabling LLVM assertions in Nightly builds?

Building rustc (usually) involves building LLVM. One of LLVM’s compile-time configuration is whether to enable assertions. Currently, in rust-lang’s official builds, LLVM assertions are enabled in the Nightly channel, and disabled on Beta and Stable.

I imagine that these assertions help catch some rustc bugs, but they also have a compiler performance cost. In one particular benchmark, they added 16% to Servo’s compilation time in debug mode and 53% in release mode.

“Rust should have a pleasant edit-compile-debug cycle” is one of the top-level items of the 2017 roadmap. +16% compile time is not insignificant. Arguably everyone should eventually switch to the stable compiler where this is not an issue, but we’re not quite there yet. Dropbox, one of the highest-profile users of Rust in production, say that they’re using Nightly and that compile time are significant issue for them.

At the Servo team’s request, the Rust team added “alternate” builds of rustc that have LLVM assertions disabled. These are made available for every PR that lands in the master git branch, but they’re not in the Nightly channel. There is currently no easy way to use them with rustup. One needs to forge a URL from a commit hash and a host triple, then download and extract a tarball. This is relatively straightforward to do in Servo’s bootstrapping scripts, but the only reason we built those at all is that rustup did not exist at the time.

I’ve filed an issue on rustup to add support for these toolchains. In that discussion, @brson said:

having a secret unofficial nightly channel that those in the know use to get a "fast" nightly is not desirable to me. If nighties should be fast we should make them fast.

I take it this suggests disabling LLVM assertions in the Nightly channel (and stop making new “alternate” builds which would be no different from the default ones). From the point of view of Rust users this is the best possible outcome. Nightly users just get a nice compile time boost “for free”.

The obvious downside is that when a bug would trigger an assertion that is disabled, weird things may happen silently (I assume) and the bug would be harder to find. But how valuable is this in practice? Is it important that many users, as opposed to only Rust’s own CI, run these assertions?

1 Like

In the past, I've been opposed to releasing nightly builds without LLVM assertions, but my position is slowly shifting. Still, I'm not sure how many bugs we find this way, but it's very useful when we do -- and I think a lot of them come from the wild, yes. I wonder though if we can achieve similar things through a combination of:

  • offering a "nightly debug" channel that is recommended for use in CI, which would enable such assertions (perhaps it could also enable debug assertions and logging; I regularly get annoyed about not having that)
  • running crater/cargobomb using said switches
2 Likes

Split Nighties in two versions? :slight_smile:

1 Like

There’s a similar situation with Fedora Rawhide kernels, which are built with lots of debug features enabled. For users that want to test rawhide without that performance penalty, there are alternate nodebug builds available.

So I think it would be best to leave the “normal” Rust nightlies with debug enabled, and just make the “alt” version better available for those that really need the performance.

2 Likes

Ideally, the default version used by any developer is faster, and the default version used by CIs has all the tests enabled. Needing to remember to enable it in TravisCI would mean a lot less testing.

2 Likes

We could tweak Travis’s rust.rb so that rust: nightly in .travis.yml is interpreted as “the one with assertions”.

1 Like

Maybe instead of nightly without assertions there should be beta that allows unstable features?

I suspect there’s a lot of users who don’t use nightly because they want to try work-in-progress unstable compiler, but they just want to use gated features.

1 Like

For Servo we want to have a compiler without LLVM assertions without waiting up to 6 weeks for a bug fix to ride the trains.

1 Like

I think my opinion has either changed a bit, or I didn’t express myself clearly, since I made that post.

I’m still not enthusiastic about teaching Rust how to install directly from the rust-ci S3 bucket, but there are other solutions that I think could be fine for rustup.

Contrary to what I said in that issue, I feel pretty good about adding more special-purpose release channels to Rust, e.g. a ‘nightly-fast’ channel. There have been a few other use cases come up recently where the obvious solution to me (as a biased author of rustup) was ‘just publish another release channel’. There seems to be some inertia against this though.

Since I posted my referenced comment on the rustup issue, Rust CI has gained support for publishing try builds to S3 in the same way as the nightly alt builds, and cargobomb has gained custom support (without direct rustup support) for running against them. So that’s no longer a pressing use case for me.

My concerns with teaching rustup about the CI builds are mainly about not having manifests to work with (resulting in degraded and special-cased capabilities), blessing the rust-ci URLs with support, the special naming scheme used by ‘master-alt’ to avoid naming collisions that would need to be hard-coded into rustup, non-discoverability of builds, unsigned toolchains.

Another solution to solve this general problem of pulling builds out of CI and adding them to rustup would be to create a special-purpose tool that downloads those crates by commit and adds them to rustup. It’s pretty easy.

1 Like

Here’s a sketch of a maximal solution that would expose everything Rust CI produces to rustup directly:

1 Like

While exposing everything could potentially be useful, in Servo we default to upgrading to the commit of the current Nigthly, which is not necessarily the latest commit. This avoid the situation (which has happened in the past) where we ask third-party dependencies to upgrade for a breaking change (to unstable features) that hasn’t reached Nightly yet.

So exposing one build of each kind per night/day would be enough for the purpose of "Nightly, but faster".

I’ve heard people say this matter was decided, but that’s not apparent in this thread. Did the compiler team make a decision that’s not stated here?

I see four possible solutions:

  1. The status quo: using both rustup and unstable features requires paying a significant tax on compile times
  2. What this thread originally proposed: disabling LLVM assertions in official Nightly builds and doing nothing else. This is simplest for users, but might reduce opportunities to catch some compiler bugs in various projects’ CI.
  3. What https://github.com/rust-lang-nursery/rustup.rs/issues/1099 first proposed: add support in rustup for two entirely new families of toolchains, named something like master and master-alt, that are addressed by merge commit hash and available for every PR merged into master. The latter has assertions disabled. This is also a subset of what @brson proposed in his last message (“everything Rust CI produces”). This is most complex for users: there’s three flavors of “nightly-like”, and full commit hashes are not super easy to manipulate.
  4. What I proposed more recently in https://github.com/rust-lang-nursery/rustup.rs/issues/1099#issuecomment-319606763, a sort of intermediate: add one new family of toolchains with assertions disabled, named something like nightly-alt and addressed by date like nightly.

I think #4 avoids each downside of the other three.

As a data point for this thread, I believe llvm assertions got enabled on nightlies between me reporting https://github.com/rust-lang/rust/issues/37508 and coming back to it later, and made it massively easier to debug (I hadn’t even thought about LLVM assertions at the time of reporting).

I want to re-raise this issue. This continues to be a place where we can easily make an “easy win” in terms of the compile-time performance experienced by our end-users.

In practice, I think that the number of regression reports that hit LLVM assertions “in the wild” is small. I would appreciate feedback from @compiler_subteam on whether this is true.

The idea would be roughly this:

  • Disable LLVM assertions on nightly builds.
  • Continue to use LLVM assertions on the CI builds prepared by bors and used by crater.
  • Offer some way to easily make use of those builds for end-users.

The last bullet point is a question mark, since we have no means to make this easy right now. But I personally feel like it’s probably enough if, when we see some suspicious crash, we can easily test it with LLVM assertions enabled to see if that diagnoses the problem.

We discussed this in the core team meeting recently and I believe the general feeling was that the above plan was a good idea, modulo resolving the final bullet point. How do people feel about this? I think it’s time we make a final decision.

7 Likes

This sounds great!

Based on previous signals from people in the compiler team I did not expect this outcome, but I think it is the best in terms of direct impact on Rust users.

4 Likes

As long as we still enable LLVM assertions for our CI and crater, I’m OK with turning them off otherwise (though I’m not particularly in favor of doing so).

1 Like

https://github.com/rust-lang/rust/pull/45810 has landed, today’s Nightly should have LLVM assertions disabled. I’m not sure how to double-check that based on binaries, though.

With the last Nightly (gnu64) I see about 15% speedup in optimized compilations, but I see no speedup if I compile with “–emit=metadata”. Do you know why?

If I remember correctly --emit=metadata is used by cargo check and does not emit machine code, right? Then LLVM is not used at all, so it is expected not to see any timing change from this LLVM config change. An optimized build however spends significant time running LLVM’s optimizer, which might previously have been executing many assertions.

2 Likes

I see, that’s a simple answer. I have another question regarding the first part of the compilation (the part done by --emit=metadata). Has someone tried to let rustc leak all memory to see how much memory it eats and how much faster it gets?