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

Incremental compilation OOM #139142

Closed
TapGhoul opened this issue Mar 30, 2025 · 7 comments · Fixed by #139153
Closed

Incremental compilation OOM #139142

TapGhoul opened this issue Mar 30, 2025 · 7 comments · Fixed by #139153
Assignees
Labels
A-async-closures `async || {}` A-closures Area: Closures (`|…| { … }`) A-incr-comp Area: Incremental compilation C-bug Category: This is a bug. I-compilemem Issue: Problems and improvements with respect to memory usage during compilation. I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue

Comments

@TapGhoul
Copy link

TapGhoul commented Mar 30, 2025

This comes from the bug report DioxusLabs/dioxus#3903 with DioxusLabs/dioxus#3903 (comment) being especially relevant.

I have no idea how to diagnose this, and there is no real crash - just an OOM that requires the linux OOM killer step in. It feels like the borrow checker freaking out? Probably an incorrect guess
This does not appear to be related to macro expansion, which is what the original report's title says - this seems to occur well after expansion. While the sample code does still contain a macro, expanding beyond this still reproduces the issue.

This occurs both on stable (1.85.1) and latest nightly (920d95eaf 2025-03-28) as of my most recent test.

I'm happy to rewrite this bug report, I just have no idea where to start in actually trying to diagnose this (I lack knowledge on tooling etc for bugs like this) - it eats about 1GB/s if you let it, and ate all 96GB of RAM I have installed, but I'm not sure how to instrument it.
The linked issue has a sample project that reproduces this, with reproduction instructions in the initial bug report.

@TapGhoul TapGhoul added the C-bug Category: This is a bug. label Mar 30, 2025
@rustbot rustbot added the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Mar 30, 2025
@saethlin
Copy link
Member

I have followed the directions and cannot reproduce an OOM on nightly or stable.

@TapGhoul
Copy link
Author

TapGhoul commented Mar 30, 2025

Strange... Is there anything I can do on my end to maybe help get a dump to investigate what may be going on? I tried some heap profiling tools before but I could never get it to actually capture a majority of the data (likely my own inexperience with such tools), so I'm not sure what's going wrong in capturing said dump, so if there's anything I can do to provide something useful let me know.

@TapGhoul
Copy link
Author

TapGhoul commented Mar 30, 2025

If it helps too - there is also a hilariously deep backtrace I can get with a debugger that's at least 600,000 entries deep.

A lot of the lines are just this repeating
0x00007ffff4e46d73 librustc_driver-fdfc7fb14758cf9e.so`rustc_query_impl::query_impl::coroutine_kind::try_collect_active_jobs + 35
0x00007ffff4e2e68c librustc_driver-fdfc7fb14758cf9e.so`<rustc_query_impl::plumbing::QueryCtxt as rustc_query_system::query::QueryContext>::collect_active_jobs + 76
0x00007ffff4d7ec1e librustc_driver-fdfc7fb14758cf9e.so`rustc_query_system::query::plumbing::cycle_error::<rustc_query_impl::DynamicConfig<rustc_query_system::query::caches::DefIdCache<rustc_middle::query::erase::Erased<[u8; 8]>>, false, false, false>, rustc_query_impl::plumbing::QueryCtxt> + 46
0x00007ffff5e30ce6 librustc_driver-fdfc7fb14758cf9e.so`rustc_query_system::query::plumbing::try_execute_query::<rustc_query_impl::DynamicConfig<rustc_query_system::query::caches::DefIdCache<rustc_middle::query::erase::Erased<[u8; 8]>>, false, false, false>, rustc_query_impl::plumbing::QueryCtxt, true> + 14886
0x00007ffff5e2c5ee librustc_driver-fdfc7fb14758cf9e.so`rustc_query_impl::query_impl::def_span::get_query_incr::__rust_end_short_backtrace + 218
0x00007ffff5d8cbe4 librustc_driver-fdfc7fb14758cf9e.so`rustc_middle::query::plumbing::query_get_at::<rustc_query_system::query::caches::DefIdCache<rustc_middle::query::erase::Erased<[u8; 8]>>> (.llvm.5519915731637686349) + 528
0x00007ffff4e30ea8 librustc_driver-fdfc7fb14758cf9e.so`rustc_query_impl::plumbing::create_query_frame::<rustc_span::def_id::DefId> + 424
0x00007ffff4e085d5 librustc_driver-fdfc7fb14758cf9e.so`<rustc_query_impl::query_impl::coroutine_kind::try_collect_active_jobs::{closure#0} as core::ops::function::FnOnce<(rustc_middle::ty::context::TyCtxt, rustc_span::def_id::DefId)>>::call_once + 37
0x00007ffff4d6682a librustc_driver-fdfc7fb14758cf9e.so`<rustc_query_system::query::plumbing::QueryState<rustc_span::def_id::DefId>>::try_collect_active_jobs::<rustc_middle::ty::context::TyCtxt> + 890

@clubby789 clubby789 added I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. A-incr-comp Area: Incremental compilation I-compilemem Issue: Problems and improvements with respect to memory usage during compilation. labels Mar 30, 2025
@cyrgani
Copy link
Contributor

cyrgani commented Mar 30, 2025

I can reproduce this on the latest nightly, will try to minimize it

@cyrgani
Copy link
Contributor

cyrgani commented Mar 30, 2025

reduction, reproducible with cargo clean && cargo build && cargo rustc -- --cfg=a:

fn main() {
    #[cfg(a)]
    || ();

    || {
        Some(())
            .map(|_| ())
            .map(|y| y);
    };

    async || {};
}

@rustbot label S-has-mcve A-closures A-async-closures

@rustbot rustbot added S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue A-async-closures `async || {}` A-closures Area: Closures (`|…| { … }`) labels Mar 30, 2025
@cyrgani
Copy link
Contributor

cyrgani commented Mar 30, 2025

reducing further to leave only closures removes the hang, but generates a nonsensical cycle error (only with incr comp too):

fn main() {
    #[cfg(a)]
    || ();

    || {
        || ();
        || ();
    };

    async || {};
}
error[E0391]: cycle detected when type-checking `main`
 --> src/main.rs:1:1
  |
1 | fn main() {
  | ^^^^^^^^^
  |
  = note: ...which requires looking up span for `main::{closure#1}::{closure#1}`...
  = note: ...which again requires type-checking `main`, completing the cycle
note: cycle used when checking that `main` is well-formed
 --> src/main.rs:1:1
  |
1 | fn main() {
  | ^^^^^^^^^
  = note: see https://rustc-dev-guide.rust-lang.org/overview.html#queries and https://rustc-dev-guide.rust-lang.org/query.html for more information

@compiler-errors
Copy link
Member

Thanks @cyrgani for the great minimizations as always. This seems to have something to do with DefId synthesis and incremental compilation. I'll look into it.

@rustbot claim

@bors bors closed this as completed in b17948a Mar 31, 2025
rust-timer added a commit to rust-lang-ci/rust that referenced this issue Mar 31, 2025
Rollup merge of rust-lang#139153 - compiler-errors:incr-comp-closure, r=oli-obk

Encode synthetic by-move coroutine body with a different `DefPathData`

See the included test. In the first revision rpass1, we have an async closure `{closure#0}` which has a coroutine as a child `{closure#0}::{closure#0}`. We synthesize a by-move coroutine body, which is `{closure#0}::{closure#1}` which depends on the mir_built query, which depends on the typeck query.

In the second revision rpass2, we've replaced the coroutine-closure by a closure with two children closure. Notably, the def path of the second child closure is the same as the synthetic def id from the last revision: `{closure#0}::{closure#1}`. When type-checking this closure, we end up trying to compute its def_span, which tries to fetch it from the incremental cache; this will try to force the dependencies from the last run, which ends up forcing the mir_built query, which ends up forcing the typeck query, which ends up with a query cycle.

The problem here is that we really should never have used the same `DefPathData` for the synthetic by-move coroutine body, since it's not a closure. Changing the `DefPathData` will mean that we can see that the def ids are distinct, which means we won't try to look up the closure's def span from the incremental cache, which will properly skip replaying the node's dependencies and avoid a query cycle.

Fixes rust-lang#139142
@saethlin saethlin removed the needs-triage This issue may need triage. Remove it if it has been sufficiently triaged. label Mar 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-async-closures `async || {}` A-closures Area: Closures (`|…| { … }`) A-incr-comp Area: Incremental compilation C-bug Category: This is a bug. I-compilemem Issue: Problems and improvements with respect to memory usage during compilation. I-crash Issue: The compiler crashes (SIGSEGV, SIGABRT, etc). Use I-ICE instead when the compiler panics. S-has-mcve Status: A Minimal Complete and Verifiable Example has been found for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants