From 4bbc385277bcab509c321b1374f72f1ef19d7750 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Tue, 7 Jul 2020 10:14:48 +0200 Subject: [PATCH] Switch to fully dynamically dispatched salsa This improves compile times quite a bit --- Cargo.lock | 8 ++---- crates/ra_db/Cargo.toml | 2 +- crates/ra_db/src/lib.rs | 7 ++--- crates/ra_hir_def/src/data.rs | 2 +- crates/ra_hir_def/src/test_db.rs | 28 +++++++++---------- crates/ra_hir_expand/src/test_db.rs | 26 ++++++++---------- crates/ra_hir_ty/src/db.rs | 1 - crates/ra_hir_ty/src/lower.rs | 2 +- crates/ra_hir_ty/src/test_db.rs | 31 ++++++++++----------- crates/ra_hir_ty/src/tests.rs | 4 +-- crates/ra_hir_ty/src/traits/chalk/tls.rs | 2 +- crates/ra_ide/src/status.rs | 17 +++++------- crates/ra_ide_db/src/change.rs | 20 +++++++------- crates/ra_ide_db/src/lib.rs | 35 ++++++++++++------------ crates/ra_ide_db/src/symbol_index.rs | 8 ++---- 15 files changed, 88 insertions(+), 105 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c23fea77be..54a95621a6 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1490,9 +1490,8 @@ checksum = "71d301d4193d031abdd79ff7e3dd721168a9572ef3fe51a1517aba235bd8f86e" [[package]] name = "salsa" -version = "0.14.4" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "d4ca1c656054666a642affbbc86ab95ed7541125a89f032483d34ee56c0f5390" +version = "0.14.3" +source = "git+https://github.com/nikomatsakis/salsa?branch=dynamic-databases-rfc#fd036a4f154c46253443b3a79b6f4400c40e87b1" dependencies = [ "crossbeam-utils", "indexmap", @@ -1508,8 +1507,7 @@ dependencies = [ [[package]] name = "salsa-macros" version = "0.14.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "038a09b6271446f1123f142fe7e5bef6d4687c4cf82e6986be574c2af3745530" +source = "git+https://github.com/nikomatsakis/salsa?branch=dynamic-databases-rfc#fd036a4f154c46253443b3a79b6f4400c40e87b1" dependencies = [ "heck", "proc-macro2", diff --git a/crates/ra_db/Cargo.toml b/crates/ra_db/Cargo.toml index 372fb242b1..b2d481dfb4 100644 --- a/crates/ra_db/Cargo.toml +++ b/crates/ra_db/Cargo.toml @@ -8,7 +8,7 @@ authors = ["rust-analyzer developers"] doctest = false [dependencies] -salsa = "0.14.1" +salsa = { git = "https://github.com/nikomatsakis/salsa", branch = "dynamic-databases-rfc" } relative-path = "1.0.0" rustc-hash = "1.1.0" diff --git a/crates/ra_db/src/lib.rs b/crates/ra_db/src/lib.rs index 1ddacc1f6c..590efffa44 100644 --- a/crates/ra_db/src/lib.rs +++ b/crates/ra_db/src/lib.rs @@ -113,7 +113,7 @@ pub trait SourceDatabase: CheckCanceled + FileLoader + std::fmt::Debug { fn crate_graph(&self) -> Arc; } -fn parse_query(db: &impl SourceDatabase, file_id: FileId) -> Parse { +fn parse_query(db: &dyn SourceDatabase, file_id: FileId) -> Parse { let _p = profile("parse_query").detail(|| format!("{:?}", file_id)); let text = db.file_text(file_id); SourceFile::parse(&*text) @@ -136,10 +136,7 @@ pub trait SourceDatabaseExt: SourceDatabase { fn source_root_crates(&self, id: SourceRootId) -> Arc>; } -fn source_root_crates( - db: &(impl SourceDatabaseExt + SourceDatabase), - id: SourceRootId, -) -> Arc> { +fn source_root_crates(db: &dyn SourceDatabaseExt, id: SourceRootId) -> Arc> { let graph = db.crate_graph(); let res = graph .iter() diff --git a/crates/ra_hir_def/src/data.rs b/crates/ra_hir_def/src/data.rs index 282ade2a3a..aa335f1e30 100644 --- a/crates/ra_hir_def/src/data.rs +++ b/crates/ra_hir_def/src/data.rs @@ -31,7 +31,7 @@ pub struct FunctionData { } impl FunctionData { - pub(crate) fn fn_data_query(db: &impl DefDatabase, func: FunctionId) -> Arc { + pub(crate) fn fn_data_query(db: &dyn DefDatabase, func: FunctionId) -> Arc { let loc = func.lookup(db); let item_tree = db.item_tree(loc.id.file_id); let func = &item_tree[loc.id.value]; diff --git a/crates/ra_hir_def/src/test_db.rs b/crates/ra_hir_def/src/test_db.rs index 4581d87453..339f819b8b 100644 --- a/crates/ra_hir_def/src/test_db.rs +++ b/crates/ra_hir_def/src/test_db.rs @@ -1,7 +1,7 @@ //! Database used for testing `hir_def`. use std::{ - panic, + fmt, panic, sync::{Arc, Mutex}, }; @@ -18,10 +18,10 @@ use crate::db::DefDatabase; crate::db::InternDatabaseStorage, crate::db::DefDatabaseStorage )] -#[derive(Debug, Default)] +#[derive(Default)] pub struct TestDB { - runtime: salsa::Runtime, - events: Mutex>>>, + storage: salsa::Storage, + events: Mutex>>, } impl Upcast for TestDB { @@ -37,20 +37,20 @@ impl Upcast for TestDB { } impl salsa::Database for TestDB { - fn salsa_runtime(&self) -> &salsa::Runtime { - &self.runtime - } - fn salsa_runtime_mut(&mut self) -> &mut salsa::Runtime { - &mut self.runtime - } - fn salsa_event(&self, event: impl Fn() -> salsa::Event) { + fn salsa_event(&self, event: salsa::Event) { let mut events = self.events.lock().unwrap(); if let Some(events) = &mut *events { - events.push(event()); + events.push(event); } } } +impl fmt::Debug for TestDB { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("TestDB").finish() + } +} + impl panic::RefUnwindSafe for TestDB {} impl FileLoader for TestDB { @@ -78,7 +78,7 @@ impl TestDB { panic!("Can't find module for file") } - pub fn log(&self, f: impl FnOnce()) -> Vec> { + pub fn log(&self, f: impl FnOnce()) -> Vec { *self.events.lock().unwrap() = Some(Vec::new()); f(); self.events.lock().unwrap().take().unwrap() @@ -92,7 +92,7 @@ impl TestDB { // This pretty horrible, but `Debug` is the only way to inspect // QueryDescriptor at the moment. salsa::EventKind::WillExecute { database_key } => { - Some(format!("{:?}", database_key)) + Some(format!("{:?}", database_key.debug(self))) } _ => None, }) diff --git a/crates/ra_hir_expand/src/test_db.rs b/crates/ra_hir_expand/src/test_db.rs index 09fc18c360..332fa556fa 100644 --- a/crates/ra_hir_expand/src/test_db.rs +++ b/crates/ra_hir_expand/src/test_db.rs @@ -1,7 +1,7 @@ //! Database used for testing `hir_expand`. use std::{ - panic, + fmt, panic, sync::{Arc, Mutex}, }; @@ -13,25 +13,23 @@ use rustc_hash::FxHashSet; ra_db::SourceDatabaseStorage, crate::db::AstDatabaseStorage )] -#[derive(Debug, Default)] +#[derive(Default)] pub struct TestDB { - runtime: salsa::Runtime, - events: Mutex>>>, + storage: salsa::Storage, + events: Mutex>>, +} + +impl fmt::Debug for TestDB { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("TestDB").finish() + } } impl salsa::Database for TestDB { - fn salsa_runtime(&self) -> &salsa::Runtime { - &self.runtime - } - - fn salsa_runtime_mut(&mut self) -> &mut salsa::Runtime { - &mut self.runtime - } - - fn salsa_event(&self, event: impl Fn() -> salsa::Event) { + fn salsa_event(&self, event: salsa::Event) { let mut events = self.events.lock().unwrap(); if let Some(events) = &mut *events { - events.push(event()); + events.push(event); } } } diff --git a/crates/ra_hir_ty/src/db.rs b/crates/ra_hir_ty/src/db.rs index dc06c0ee72..84afe0484b 100644 --- a/crates/ra_hir_ty/src/db.rs +++ b/crates/ra_hir_ty/src/db.rs @@ -19,7 +19,6 @@ use crate::{ use hir_expand::name::Name; #[salsa::query_group(HirDatabaseStorage)] -#[salsa::requires(salsa::Database)] pub trait HirDatabase: DefDatabase + Upcast { #[salsa::invoke(infer_wait)] #[salsa::transparent] diff --git a/crates/ra_hir_ty/src/lower.rs b/crates/ra_hir_ty/src/lower.rs index 3dc154e92b..0fa0f7908c 100644 --- a/crates/ra_hir_ty/src/lower.rs +++ b/crates/ra_hir_ty/src/lower.rs @@ -1216,7 +1216,7 @@ pub(crate) fn impl_trait_query(db: &dyn HirDatabase, impl_id: ImplId) -> Option< } pub(crate) fn return_type_impl_traits( - db: &impl HirDatabase, + db: &dyn HirDatabase, def: hir_def::FunctionId, ) -> Option>> { // FIXME unify with fn_sig_for_fn instead of doing lowering twice, maybe diff --git a/crates/ra_hir_ty/src/test_db.rs b/crates/ra_hir_ty/src/test_db.rs index fddf0604d1..dc447955f7 100644 --- a/crates/ra_hir_ty/src/test_db.rs +++ b/crates/ra_hir_ty/src/test_db.rs @@ -1,7 +1,7 @@ //! Database used for testing `hir`. use std::{ - panic, + fmt, panic, sync::{Arc, Mutex}, }; @@ -26,10 +26,15 @@ use crate::{ hir_def::db::DefDatabaseStorage, crate::db::HirDatabaseStorage )] -#[derive(Debug, Default)] +#[derive(Default)] pub struct TestDB { - events: Mutex>>>, - runtime: salsa::Runtime, + storage: salsa::Storage, + events: Mutex>>, +} +impl fmt::Debug for TestDB { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("TestDB").finish() + } } impl Upcast for TestDB { @@ -45,18 +50,10 @@ impl Upcast for TestDB { } impl salsa::Database for TestDB { - fn salsa_runtime(&self) -> &salsa::Runtime { - &self.runtime - } - - fn salsa_runtime_mut(&mut self) -> &mut salsa::Runtime { - &mut self.runtime - } - - fn salsa_event(&self, event: impl Fn() -> salsa::Event) { + fn salsa_event(&self, event: salsa::Event) { let mut events = self.events.lock().unwrap(); if let Some(events) = &mut *events { - events.push(event()); + events.push(event); } } } @@ -64,8 +61,8 @@ impl salsa::Database for TestDB { impl salsa::ParallelDatabase for TestDB { fn snapshot(&self) -> salsa::Snapshot { salsa::Snapshot::new(TestDB { + storage: self.storage.snapshot(), events: Default::default(), - runtime: self.runtime.snapshot(self), }) } } @@ -182,7 +179,7 @@ impl TestDB { } impl TestDB { - pub fn log(&self, f: impl FnOnce()) -> Vec> { + pub fn log(&self, f: impl FnOnce()) -> Vec { *self.events.lock().unwrap() = Some(Vec::new()); f(); self.events.lock().unwrap().take().unwrap() @@ -196,7 +193,7 @@ impl TestDB { // This pretty horrible, but `Debug` is the only way to inspect // QueryDescriptor at the moment. salsa::EventKind::WillExecute { database_key } => { - Some(format!("{:?}", database_key)) + Some(format!("{:?}", database_key.debug(self))) } _ => None, }) diff --git a/crates/ra_hir_ty/src/tests.rs b/crates/ra_hir_ty/src/tests.rs index eeac34d140..69f2d7667e 100644 --- a/crates/ra_hir_ty/src/tests.rs +++ b/crates/ra_hir_ty/src/tests.rs @@ -21,7 +21,7 @@ use hir_def::{ }; use hir_expand::{db::AstDatabase, InFile}; use insta::assert_snapshot; -use ra_db::{fixture::WithFixture, salsa::Database, FileRange, SourceDatabase}; +use ra_db::{fixture::WithFixture, FileRange, SourceDatabase, SourceDatabaseExt}; use ra_syntax::{ algo, ast::{self, AstNode}, @@ -317,7 +317,7 @@ fn typing_whitespace_inside_a_function_should_not_invalidate_types() { " .to_string(); - db.query_mut(ra_db::FileTextQuery).set(pos.file_id, Arc::new(new_text)); + db.set_file_text(pos.file_id, Arc::new(new_text)); { let events = db.log_executed(|| { diff --git a/crates/ra_hir_ty/src/traits/chalk/tls.rs b/crates/ra_hir_ty/src/traits/chalk/tls.rs index 556af70989..e6a9d32116 100644 --- a/crates/ra_hir_ty/src/traits/chalk/tls.rs +++ b/crates/ra_hir_ty/src/traits/chalk/tls.rs @@ -10,7 +10,7 @@ use hir_def::{AdtId, AssocContainerId, DefWithBodyId, Lookup, TypeAliasId}; pub use unsafe_tls::{set_current_program, with_current_program}; -pub struct DebugContext<'a>(&'a (dyn HirDatabase + 'a)); +pub struct DebugContext<'a>(&'a dyn HirDatabase); impl DebugContext<'_> { pub fn debug_struct_id( diff --git a/crates/ra_ide/src/status.rs b/crates/ra_ide/src/status.rs index 45411b357c..08e6f69cb3 100644 --- a/crates/ra_ide/src/status.rs +++ b/crates/ra_ide/src/status.rs @@ -2,10 +2,7 @@ use std::{fmt, iter::FromIterator, sync::Arc}; use hir::MacroFile; use ra_db::{ - salsa::{ - debug::{DebugQueryTable, TableEntry}, - Database, - }, + salsa::debug::{DebugQueryTable, TableEntry}, FileTextQuery, SourceRootId, }; use ra_ide_db::{ @@ -14,15 +11,15 @@ use ra_ide_db::{ }; use ra_prof::{memory_usage, Bytes}; use ra_syntax::{ast, Parse, SyntaxNode}; - -use crate::FileId; use rustc_hash::FxHashMap; +use crate::FileId; + fn syntax_tree_stats(db: &RootDatabase) -> SyntaxTreeStats { - db.query(ra_db::ParseQuery).entries::() + ra_db::ParseQuery.in_db(db).entries::() } fn macro_syntax_tree_stats(db: &RootDatabase) -> SyntaxTreeStats { - db.query(hir::db::ParseMacroQuery).entries::() + hir::db::ParseMacroQuery.in_db(db).entries::() } // Feature: Status @@ -35,10 +32,10 @@ fn macro_syntax_tree_stats(db: &RootDatabase) -> SyntaxTreeStats { // | VS Code | **Rust Analyzer: Status** // |=== pub(crate) fn status(db: &RootDatabase) -> String { - let files_stats = db.query(FileTextQuery).entries::(); + let files_stats = FileTextQuery.in_db(db).entries::(); let syntax_tree_stats = syntax_tree_stats(db); let macro_syntax_tree_stats = macro_syntax_tree_stats(db); - let symbols_stats = db.query(LibrarySymbolsQuery).entries::(); + let symbols_stats = LibrarySymbolsQuery.in_db(db).entries::(); format!( "{}\n{}\n{}\n{} (macros)\n\n\nmemory:\n{}\ngc {:?} seconds ago", files_stats, diff --git a/crates/ra_ide_db/src/change.rs b/crates/ra_ide_db/src/change.rs index 2504d7a338..d8da3f949e 100644 --- a/crates/ra_ide_db/src/change.rs +++ b/crates/ra_ide_db/src/change.rs @@ -147,21 +147,21 @@ impl RootDatabase { let sweep = SweepStrategy::default().discard_values().sweep_all_revisions(); - self.query(ra_db::ParseQuery).sweep(sweep); - self.query(hir::db::ParseMacroQuery).sweep(sweep); + ra_db::ParseQuery.in_db(self).sweep(sweep); + hir::db::ParseMacroQuery.in_db(self).sweep(sweep); // Macros do take significant space, but less then the syntax trees // self.query(hir::db::MacroDefQuery).sweep(sweep); // self.query(hir::db::MacroArgQuery).sweep(sweep); // self.query(hir::db::MacroExpandQuery).sweep(sweep); - self.query(hir::db::AstIdMapQuery).sweep(sweep); + hir::db::AstIdMapQuery.in_db(self).sweep(sweep); - self.query(hir::db::BodyWithSourceMapQuery).sweep(sweep); + hir::db::BodyWithSourceMapQuery.in_db(self).sweep(sweep); - self.query(hir::db::ExprScopesQuery).sweep(sweep); - self.query(hir::db::InferQueryQuery).sweep(sweep); - self.query(hir::db::BodyQuery).sweep(sweep); + hir::db::ExprScopesQuery.in_db(self).sweep(sweep); + hir::db::InferQueryQuery.in_db(self).sweep(sweep); + hir::db::BodyQuery.in_db(self).sweep(sweep); } pub fn per_query_memory_usage(&mut self) -> Vec<(String, Bytes)> { @@ -170,14 +170,14 @@ impl RootDatabase { macro_rules! sweep_each_query { ($($q:path)*) => {$( let before = memory_usage().allocated; - self.query($q).sweep(sweep); + $q.in_db(self).sweep(sweep); let after = memory_usage().allocated; let q: $q = Default::default(); let name = format!("{:?}", q); acc.push((name, before - after)); let before = memory_usage().allocated; - self.query($q).sweep(sweep.discard_everything()); + $q.in_db(self).sweep(sweep.discard_everything()); let after = memory_usage().allocated; let q: $q = Default::default(); let name = format!("{:?} (deps)", q); @@ -252,7 +252,7 @@ impl RootDatabase { // write. // We do this after collecting the non-interned queries to correctly attribute memory used // by interned data. - self.runtime.synthetic_write(Durability::HIGH); + self.salsa_runtime_mut().synthetic_write(Durability::HIGH); sweep_each_query![ // AstDatabase diff --git a/crates/ra_ide_db/src/lib.rs b/crates/ra_ide_db/src/lib.rs index c78071ad6e..6900cac73e 100644 --- a/crates/ra_ide_db/src/lib.rs +++ b/crates/ra_ide_db/src/lib.rs @@ -11,11 +11,11 @@ pub mod imports_locator; pub mod source_change; mod wasm_shims; -use std::sync::Arc; +use std::{fmt, sync::Arc}; use hir::db::{AstDatabase, DefDatabase, HirDatabase}; use ra_db::{ - salsa::{self, Database, Durability}, + salsa::{self, Durability}, Canceled, CheckCanceled, CrateId, FileId, FileLoader, FileLoaderDelegate, SourceDatabase, Upcast, }; @@ -33,13 +33,18 @@ use crate::{line_index::LineIndex, symbol_index::SymbolsDatabase}; hir::db::DefDatabaseStorage, hir::db::HirDatabaseStorage )] -#[derive(Debug)] pub struct RootDatabase { - runtime: salsa::Runtime, + storage: salsa::Storage, pub last_gc: crate::wasm_shims::Instant, pub last_gc_check: crate::wasm_shims::Instant, } +impl fmt::Debug for RootDatabase { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("RootDatabase").finish() + } +} + impl Upcast for RootDatabase { fn upcast(&self) -> &(dyn AstDatabase + 'static) { &*self @@ -71,17 +76,11 @@ impl FileLoader for RootDatabase { } impl salsa::Database for RootDatabase { - fn salsa_runtime(&self) -> &salsa::Runtime { - &self.runtime - } - fn salsa_runtime_mut(&mut self) -> &mut salsa::Runtime { - &mut self.runtime - } fn on_propagated_panic(&self) -> ! { Canceled::throw() } - fn salsa_event(&self, event: impl Fn() -> salsa::Event) { - match event().kind { + fn salsa_event(&self, event: salsa::Event) { + match event.kind { salsa::EventKind::DidValidateMemoizedValue { .. } | salsa::EventKind::WillExecute { .. } => { self.check_canceled(); @@ -100,7 +99,7 @@ impl Default for RootDatabase { impl RootDatabase { pub fn new(lru_capacity: Option) -> RootDatabase { let mut db = RootDatabase { - runtime: salsa::Runtime::default(), + storage: salsa::Storage::default(), last_gc: crate::wasm_shims::Instant::now(), last_gc_check: crate::wasm_shims::Instant::now(), }; @@ -113,16 +112,16 @@ impl RootDatabase { pub fn update_lru_capacity(&mut self, lru_capacity: Option) { let lru_capacity = lru_capacity.unwrap_or(ra_db::DEFAULT_LRU_CAP); - self.query_mut(ra_db::ParseQuery).set_lru_capacity(lru_capacity); - self.query_mut(hir::db::ParseMacroQuery).set_lru_capacity(lru_capacity); - self.query_mut(hir::db::MacroExpandQuery).set_lru_capacity(lru_capacity); + ra_db::ParseQuery.in_db_mut(self).set_lru_capacity(lru_capacity); + hir::db::ParseMacroQuery.in_db_mut(self).set_lru_capacity(lru_capacity); + hir::db::MacroExpandQuery.in_db_mut(self).set_lru_capacity(lru_capacity); } } impl salsa::ParallelDatabase for RootDatabase { fn snapshot(&self) -> salsa::Snapshot { salsa::Snapshot::new(RootDatabase { - runtime: self.runtime.snapshot(self), + storage: self.storage.snapshot(), last_gc: self.last_gc, last_gc_check: self.last_gc_check, }) @@ -134,7 +133,7 @@ pub trait LineIndexDatabase: ra_db::SourceDatabase + CheckCanceled { fn line_index(&self, file_id: FileId) -> Arc; } -fn line_index(db: &impl LineIndexDatabase, file_id: FileId) -> Arc { +fn line_index(db: &dyn LineIndexDatabase, file_id: FileId) -> Arc { let text = db.file_text(file_id); Arc::new(LineIndex::new(&*text)) } diff --git a/crates/ra_ide_db/src/symbol_index.rs b/crates/ra_ide_db/src/symbol_index.rs index 5a09e7d1de..131e2a128d 100644 --- a/crates/ra_ide_db/src/symbol_index.rs +++ b/crates/ra_ide_db/src/symbol_index.rs @@ -87,7 +87,7 @@ impl Query { } #[salsa::query_group(SymbolsDatabaseStorage)] -pub trait SymbolsDatabase: hir::db::HirDatabase + SourceDatabaseExt + ParallelDatabase { +pub trait SymbolsDatabase: hir::db::HirDatabase + SourceDatabaseExt { fn file_symbols(&self, file_id: FileId) -> Arc; fn library_symbols(&self) -> Arc>; /// The set of "local" (that is, from the current workspace) roots. @@ -100,9 +100,7 @@ pub trait SymbolsDatabase: hir::db::HirDatabase + SourceDatabaseExt + ParallelDa fn library_roots(&self) -> Arc>; } -fn library_symbols( - db: &(impl SymbolsDatabase + ParallelDatabase), -) -> Arc> { +fn library_symbols(db: &dyn SymbolsDatabase) -> Arc> { let _p = profile("library_symbols"); let roots = db.library_roots(); @@ -123,7 +121,7 @@ fn library_symbols( Arc::new(res) } -fn file_symbols(db: &impl SymbolsDatabase, file_id: FileId) -> Arc { +fn file_symbols(db: &dyn SymbolsDatabase, file_id: FileId) -> Arc { db.check_canceled(); let parse = db.parse(file_id);