Merge #4934

4934: Remove special casing for library symbols r=matklad a=matklad

We might as well handle them internally, via queries.

I am not sure, but it looks like the current LibraryData setup might
even predate salsa? It's not really needed and creates a bunch of
complexity.



bors r+
🤖

Co-authored-by: Aleksey Kladov <aleksey.kladov@gmail.com>

Author: bors[bot]

crates/ra_ide/src/lib.rs

@@ -82,7 +82,7 @@ pub use ra_db::{
     Canceled, CrateGraph, CrateId, Edition, FileId, FilePosition, FileRange, SourceRootId,
 };
 pub use ra_ide_db::{
-    change::{AnalysisChange, LibraryData},
+    change::AnalysisChange,
     line_index::{LineCol, LineIndex},
     search::SearchScope,
     source_change::{FileSystemEdit, SourceChange, SourceFileEdit},

crates/ra_ide/src/status.rs

@@ -16,6 +16,7 @@ use ra_prof::{memory_usage, Bytes};
 use ra_syntax::{ast, Parse, SyntaxNode};
 
 use crate::FileId;
+use rustc_hash::FxHashMap;
 
 fn syntax_tree_stats(db: &RootDatabase) -> SyntaxTreeStats {
     db.query(ra_db::ParseQuery).entries::<SyntaxTreeStats>()
@@ -123,20 +124,24 @@ struct LibrarySymbolsStats {
 
 impl fmt::Display for LibrarySymbolsStats {
     fn fmt(&self, fmt: &mut fmt::Formatter) -> fmt::Result {
-        write!(fmt, "{} ({}) symbols", self.total, self.size,)
+        write!(fmt, "{} ({}) symbols", self.total, self.size)
     }
 }
 
-impl FromIterator<TableEntry<SourceRootId, Arc<SymbolIndex>>> for LibrarySymbolsStats {
+impl FromIterator<TableEntry<(), Arc<FxHashMap<SourceRootId, SymbolIndex>>>>
+    for LibrarySymbolsStats
+{
     fn from_iter<T>(iter: T) -> LibrarySymbolsStats
     where
-        T: IntoIterator<Item = TableEntry<SourceRootId, Arc<SymbolIndex>>>,
+        T: IntoIterator<Item = TableEntry<(), Arc<FxHashMap<SourceRootId, SymbolIndex>>>>,
     {
         let mut res = LibrarySymbolsStats::default();
         for entry in iter {
             let value = entry.value.unwrap();
-            res.total += value.len();
-            res.size += value.memory_size();
+            for symbols in value.values() {
+                res.total += symbols.len();
+                res.size += symbols.memory_size();
+            }
         }
         res
     }

crates/ra_ide_db/src/change.rs

@@ -9,22 +9,15 @@ use ra_db::{
     SourceRootId,
 };
 use ra_prof::{memory_usage, profile, Bytes};
-use ra_syntax::SourceFile;
-#[cfg(not(feature = "wasm"))]
-use rayon::prelude::*;
 use rustc_hash::FxHashMap;
 
-use crate::{
-    symbol_index::{SymbolIndex, SymbolsDatabase},
-    RootDatabase,
-};
+use crate::{symbol_index::SymbolsDatabase, RootDatabase};
 
 #[derive(Default)]
 pub struct AnalysisChange {
     new_roots: Vec<(SourceRootId, bool)>,
     roots_changed: FxHashMap<SourceRootId, RootChange>,
     files_changed: Vec<(FileId, Arc<String>)>,
-    libraries_added: Vec<LibraryData>,
     crate_graph: Option<CrateGraph>,
 }
 
@@ -40,9 +33,6 @@ impl fmt::Debug for AnalysisChange {
         if !self.files_changed.is_empty() {
             d.field("files_changed", &self.files_changed.len());
         }
-        if !self.libraries_added.is_empty() {
-            d.field("libraries_added", &self.libraries_added.len());
-        }
         if self.crate_graph.is_some() {
             d.field("crate_graph", &self.crate_graph);
         }
@@ -79,10 +69,6 @@ impl AnalysisChange {
         self.roots_changed.entry(root_id).or_default().removed.push(file);
     }
 
-    pub fn add_library(&mut self, data: LibraryData) {
-        self.libraries_added.push(data)
-    }
-
     pub fn set_crate_graph(&mut self, graph: CrateGraph) {
         self.crate_graph = Some(graph);
     }
@@ -116,47 +102,6 @@ impl fmt::Debug for RootChange {
     }
 }
 
-pub struct LibraryData {
-    root_id: SourceRootId,
-    root_change: RootChange,
-    symbol_index: SymbolIndex,
-}
-
-impl fmt::Debug for LibraryData {
-    fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        f.debug_struct("LibraryData")
-            .field("root_id", &self.root_id)
-            .field("root_change", &self.root_change)
-            .field("n_symbols", &self.symbol_index.len())
-            .finish()
-    }
-}
-
-impl LibraryData {
-    pub fn prepare(
-        root_id: SourceRootId,
-        files: Vec<(FileId, RelativePathBuf, Arc<String>)>,
-    ) -> LibraryData {
-        let _p = profile("LibraryData::prepare");
-
-        #[cfg(not(feature = "wasm"))]
-        let iter = files.par_iter();
-        #[cfg(feature = "wasm")]
-        let iter = files.iter();
-
-        let symbol_index = SymbolIndex::for_files(iter.map(|(file_id, _, text)| {
-            let parse = SourceFile::parse(text);
-            (*file_id, parse)
-        }));
-        let mut root_change = RootChange::default();
-        root_change.added = files
-            .into_iter()
-            .map(|(file_id, path, text)| AddFile { file_id, path, text })
-            .collect();
-        LibraryData { root_id, root_change, symbol_index }
-    }
-}
-
 const GC_COOLDOWN: time::Duration = time::Duration::from_millis(100);
 
 impl RootDatabase {
@@ -171,16 +116,20 @@ impl RootDatabase {
         log::info!("apply_change {:?}", change);
         if !change.new_roots.is_empty() {
             let mut local_roots = Vec::clone(&self.local_roots());
+            let mut libraries = Vec::clone(&self.library_roots());
             for (root_id, is_local) in change.new_roots {
                 let root =
                     if is_local { SourceRoot::new_local() } else { SourceRoot::new_library() };
                 let durability = durability(&root);
                 self.set_source_root_with_durability(root_id, Arc::new(root), durability);
                 if is_local {
                     local_roots.push(root_id);
+                } else {
+                    libraries.push(root_id)
                 }
             }
             self.set_local_roots_with_durability(Arc::new(local_roots), Durability::HIGH);
+            self.set_library_roots_with_durability(Arc::new(libraries), Durability::HIGH);
         }
 
         for (root_id, root_change) in change.roots_changed {
@@ -192,24 +141,6 @@ impl RootDatabase {
             let durability = durability(&source_root);
             self.set_file_text_with_durability(file_id, text, durability)
         }
-        if !change.libraries_added.is_empty() {
-            let mut libraries = Vec::clone(&self.library_roots());
-            for library in change.libraries_added {
-                libraries.push(library.root_id);
-                self.set_source_root_with_durability(
-                    library.root_id,
-                    Arc::new(SourceRoot::new_library()),
-                    Durability::HIGH,
-                );
-                self.set_library_symbols_with_durability(
-                    library.root_id,
-                    Arc::new(library.symbol_index),
-                    Durability::HIGH,
-                );
-                self.apply_root_change(library.root_id, library.root_change);
-            }
-            self.set_library_roots_with_durability(Arc::new(libraries), Durability::HIGH);
-        }
         if let Some(crate_graph) = change.crate_graph {
             self.set_crate_graph_with_durability(Arc::new(crate_graph), Durability::HIGH)
         }

crates/ra_ide_db/src/symbol_index.rs

@@ -34,14 +34,15 @@ use ra_db::{
     salsa::{self, ParallelDatabase},
     CrateId, FileId, SourceDatabaseExt, SourceRootId,
 };
+use ra_prof::profile;
 use ra_syntax::{
     ast::{self, NameOwner},
     match_ast, AstNode, Parse, SmolStr, SourceFile,
     SyntaxKind::{self, *},
     SyntaxNode, SyntaxNodePtr, TextRange, WalkEvent,
 };
-#[cfg(not(feature = "wasm"))]
 use rayon::prelude::*;
+use rustc_hash::FxHashMap;
 
 use crate::RootDatabase;
 
@@ -86,10 +87,9 @@ impl Query {
 }
 
 #[salsa::query_group(SymbolsDatabaseStorage)]
-pub trait SymbolsDatabase: hir::db::HirDatabase {
+pub trait SymbolsDatabase: hir::db::HirDatabase + SourceDatabaseExt + ParallelDatabase {
     fn file_symbols(&self, file_id: FileId) -> Arc<SymbolIndex>;
-    #[salsa::input]
-    fn library_symbols(&self, id: SourceRootId) -> Arc<SymbolIndex>;
+    fn library_symbols(&self) -> Arc<FxHashMap<SourceRootId, SymbolIndex>>;
     /// The set of "local" (that is, from the current workspace) roots.
     /// Files in local roots are assumed to change frequently.
     #[salsa::input]
@@ -100,6 +100,29 @@ pub trait SymbolsDatabase: hir::db::HirDatabase {
     fn library_roots(&self) -> Arc<Vec<SourceRootId>>;
 }
 
+fn library_symbols(
+    db: &(impl SymbolsDatabase + ParallelDatabase),
+) -> Arc<FxHashMap<SourceRootId, SymbolIndex>> {
+    let _p = profile("library_symbols");
+
+    let roots = db.library_roots();
+    let res = roots
+        .iter()
+        .map(|&root_id| {
+            let root = db.source_root(root_id);
+            let files = root
+                .walk()
+                .map(|it| (it, SourceDatabaseExt::file_text(db, it)))
+                .collect::<Vec<_>>();
+            let symbol_index = SymbolIndex::for_files(
+                files.into_par_iter().map(|(file, text)| (file, SourceFile::parse(&text))),
+            );
+            (root_id, symbol_index)
+        })
+        .collect();
+    Arc::new(res)
+}
+
 fn file_symbols(db: &impl SymbolsDatabase, file_id: FileId) -> Arc<SymbolIndex> {
     db.check_canceled();
     let parse = db.parse(file_id);
@@ -112,9 +135,9 @@ fn file_symbols(db: &impl SymbolsDatabase, file_id: FileId) -> Arc<SymbolIndex>
 }
 
 /// Need to wrap Snapshot to provide `Clone` impl for `map_with`
-struct Snap(salsa::Snapshot<RootDatabase>);
-impl Clone for Snap {
-    fn clone(&self) -> Snap {
+struct Snap<DB>(DB);
+impl<DB: ParallelDatabase> Clone for Snap<salsa::Snapshot<DB>> {
+    fn clone(&self) -> Snap<salsa::Snapshot<DB>> {
         Snap(self.0.snapshot())
     }
 }
@@ -143,19 +166,11 @@ impl Clone for Snap {
 pub fn world_symbols(db: &RootDatabase, query: Query) -> Vec<FileSymbol> {
     let _p = ra_prof::profile("world_symbols").detail(|| query.query.clone());
 
-    let buf: Vec<Arc<SymbolIndex>> = if query.libs {
-        let snap = Snap(db.snapshot());
-        #[cfg(not(feature = "wasm"))]
-        let buf = db
-            .library_roots()
-            .par_iter()
-            .map_with(snap, |db, &lib_id| db.0.library_symbols(lib_id))
-            .collect();
-
-        #[cfg(feature = "wasm")]
-        let buf = db.library_roots().iter().map(|&lib_id| snap.0.library_symbols(lib_id)).collect();
-
-        buf
+    let tmp1;
+    let tmp2;
+    let buf: Vec<&SymbolIndex> = if query.libs {
+        tmp1 = db.library_symbols();
+        tmp1.values().collect()
     } else {
         let mut files = Vec::new();
         for &root in db.local_roots().iter() {
@@ -164,14 +179,11 @@ pub fn world_symbols(db: &RootDatabase, query: Query) -> Vec<FileSymbol> {
         }
 
         let snap = Snap(db.snapshot());
-        #[cfg(not(feature = "wasm"))]
-        let buf =
-            files.par_iter().map_with(snap, |db, &file_id| db.0.file_symbols(file_id)).collect();
-
-        #[cfg(feature = "wasm")]
-        let buf = files.iter().map(|&file_id| snap.0.file_symbols(file_id)).collect();
-
-        buf
+        tmp2 = files
+            .par_iter()
+            .map_with(snap, |db, &file_id| db.0.file_symbols(file_id))
+            .collect::<Vec<_>>();
+        tmp2.iter().map(|it| &**it).collect()
     };
     query.search(&buf)
 }
@@ -191,14 +203,11 @@ pub fn crate_symbols(db: &RootDatabase, krate: CrateId, query: Query) -> Vec<Fil
 
     let snap = Snap(db.snapshot());
 
-    #[cfg(not(feature = "wasm"))]
     let buf = files
         .par_iter()
         .map_with(snap, |db, &file_id| db.0.file_symbols(file_id))
         .collect::<Vec<_>>();
-
-    #[cfg(feature = "wasm")]
-    let buf = files.iter().map(|&file_id| snap.0.file_symbols(file_id)).collect::<Vec<_>>();
+    let buf = buf.iter().map(|it| &**it).collect::<Vec<_>>();
 
     query.search(&buf)
 }
@@ -245,12 +254,8 @@ impl SymbolIndex {
             lhs_chars.cmp(rhs_chars)
         }
 
-        #[cfg(not(feature = "wasm"))]
         symbols.par_sort_by(cmp);
 
-        #[cfg(feature = "wasm")]
-        symbols.sort_by(cmp);
-
         let mut builder = fst::MapBuilder::memory();
 
         let mut last_batch_start = 0;
@@ -284,7 +289,6 @@ impl SymbolIndex {
         self.map.as_fst().size() + self.symbols.len() * mem::size_of::<FileSymbol>()
     }
 
-    #[cfg(not(feature = "wasm"))]
     pub(crate) fn for_files(
         files: impl ParallelIterator<Item = (FileId, Parse<ast::SourceFile>)>,
     ) -> SymbolIndex {
@@ -294,16 +298,6 @@ impl SymbolIndex {
         SymbolIndex::new(symbols)
     }
 
-    #[cfg(feature = "wasm")]
-    pub(crate) fn for_files(
-        files: impl Iterator<Item = (FileId, Parse<ast::SourceFile>)>,
-    ) -> SymbolIndex {
-        let symbols = files
-            .flat_map(|(file_id, file)| source_file_to_file_symbols(&file.tree(), file_id))
-            .collect::<Vec<_>>();
-        SymbolIndex::new(symbols)
-    }
-
     fn range_to_map_value(start: usize, end: usize) -> u64 {
         debug_assert![start <= (std::u32::MAX as usize)];
         debug_assert![end <= (std::u32::MAX as usize)];
@@ -319,7 +313,7 @@ impl SymbolIndex {
 }
 
 impl Query {
-    pub(crate) fn search(self, indices: &[Arc<SymbolIndex>]) -> Vec<FileSymbol> {
+    pub(crate) fn search(self, indices: &[&SymbolIndex]) -> Vec<FileSymbol> {
         let mut op = fst::map::OpBuilder::new();
         for file_symbols in indices.iter() {
             let automaton = fst::automaton::Subsequence::new(&self.lowercased);