-
Notifications
You must be signed in to change notification settings - Fork 12.9k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Allow disabling optimizations in tests only #8450
Conversation
Hmm, the compiler and stdlib have caught a few regressions for us in the past. I'm a little concerned about giving that up, although I don't have any statistics as to how much coverage we would lose. |
I think what we can do (and I'm happy to do) is configure separate non-opt bots, one that just does a non-opt bootstrap, and one that does an opt bootstrap and runs a non-opt testsuite under it. Is that ok? It will skip the version where a non-opt-built compiler runs the non-opt testsuite, but I think that might be a rare enough corner of the state space to not worry about. |
(IOW parallelize the testing-non-opt-bootstrap and testing-non-opt-testsuite problems between different bots) |
I think that'd be beneficial to both build times and compile times. Perhaps we'd still want snapshots to run the nopt compiler on an opt testsuite just to be 100% sure? |
Doubt it makes much difference. We also don't have a non-opt compiler running an opt testsuite ... diminishing returns. |
I think the tests are slow because the compiler, runtime and standard library are ridiculously slow. The tests themselves generally don't do much work at all, excluding the benchmarks. |
That's fine. |
Since the new runtime landed, the *-nopt builders have increased cycle time by roughly an hour. I have a feeling that this is because the entire runtime is in rust and it's not being optimized at all. In that past with an optimized C++ runtime it looks like things ran faster. This adds the ability to disable optimizations in tests only, not for the entire compiler. This means that the entire compiler and associated libraries will be built with optimizations, but the tests themselves would be built and run without optimizations. This isn't quite as good of a guarantee as disabling optimizations everywhere, but hopefully it'll improve cycle time for the *-nopt builds to move the queue along faster.
Rework `undocumented_unsafe_blocks` fixes: rust-lang#8264 fixes: rust-lang#8449 One thing came up while working on this. Currently comments on the same line are supported like so: ```rust /* SAFETY: reason */ unsafe {} ``` Is this worth supporting at all? Anything other than a couple of words doesn't really fit well. edit: [zulip topic](https://rust-lang.zulipchat.com/#narrow/stream/257328-clippy/topic/.60undocumented_unsafe_blocks.60.20same.20line.20comment) changelog: Don't lint `undocumented_unsafe_blocks` when the unsafe block comes from a proc-macro. changelog: Don't lint `undocumented_unsafe_blocks` when the preceding line has a safety comment and the unsafe block is a sub-expression.
Since the new runtime landed, the *-nopt builders have increased cycle time by roughly an hour. I have a feeling that this is because the entire runtime is in rust and it's not being optimized at all. In that past with an optimized C++ runtime it looks like things ran faster.
This adds the ability to disable optimizations in tests only, not for the entire compiler. This means that the entire compiler and associated libraries will be built with optimizations, but the tests themselves would be built and run without optimizations.
This isn't quite as good of a guarantee as disabling optimizations everywhere, but hopefully it'll improve cycle time for the *-nopt builds to move the queue along faster.