resolve: Scale back unloading of speculatively loaded crates #121167

petrochenkov · 2024-02-15T21:50:14Z

Fixes #120830 and fixes #120909 while still unblocking #117772.

I cannot reproduce https://github.com/parasyte/crash-rustc as an UI test for some reason, but I tested all the cases linked above manually.

rustbot · 2024-02-15T21:50:21Z

r? @wesleywiser

rustbot has assigned @wesleywiser.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

petrochenkov · 2024-02-15T22:04:41Z

Why #120830 had issues:

CrateNums of unloaded crates are still remain in SourceFiles in source map, and we cannot unload source files (Revert "resolve: Unload speculatively resolved crates before freezing… #121038 (comment)).
Holes in CrateNum list mess up crate num encoding/decoding in encode_crate_deps/resolve_crate_deps, which assumes that they are continuous between 1 and N.

Both issues are potentially fixable.
Uses of SourceFile::cnum can be updated to check for the presence of CrateMetadata (there are just 2 or 3 of them), and the encoding/decoding logic can be rewritten.
I just won't have time to do all this in the next couple of weeks.

There may be other issues though.
Any attempts to re-land the more radical version of crate unloading, resetting crate metadatas to None, will need to go through crater.

cjgillot · 2024-02-15T22:39:24Z

compiler/rustc_middle/src/query/mod.rs

+    query used_crates(_: ()) -> &'tcx [CrateNum] {
+        eval_always
+        desc { "fetching `CrateNum`s for all crates loaded non-speculatively" }
+    }


Should we directly change the implementation of crates?

In some cases the new version should not be used (e.g. in metadata encoding due to #121167 (comment)), but I didn't audit in which exactly and switched only one case that is necessary for #117772.

wesleywiser · 2024-02-16T16:39:42Z

compiler/rustc_middle/src/query/mod.rs

@@ -1872,6 +1872,13 @@ rustc_queries! {
        eval_always
        desc { "fetching all foreign CrateNum instances" }
    }
+    // Crates that are loaded non-speculatively (not for diagnostics or doc links).
+    // FIXME: This is currently only used for collecting lang items, but should be used instead of
+    // `crates` in most other cases too.


Can we describe what situations would require using crates instead or is there not a common theme?

Metadata encoding at least, if the second item in #121167 (comment) is not fixed.
If metadata includes unused crates, then SVH and binary depinfo should probably include them either.

Any diagnostic logic, like trimmed_def_paths may include them as well, because why not.

Lang item collection, impl collection, linking certainly shouldn't include speculatively loaded crates.

wesleywiser · 2024-02-20T02:43:52Z

Thanks for the quick fix @petrochenkov!

@bors r+

bors · 2024-02-20T02:43:54Z

📌 Commit 24cffbf has been approved by wesleywiser

It is now in the queue for this repository.

resolve: Scale back unloading of speculatively loaded crates Fixes rust-lang#120830 and fixes rust-lang#120909 while still unblocking rust-lang#117772. I cannot reproduce https://github.com/parasyte/crash-rustc as an UI test for some reason, but I tested all the cases linked above manually.

Rollup of 8 pull requests Successful merges: - rust-lang#121167 (resolve: Scale back unloading of speculatively loaded crates) - rust-lang#121196 (Always inline check in `assert_unsafe_precondition` with cfg(debug_assertions)) - rust-lang#121206 (Top level error handling) - rust-lang#121223 (intrinsics::simd: add missing functions) - rust-lang#121241 (Implement `NonZero` traits generically.) - rust-lang#121242 (Generate `getelementptr` instead of `inttoptr` for `ptr::invalid`) - rust-lang#121278 (Remove the "codegen" profile from bootstrap) - rust-lang#121286 (Rename `ConstPropLint` to `KnownPanicsLint`) r? `@ghost` `@rustbot` modify labels: rollup

resolve: Scale back unloading of speculatively loaded crates Fixes rust-lang#120830 and fixes rust-lang#120909 while still unblocking rust-lang#117772. I cannot reproduce https://github.com/parasyte/crash-rustc as an UI test for some reason, but I tested all the cases linked above manually.

Rollup of 8 pull requests Successful merges: - rust-lang#121167 (resolve: Scale back unloading of speculatively loaded crates) - rust-lang#121196 (Always inline check in `assert_unsafe_precondition` with cfg(debug_assertions)) - rust-lang#121241 (Implement `NonZero` traits generically.) - rust-lang#121278 (Remove the "codegen" profile from bootstrap) - rust-lang#121286 (Rename `ConstPropLint` to `KnownPanicsLint`) - rust-lang#121291 (target: Revert default to the medium code model on LoongArch targets) - rust-lang#121302 (Remove `RefMutL` hack in `proc_macro::bridge`) - rust-lang#121318 (Trigger `unsafe_code` lint on invocations of `global_asm`) Failed merges: - rust-lang#121206 (Top level error handling) r? `@ghost` `@rustbot` modify labels: rollup

Rollup merge of rust-lang#121167 - petrochenkov:unload2, r=wesleywiser resolve: Scale back unloading of speculatively loaded crates Fixes rust-lang#120830 and fixes rust-lang#120909 while still unblocking rust-lang#117772. I cannot reproduce https://github.com/parasyte/crash-rustc as an UI test for some reason, but I tested all the cases linked above manually.

rustc: Use `tcx.used_crates(())` more And explain when it should be used. Addresses comments from rust-lang#121167.

Rollup merge of rust-lang#124976 - petrochenkov:usedcrates, r=oli-obk rustc: Use `tcx.used_crates(())` more And explain when it should be used. Addresses comments from rust-lang#121167.

rustc: Use `tcx.used_crates(())` more And explain when it should be used. Addresses comments from rust-lang/rust#121167.

rustbot assigned wesleywiser Feb 15, 2024

rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Feb 15, 2024

cjgillot reviewed Feb 15, 2024

View reviewed changes

wesleywiser approved these changes Feb 16, 2024

View reviewed changes

This comment was marked as resolved.

Sign in to view

resolve: Scale back unloading of speculatively loaded crates

24cffbf

petrochenkov force-pushed the unload2 branch from c2f2714 to 24cffbf Compare February 18, 2024 18:35

wesleywiser approved these changes Feb 20, 2024

View reviewed changes

bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Feb 20, 2024

Nilstrieb mentioned this pull request Feb 20, 2024

Rollup of 8 pull requests #121332

Closed

This was referenced Feb 20, 2024

Rollup of 8 pull requests #121339

Closed

Rollup of 8 pull requests #121345

Merged

bors merged commit 073d298 into rust-lang:master Feb 20, 2024
11 checks passed

rustbot added this to the 1.78.0 milestone Feb 20, 2024

Ideflop mentioned this pull request Mar 10, 2024

More implement argument pattern uutils/coreutils#5966

Merged

petrochenkov mentioned this pull request May 10, 2024

rustc: Use tcx.used_crates(()) more #124976

Merged

bors added a commit to rust-lang-ci/rust that referenced this pull request May 22, 2024

Auto merge of rust-lang#124976 - petrochenkov:usedcrates, r=<try>

0cba80b

rustc: Use `tcx.used_crates(())` more And explain when it should be used. Addresses comments from rust-lang#121167.

bors added a commit to rust-lang-ci/rust that referenced this pull request May 22, 2024

Auto merge of rust-lang#124976 - petrochenkov:usedcrates, r=<try>

20a9c4a

rustc: Use `tcx.used_crates(())` more And explain when it should be used. Addresses comments from rust-lang#121167.

matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request May 23, 2024

Rollup merge of rust-lang#124976 - petrochenkov:usedcrates, r=oli-obk

eda4a35

rustc: Use `tcx.used_crates(())` more And explain when it should be used. Addresses comments from rust-lang#121167.

github-actions bot pushed a commit to rust-lang/miri that referenced this pull request May 24, 2024

Rollup merge of #124976 - petrochenkov:usedcrates, r=oli-obk

43c6643

rustc: Use `tcx.used_crates(())` more And explain when it should be used. Addresses comments from rust-lang/rust#121167.

flip1995 pushed a commit to flip1995/rust-clippy that referenced this pull request May 24, 2024

Rollup merge of #124976 - petrochenkov:usedcrates, r=oli-obk

266583a

rustc: Use `tcx.used_crates(())` more And explain when it should be used. Addresses comments from rust-lang/rust#121167.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

resolve: Scale back unloading of speculatively loaded crates #121167

resolve: Scale back unloading of speculatively loaded crates #121167

petrochenkov commented Feb 15, 2024

rustbot commented Feb 15, 2024

petrochenkov commented Feb 15, 2024

cjgillot Feb 15, 2024

petrochenkov Feb 16, 2024

wesleywiser Feb 16, 2024

petrochenkov Feb 18, 2024 •

edited

Loading

This comment was marked as resolved.

wesleywiser commented Feb 20, 2024

bors commented Feb 20, 2024

resolve: Scale back unloading of speculatively loaded crates #121167

resolve: Scale back unloading of speculatively loaded crates #121167

Conversation

petrochenkov commented Feb 15, 2024

rustbot commented Feb 15, 2024

petrochenkov commented Feb 15, 2024

cjgillot Feb 15, 2024

Choose a reason for hiding this comment

petrochenkov Feb 16, 2024

Choose a reason for hiding this comment

wesleywiser Feb 16, 2024

Choose a reason for hiding this comment

petrochenkov Feb 18, 2024 • edited Loading

Choose a reason for hiding this comment

This comment was marked as resolved.

wesleywiser commented Feb 20, 2024

bors commented Feb 20, 2024

petrochenkov Feb 18, 2024 •

edited

Loading