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.

@bors try @rust-timer queue

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

I meant that we need read access during speculative resolution and interior mutability elsewhere for things wrapped in CmRefCell.

Yes, but it doesn't mean the data in CmRefCell needs to be additionally protected by a mutex or rwlock.
It is already protected by the assert_speculative flag (which needs to be atomic, most likely).
I guess the main question is how to express it in Rust terms, without plaguing the whole resolver with unsafe.

After a lot of brainstorming I came up with an API like this (not detailed though):

/// A cell that supports:
/// - mutable access in normal resolution (single-threaded)
/// - shared access in speculative resolution (multi-threaded)
struct CmRefCell<T> {
    // mimic RefCell but with some tricks.
}

enum CmBorrow<'a, T> {
    /// Normal resolution: RefCell-like borrow tracking
    Normal(Ref<'a, T>),

    /// Speculative resolution: plain shared reference
    Speculative(&'a T),
}

impl<T> CmRefCell<T> {
    fn borrow(&self, resolver: &CmResolver<'_, '_, '_>) -> CmBorrow<'_, T> {
        if resolver.is_speculative() {
            CmBorrow::Speculative(/* shared access */)
        } else {
            CmBorrow::Normal(/* RefCell borrow */)
        }
    }

    fn borrow_mut(&self, resolver: &CmResolver<'_, '_, '_>) -> RefMut<'_, T> {
        assert!(!resolver.is_speculative());
        /* mutable access */
    }
}

Which has no synchronization because we do not need to mutate anything during speculative resolution, but we can still read. During non-speculative resolution we have the semantics we always had and Deref to &T for CmBorrow should be "zero-cost" if the &T are stored in the same place in both variants (otherwise a union?)

I have a feeling this could work, is this something that you were thinking of as well? Though unsafe is required here.

I am currently merging it over to a seperate crate to check with miri, but in the case you want to take a look and maybe do a perf run, @rustbot ready.

Note it is not done, i am still working on it.

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.

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

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.

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

Benchmarking the changes in #160064.

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.

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.

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

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

Also blocked on #160090.

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.

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.