Skip to content
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.

Sign up for GitHub

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

Jump to bottom

The compiler with debug assertions is really slow #121110

Closed
Noratrieb opened this issue Feb 14, 2024 · 2 comments · Fixed by #121196
Closed

The compiler with debug assertions is really slow #121110

Noratrieb opened this issue Feb 14, 2024 · 2 comments · Fixed by #121196
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@Noratrieb
Copy link
Member

The compiler compiled with debug assertions has gotten really slow recently. For example, compiling the standard library takes way longer than it used to.

This is bad, it makes it way harder to work on rustc.
@Noratrieb Noratrieb added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself labels Feb 14, 2024
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Feb 14, 2024
@Noratrieb Noratrieb changed the title Compile with debug assertions is really slow The compiler with debug assertions is really slow Feb 14, 2024
@Noratrieb Noratrieb removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Feb 14, 2024
@Noratrieb
Copy link
Member Author

This is probably because of the recent changes to assert_unsafe_precondition. It contains a cfg(bootstrap) hack for debug_assertions() that's not inlined. Additionally, it outlines the predicate, which is super slow as well. Fixing the former is easy, fixing the latter is harder and requires adding #[inline(not_the_mir_inliner)].

This was referenced Feb 14, 2024
Closed
Merged
bors added a commit to rust-lang-ci/rust that referenced this issue Feb 14, 2024
@bors
Auto merge of rust-lang#121114 - Nilstrieb:no-inline!, r=<try>
90741d3 
Add `#[rustc_no_mir_inline]` for standard library UB checks

should help with rust-lang#121110 and also with rust-lang#120848

I am not entirely sure whether this is the correct solution and I haven't validated it, I just quickly threw it together before going to sleep.

r? `@saethlin`
@Noratrieb Noratrieb mentioned this issue Feb 16, 2024
Merged
Noratrieb added a commit to Noratrieb/rust that referenced this issue Feb 20, 2024
@Noratrieb
Rollup merge of rust-lang#121196 - Nilstrieb:the-clever-solution, r=s…
265ae67 
…aethlin

Always inline check in `assert_unsafe_precondition` with cfg(debug_assertions)

The current complexities in `assert_unsafe_precondition` are delicately balancing several concerns, among them compile times for the cases where there are no debug assertions. This comes at a large runtime cost when the assertions are enabled, making the debug assertion compiler a lot slower, which is very annoying.

To avoid this, we always inline the check when building with debug assertions.

Numbers (compiling stage1 library after touching core):
- master: 80s
- just adding `#[inline(always)]` to the `cfg(bootstrap)` `debug_assertions` (equivalent to a bootstrap bump (uhh, i just realized that i was on a slightly outdated master so this bump might have happened already), (rust-lang#121112)): 67s
- this: 54s

So this seems like a good solution. I think we can still get the same run-time perf improvements for other users too by massaging this code further (see my other PR about adding `#[rustc_no_mir_inline]` rust-lang#121114) but this is a simpler step that solves the imminent problem of "holy shit my rustc is sooo slow".

Funny consequence: This now means compiling the standard library with dbeug assertions makes it faster (than without, when using debug assertions downstream)!

r? `@saethlin` (or anyone else if someone wants to review this)

fixes rust-lang#121110, supposedly
Noratrieb added a commit to Noratrieb/rust that referenced this issue Feb 20, 2024
@Noratrieb
Rollup merge of rust-lang#121196 - Nilstrieb:the-clever-solution, r=s…
a3519ed 
…aethlin

Always inline check in `assert_unsafe_precondition` with cfg(debug_assertions)

The current complexities in `assert_unsafe_precondition` are delicately balancing several concerns, among them compile times for the cases where there are no debug assertions. This comes at a large runtime cost when the assertions are enabled, making the debug assertion compiler a lot slower, which is very annoying.

To avoid this, we always inline the check when building with debug assertions.

Numbers (compiling stage1 library after touching core):
- master: 80s
- just adding `#[inline(always)]` to the `cfg(bootstrap)` `debug_assertions` (equivalent to a bootstrap bump (uhh, i just realized that i was on a slightly outdated master so this bump might have happened already), (rust-lang#121112)): 67s
- this: 54s

So this seems like a good solution. I think we can still get the same run-time perf improvements for other users too by massaging this code further (see my other PR about adding `#[rustc_no_mir_inline]` rust-lang#121114) but this is a simpler step that solves the imminent problem of "holy shit my rustc is sooo slow".

Funny consequence: This now means compiling the standard library with dbeug assertions makes it faster (than without, when using debug assertions downstream)!

r? ``@saethlin`` (or anyone else if someone wants to review this)

fixes rust-lang#121110, supposedly
@bors bors closed this as completed in 4ff6bb5 Feb 20, 2024
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Feb 20, 2024
@rust-timer
Unrolled build for rust-lang#121196
a405cc6 
Rollup merge of rust-lang#121196 - Nilstrieb:the-clever-solution, r=saethlin

Always inline check in `assert_unsafe_precondition` with cfg(debug_assertions)

The current complexities in `assert_unsafe_precondition` are delicately balancing several concerns, among them compile times for the cases where there are no debug assertions. This comes at a large runtime cost when the assertions are enabled, making the debug assertion compiler a lot slower, which is very annoying.

To avoid this, we always inline the check when building with debug assertions.

Numbers (compiling stage1 library after touching core):
- master: 80s
- just adding `#[inline(always)]` to the `cfg(bootstrap)` `debug_assertions` (equivalent to a bootstrap bump (uhh, i just realized that i was on a slightly outdated master so this bump might have happened already), (rust-lang#121112)): 67s
- this: 54s

So this seems like a good solution. I think we can still get the same run-time perf improvements for other users too by massaging this code further (see my other PR about adding `#[rustc_no_mir_inline]` rust-lang#121114) but this is a simpler step that solves the imminent problem of "holy shit my rustc is sooo slow".

Funny consequence: This now means compiling the standard library with dbeug assertions makes it faster (than without, when using debug assertions downstream)!

r? ```@saethlin``` (or anyone else if someone wants to review this)

fixes rust-lang#121110, supposedly
@Noratrieb
Copy link
Member Author

turns out this was caused by many different factors compounding in an unfortunate way :D
oli's intrinsic changes outlining some important functions, and the unsafe preconditions being quite bad for runtime performance

bors added a commit to rust-lang-ci/rust that referenced this issue Feb 22, 2024
@bors
Auto merge of rust-lang#121114 - Nilstrieb:no-inline!, r=<try>
4ba1a99 
Add `#[rustc_no_mir_inline]` for standard library UB checks

should help with rust-lang#121110 and also with rust-lang#120848

I am not entirely sure whether this is the correct solution and I haven't validated it, I just quickly threw it together before going to sleep.

r? `@saethlin`
bors added a commit to rust-lang-ci/rust that referenced this issue Feb 25, 2024
@bors
Auto merge of rust-lang#121114 - Nilstrieb:no-inline!, r=saethlin
e9f9594 
Add `#[rustc_no_mir_inline]` for standard library UB checks

should help with rust-lang#121110 and also with rust-lang#120848

Because the MIR inliner cannot know whether the checks are enabled or not, so inlining is an unnecessary compile time pessimization when debug assertions are disabled. LLVM knows whether they are enabled or not, so it can optimize accordingly without wasting time.

r? `@saethlin`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-contributor-roadblock Area: Makes things more difficult for new contributors to rust itself C-bug Category: This is a bug. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Always inline check in assert_unsafe_precondition with cfg(debug_assertions) Noratrieb/rust
2 participants
@rustbot @Noratrieb