Resolver: Parallelize the import resolution loop by LorrensP-2158466 · Pull Request #158845 · rust-lang/rust

LorrensP-2158466 · 2026-07-06T08:22:15Z

Follow up of #159440. This pr implements the parallel part of par_for_each_slice. And implements DynSend and DynSync for RefOrMut and CmCell to make the call to par_for_each_slice compile.

This is the bare minimum to make the parallel loop work and is not at all optimized, this will follow :).

r? @petrochenkov

LorrensP-2158466 · 2026-07-06T08:22:55Z

Lets see what CI says.

I have tried to add comments to changes to show my thought process behind them, but i'll make another pass here as well to be sure.

petrochenkov · 2026-07-06T16:24:33Z

@bors try @rust-timer queue

…r=<try> Resolver: Parallelize the import resolution loop

rust-bors · 2026-07-06T18:42:55Z

☀️ Try build successful (CI)
Build commit: 3df1fed (3df1fed3aaf1081aa59cfe2228eed289a2ef744e)
Base parent: 3c00c96 (3c00c96d3af4d5b5e101e56cc161a608b21366ee)

…n. When in speculative resolution, only shared borrows are allowed. An actual conditional `RefCell`.

petrochenkov · 2026-07-27T15:21:25Z

is this something that you were thinking of as well?

Same as #158845 (comment), I cannot answer without repeating the work and trying to implement in myself, but the high-level design seems plausible.

petrochenkov · 2026-07-27T15:27:03Z

I've finally read the safety comments in detail, they look more convincing now.
@rustbot author

LorrensP-2158466 · 2026-07-27T17:39:36Z

I've finally read the safety comments in detail, they look more convincing now.

I'll think that I will extend them a bit and add a comment to the ref_mut module as to why we want these (weird) data structures.

…safety) comments for `ref_mut` structures

LorrensP-2158466 · 2026-07-27T19:30:27Z

Updated the comments and used RefCell instead of the manual impl, @rustbot ready.

LorrensP-2158466 · 2026-07-27T19:31:20Z

+    // Soundness depends entirely on proper usage of the `assert_speculative` field in the
+    // `Resolver`. This means that whenever we hold a `CmRef::Normal` or `cell::RefMut` and
+    // flip the switch on `assert_speculative` we also drop these borrows.
    /// A wrapper around `RefCell` semantics that only allows writes (mutable borrows) based on a condition in the resolver.
    #[derive(Default)]
    pub(crate) struct CmRefCell<T> {
-        borrow: Cell<isize>,
-        value: UnsafeCell<T>,
+        inner: RefCell<T>,
    }


Though I am really inclined to just switch between isize and AtomicIsize. That way we can never go wrong with aliased borrows

View changes since the review

petrochenkov · 2026-07-28T11:09:40Z

Benchmarking the changes in #160064.

petrochenkov · 2026-07-28T11:27:38Z

+    //
+    // Thus, these `impl`s rely entirely on safe usage of `RefOrMut::new`.
+    unsafe impl<'a, T: DynSync> DynSync for RefOrMut<'a, T> {}
+    unsafe impl<'a, T: DynSync> DynSend for RefOrMut<'a, T> {}


This DynSend impl is not used anywhere and can be removed.

View changes since the review

The DynSync impl here is also unnecessary, to avoid it we only need a second RefOrMut constructor taking an immutable reference petrochenkov@239c542.

Ooh, nice catch. This would remove the unsafe on RefOrMut::new as well :D.

LorrensP-2158466 · 2026-07-28T12:20:33Z

I'm also tempted to add extra methods for CmCell and CmRefCell that accept a &mut Resolver as a "token" that we can do mutations without checking assert_speculative.

LorrensP-2158466 · 2026-07-28T14:08:33Z

https://github.com/LorrensP-2158466/ref_mut/tree/main has the code for the miri tests, as you will see (and the comments in this pr explain) CmRefCell is not entirely safe.

LorrensP-2158466 · 2026-07-28T15:53:08Z

decided to create a CmToken instead of using &mut Resolver in these things, seems cleaner to me at least. @RutBot ready.

petrochenkov · 2026-07-28T16:10:16Z

#160064 already showed regressions, so let's benchmark and land this piece by piece, starting from ResolutionTable.

Also blocked on #160090.

petrochenkov · 2026-07-28T16:28:25Z

the assert_speculative flag (which needs to be atomic, most likely).

At least right now it probably doesn't need to be atomic though.
assert_speculative = true, par_for_each_slice start, par_for_each_slice end, and assert_speculative = false all happens on the same thread, so there's happens-before relationship between them.
And par_for_each_slice's start and end are supposed to synchronize with everything that happens between them on all the used threads, as I understand.

petrochenkov · 2026-07-28T16:30:29Z

decided to create a CmToken instead of using &mut Resolver in these things, seems cleaner to me at least

Again, we'll need to benchmark whether eliding this boolean assert even matters, considering that it's not even atomic.

LorrensP-2158466 · 2026-07-29T14:15:59Z

I'll split the CmRefCell changes as well (first without token). Okey with you?

petrochenkov · 2026-07-29T14:20:37Z

I'll split the CmRefCell changes as well (first without token). Okey with you?

Yes, after the ResolutionTable lands

rust-bors · 2026-07-30T10:56:19Z

☔ The latest upstream changes (presumably #160190) made this pull request unmergeable. Please resolve the merge conflicts by rebasing.

rustbot added the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jul 6, 2026

rustbot assigned petrochenkov Jul 6, 2026

rustbot added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jul 6, 2026