From 56572203199216a87f69fa954bb1864a0ee8aade Mon Sep 17 00:00:00 2001 From: Chayim Refael Friedman Date: Mon, 20 Apr 2026 05:30:14 +0300 Subject: [PATCH] Do not check solver's cache validity on every access This should help speed. Instead, we do it only when an `Interner` is constructed, since an interner cannot be held across revisions. There is a problem, though: an interner *can* be held across different databases. To solve that, we also reinit the cache when attaching databases, and take the assumption that everything significant (i.e. that can access the cache) will need to attach the db. This is somewhat risky, but we'll take it for the speed. Theoretically we could *only* reinit on db attach, but it's less risky this way, and with-crate interner construction is rare. --- .../hir-ty/src/next_solver/infer/mod.rs | 4 ++ .../crates/hir-ty/src/next_solver/interner.rs | 44 +++++++++++++++---- 2 files changed, 39 insertions(+), 9 deletions(-) diff --git a/src/tools/rust-analyzer/crates/hir-ty/src/next_solver/infer/mod.rs b/src/tools/rust-analyzer/crates/hir-ty/src/next_solver/infer/mod.rs index a6957c66ff0c..fa7aa518764b 100644 --- a/src/tools/rust-analyzer/crates/hir-ty/src/next_solver/infer/mod.rs +++ b/src/tools/rust-analyzer/crates/hir-ty/src/next_solver/infer/mod.rs @@ -372,6 +372,10 @@ pub fn build_with_canonical( } pub fn build(&mut self, typing_mode: TypingMode<'db>) -> InferCtxt<'db> { + // We do not allow creating an InferCtxt for an Interner without a crate, because this means + // an interner without a crate cannot access the cache, therefore constructing it doesn't need + // to reinit the cache, and we construct a lot of no-crate interners. + self.interner.expect_crate(); let InferCtxtBuilder { interner } = *self; InferCtxt { interner, diff --git a/src/tools/rust-analyzer/crates/hir-ty/src/next_solver/interner.rs b/src/tools/rust-analyzer/crates/hir-ty/src/next_solver/interner.rs index 1078a6af420e..9ad1b3b71227 100644 --- a/src/tools/rust-analyzer/crates/hir-ty/src/next_solver/interner.rs +++ b/src/tools/rust-analyzer/crates/hir-ty/src/next_solver/interner.rs @@ -327,6 +327,7 @@ unsafe impl Sync for DbInterner<'_> {} impl<'db> DbInterner<'db> { // FIXME(next-solver): remove this method pub fn conjure() -> DbInterner<'db> { + // Here we can not reinit the cache since we do that when we attach the db. crate::with_attached_db(|db| DbInterner { db: unsafe { std::mem::transmute::<&dyn HirDatabase, &'db dyn HirDatabase>(db) }, krate: None, @@ -339,10 +340,13 @@ pub fn conjure() -> DbInterner<'db> { /// /// Elaboration is a special kind: it needs lang items (for `Sized`), therefore it needs `new_with()`. pub fn new_no_crate(db: &'db dyn HirDatabase) -> Self { + // We do not reinit the cache here, since anything accessing the cache needs an InferCtxt, + // and we panic when trying to construct an InferCtxt for an Interner without a crate. DbInterner { db, krate: None, lang_items: None } } pub fn new_with(db: &'db dyn HirDatabase, krate: Crate) -> DbInterner<'db> { + tls_cache::reinit_cache(db); DbInterner { db, krate: Some(krate), @@ -1120,7 +1124,8 @@ fn with_global_cache( self, f: impl FnOnce(&mut rustc_type_ir::search_graph::GlobalCache) -> R, ) -> R { - tls_cache::with_cache(self.db, f) + // We make sure to reinit the cache when constructing the Interner. + tls_cache::borrow_assume_valid(self.db, f) } fn canonical_param_env_cache_get_or_insert( @@ -2475,6 +2480,7 @@ fn drop(&mut self) { } let _guard = DbGuard::new(self, db); + super::tls_cache::reinit_cache(db); op() } @@ -2497,10 +2503,14 @@ impl Drop for DbGuard<'_> { #[inline] fn drop(&mut self) { self.state.database.set(self.prev); + if let Some(prev) = self.prev { + super::tls_cache::reinit_cache(unsafe { prev.as_ref() }); + } } } let _guard = DbGuard::new(self, db); + super::tls_cache::reinit_cache(db); op() } @@ -2555,22 +2565,38 @@ struct Cache { static GLOBAL_CACHE: RefCell> = const { RefCell::new(None) }; } - pub(super) fn with_cache<'db, T>( - db: &'db dyn HirDatabase, - f: impl FnOnce(&mut GlobalCache>) -> T, - ) -> T { + pub(super) fn reinit_cache(db: &dyn HirDatabase) { GLOBAL_CACHE.with_borrow_mut(|handle| { let (db_nonce, revision) = db.nonce_and_revision(); - let handle = match handle { + match handle { Some(handle) => { if handle.revision != revision || db_nonce != handle.db_nonce { *handle = Cache { cache: GlobalCache::default(), revision, db_nonce }; } - handle } - None => handle.insert(Cache { cache: GlobalCache::default(), revision, db_nonce }), - }; + None => *handle = Some(Cache { cache: GlobalCache::default(), revision, db_nonce }), + } + }) + } + pub(super) fn borrow_assume_valid<'db, T>( + db: &'db dyn HirDatabase, + f: impl FnOnce(&mut GlobalCache>) -> T, + ) -> T { + if cfg!(debug_assertions) { + let get_state = || { + GLOBAL_CACHE.with_borrow(|handle| { + handle.as_ref().map(|handle| (handle.db_nonce, handle.revision)) + }) + }; + let old_state = get_state(); + reinit_cache(db); + let new_state = get_state(); + assert_eq!(old_state, new_state, "you assumed the cache is valid!"); + } + + GLOBAL_CACHE.with_borrow_mut(|handle| { + let handle = handle.as_mut().expect("you assumed the cache is valid!"); // SAFETY: No idea f(unsafe { std::mem::transmute::<