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

Exponential (?) time complexity in evaluate_trait_predicate_recursively in rustdoc when proving Send/Sync #106930

Closed
Tracked by #113349
Noratrieb opened this issue Jan 16, 2023 · 14 comments · Fixed by #114321
Assignees
Labels
A-parallel-queries Area: Parallel query execution C-bug Category: This is a bug. P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.

Comments

@Noratrieb
Copy link
Member

Noratrieb commented Jan 16, 2023

There is a latent bug in parallel rustdoc that hangs in x.py doc compiler/rustc_driver --stage=1 when parallel_compiler is enabled.

This is currently blocking #104754 and #106745.

@Noratrieb Noratrieb added T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. C-bug Category: This is a bug. A-parallel-queries Area: Parallel query execution labels Jan 16, 2023
@Noratrieb Noratrieb added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Jan 16, 2023
@m-ou-se
Copy link
Member

m-ou-se commented Jan 16, 2023

From the thread on #106745:

It only happens when rust.parallel-compiler = true is enabled, which explains why it only happens in the -alt builder.

It gets stuck in rustc_trait_selection somehow. Here's a backtrace: backtrace.txt

No clue what is happening.

It looks like this recurses infinitely:

  • evaluate_stack
  • evaluate_candidate
  • evaluate_predicates_recursively
  • evaluate_predicate_recursively
  • evaluate_trait_predicate_recursively
  • evaluate_stack (and repeat)

Opened a Zulip thread about this.

@m-ou-se
Copy link
Member

m-ou-se commented Jan 20, 2023

Temporary fix here: #107121

@m-ou-se
Copy link
Member

m-ou-se commented Jan 24, 2023

Interestingly, ./x.py doc --stage 0 compiler/rustc_middle also gets stuck. I believe that runs the beta (non-parallel) rustdoc. So that'd mean that rustdoc gets stuck when documenting parallel rustc, not that parallel rustdoc gets stuck when documenting rustc.

@m-ou-se
Copy link
Member

m-ou-se commented Jan 24, 2023

Both of the PRs that are blocked by this issue are changes that change the ast.

@m-ou-se
Copy link
Member

m-ou-se commented Jan 24, 2023

It seems it's not an infinite loop, but just bad (exponential?) time complexity somewhere. @oli-obk suggested trying out what happens when explicitly implementing Send and Sync for the new ast node, and that indeed 'fixes' the problem in #106745:

#[cfg(parallel_compiler)]
unsafe impl Sync for FormatArgs {}

#[cfg(parallel_compiler)]
unsafe impl Send for FormatArgs {}

So this issue has almost nothing to do with parallel compiler. It occurs in regular non-parallel rustdoc. It's just that with cfg(parallel_compiler), the ast is Send and Sync, and apparently that takes a huge amount of time to prove when documenting rustc_middle.

(Oli is investigating.)

@m-ou-se m-ou-se changed the title Infinite loop in evaluate_trait_predicate_recursively in rustdoc with parallel compiler Exponential (?) time complexity in evaluate_trait_predicate_recursively in rustdoc when proving Send/Sync Jan 24, 2023
@oli-obk
Copy link
Contributor

oli-obk commented Jan 25, 2023

Data point: Adding the following snippet to rustc_middle will not stop it from compiling successfully, even though documenting the crate will never stop.

#[cfg(parallel_compiler)]
#[allow(rustc::usage_of_qualified_ty)]
pub fn check_for_send(tcx: ty::TyCtxt<'static>) -> impl Send {
    tcx
}

This makes me think the issue is very much rustdoc related, as it appears to get stuck on proving TyCtxt: Send.

@apiraino
Copy link
Contributor

WG-prioritization assigning priority (Zulip discussion) (note: seems not a regression)

@rustbot label -I-prioritize +P-high

@rustbot rustbot added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jan 31, 2023
@jyn514
Copy link
Member

jyn514 commented Feb 13, 2023

cc #79103 (comment)

@SparrowLii
Copy link
Member

SparrowLii commented Apr 27, 2023

#[Edit] I'm trying to solve this problem and what I'm currently observing is that the evaluation_cache does not store all the information when using rustdoc. This causes rustdoc to recalculate many times when collecting auto trait impl and blanket trait impl.

I guess it may be that rustdoc does not calculate the trait implementation on some data structures (such as TyCtxt) in advance, which resulting in too deep recursion

@SparrowLii
Copy link
Member

A revealing phenomenon:
The following struct in rustc_const_eval will cause rustdoc to block while proving that tcx is send:

pub struct AllocRef<'a, 'tcx, Prov: Provenance, Extra, Bytes: AllocBytes = Box<[u8]>> {
    alloc: &'a Allocation<Prov, Extra, Bytes>,
    range: AllocRange,
    tcx: TyCtxt<'tcx>,
    alloc_id: AllocId,
}

But when the generics are instantiated, it will not cause blocking:

pub struct AllocRefFake<'a, 'tcx> {
    _alloc: &'a Allocation<AllocId, (), Box<[u8]>>,
    _range: AllocRange,
    _tcx: TyCtxt<'tcx>,
    _alloc_id: AllocId,
}

So I guess it is param_env that causes the values in evaluation_cache to not be reused. Because this cache needs inserting (predictions, param_env).

@jyn514
Copy link
Member

jyn514 commented Apr 27, 2023

@SparrowLii does this happen even with -Znormalize-docs disabled?

cargo.rustdocflag("-Znormalize-docs");
cmd.arg("-Znormalize-docs");

@jyn514
Copy link
Member

jyn514 commented Apr 27, 2023

If so, this is probably related to get_auto_trait_impls. This is how we determine the param_env:

let param_env = tcx.param_env(item_def_id);

@SparrowLii
Copy link
Member

I can't reproduce this issue now, including my local test

Have there been any recent changes?

@bors bors closed this as completed in 0475873 Aug 2, 2023
github-actions bot pushed a commit to rust-lang/miri that referenced this issue Aug 3, 2023
get auto traits for parallel rustc

test for #106930
#[Edit] Since this doesn't block try build now, we can close rust-lang/rust#106930

fixes #106930
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-parallel-queries Area: Parallel query execution C-bug Category: This is a bug. P-high High priority T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants