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.

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

polymorphic_recursion.rs takes longer to compile #120912

Closed
DianQK opened this issue Feb 11, 2024 · 5 comments · Fixed by #120942
Closed

polymorphic_recursion.rs takes longer to compile #120912

DianQK opened this issue Feb 11, 2024 · 5 comments · Fixed by #120942
Assignees
Labels
C-bug Category: This is a bug. I-compiletime Issue: Problems and improvements with respect to compile times. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@DianQK
Copy link
Member

DianQK commented Feb 11, 2024

Code

I tried this code:

// skip-filecheck
// Make sure that the MIR inliner does not loop indefinitely on polymorphic recursion.
// compile-flags: --crate-type lib

// Randomize `def_path_hash` by defining them under a module with different names
macro_rules! emit {
    ($($m:ident)*) => {$(
        pub mod $m {
            pub trait Tr { type Next: Tr; }

            pub fn hoge<const N: usize, T: Tr>() {
                inner::<N, T>();
            }

            #[inline(always)]
            fn inner<const N: usize, T: Tr>()
            {
                inner::<N, T::Next>();
                inner::<N, T::Next>();
            }
        }
    )*};
}

// Increase the chance of triggering the bug
emit!(m00 m01 m02 m03 m04 m05 m06 m07 m08 m09 m10 m11 m12 m13 m14 m15 m16 m17 m18 m19);

https://github.com/rust-lang/rust/blob/1.76.0/tests/mir-opt/inline/polymorphic_recursion.rs

I expected to see this happen: It compiles quickly.

Instead, this happened: It takes 10s to complete.

Version with regression

Bisected to #120584.

cc @compiler-errors

@DianQK DianQK added C-bug Category: This is a bug. regression-untriaged Untriaged performance or correctness regression. labels Feb 11, 2024
@rustbot rustbot added I-prioritize Issue: Indicates that prioritization has been requested for this issue. needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Feb 11, 2024
@compiler-errors
Copy link
Member

@DianQK: Why did you title this "is broken"? It just takes longer to compile, or is it broken some other way?

I'll look into the upcoming week.

@compiler-errors compiler-errors self-assigned this Feb 11, 2024
@DianQK
Copy link
Member Author

DianQK commented Feb 11, 2024

@DianQK: Why did you title this "is broken"? It just takes longer to compile, or is it broken some other way?

I'll look into the upcoming week.

Sorry, it just takes longer to compile.

@DianQK DianQK changed the title polymorphic_recursion.rs is broken polymorphic_recursion.rs takes longer to compile Feb 11, 2024
@saethlin saethlin added I-compiletime Issue: Problems and improvements with respect to compile times. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. labels Feb 11, 2024
@apiraino
Copy link
Contributor

WG-prioritization assigning priority (Zulip discussion).

@rustbot label -I-prioritize +P-medium -regression-untriaged

@rustbot rustbot added P-medium Medium priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. regression-untriaged Untriaged performance or correctness regression. labels Feb 12, 2024
bors added a commit to rust-lang-ci/rust that referenced this issue Feb 13, 2024
Ignore own item bounds in parent alias types in `for_each_item_bound`

Fixes rust-lang#120912

I want to get a vibe check on this approach, which is very obviously a hack, but I believe something that is forwards-compatible with a more thorough solution and "good enough for now".

The problem here is that for a really deep rigid associated type, we are now repeatedly considering unrelated item bounds from the parent alias types, meaning we're doing a *lot* of extra work in the MIR inliner for deeply substituted rigid projections.

This feels intimately related to rust-lang#107614. In that PR, we split *supertrait* bounds (bound which share the same `Self` type as the predicate which is being elaborated) and *implied* bounds (anything that is implied by elaborating the predicate).

The problem here is related to the fact that we don't maintain the split between these two for `item_bounds`. If we did, then when recursing into a parent alias type, we'd want to consider only the bounds that are given by [`PredicateFilter::All`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_hir_analysis/astconv/enum.PredicateFilter.html#variant.SelfOnly) **except** those given by [`PredicateFilter::SelfOnly`](https://doc.rust-lang.org/nightly/nightly-rustc/rustc_hir_analysis/astconv/enum.PredicateFilter.html#variant.SelfOnly).
@Zalathar
Copy link
Contributor

Zalathar commented Feb 13, 2024

On my machine (Apple M2 Pro), this test currently takes about 30 seconds.

But if I enable [rust] debug-assertions = true, the compiler appears to consume 100% CPU forever.

I'm not sure whether it's actually stuck, or just taking an incredibly long time, but I've never seen it finish with debug assertions enabled.

@Zalathar
Copy link
Contributor

Zalathar commented Feb 13, 2024

If I apply #120942, the test finishes in about 45 seconds with debug assertions enabled, and about 3 seconds with them disabled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. I-compiletime Issue: Problems and improvements with respect to compile times. P-medium Medium priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants