From d1d0b5a88c666048c21fd225a08170fcc06060e5 Mon Sep 17 00:00:00 2001 From: Aleksey Kladov Date: Thu, 18 Jun 2020 08:29:34 +0200 Subject: Remove special casing for library symbols 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. --- crates/ra_ide/src/lib.rs | 2 +- crates/ra_ide/src/status.rs | 15 ++++-- crates/ra_ide_db/src/change.rs | 79 ++-------------------------- crates/ra_ide_db/src/symbol_index.rs | 90 +++++++++++++++----------------- crates/rust-analyzer/src/global_state.rs | 37 +++---------- crates/rust-analyzer/src/main_loop.rs | 58 ++------------------ 6 files changed, 69 insertions(+), 212 deletions(-) (limited to 'crates') diff --git a/crates/ra_ide/src/lib.rs b/crates/ra_ide/src/lib.rs index 375da1f45..51dc1f041 100644 --- a/crates/ra_ide/src/lib.rs +++ b/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}, diff --git a/crates/ra_ide/src/status.rs b/crates/ra_ide/src/status.rs index 5b7992920..45411b357 100644 --- a/crates/ra_ide/src/status.rs +++ b/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::() @@ -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>> for LibrarySymbolsStats { +impl FromIterator>>> + for LibrarySymbolsStats +{ fn from_iter(iter: T) -> LibrarySymbolsStats where - T: IntoIterator>>, + T: IntoIterator>>>, { 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 } diff --git a/crates/ra_ide_db/src/change.rs b/crates/ra_ide_db/src/change.rs index 2fc796a85..78ee6a515 100644 --- a/crates/ra_ide_db/src/change.rs +++ b/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, files_changed: Vec<(FileId, Arc)>, - libraries_added: Vec, crate_graph: Option, } @@ -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)>, - ) -> 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,6 +116,7 @@ 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() }; @@ -178,9 +124,12 @@ impl RootDatabase { 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) } diff --git a/crates/ra_ide_db/src/symbol_index.rs b/crates/ra_ide_db/src/symbol_index.rs index aab918973..25c99813f 100644 --- a/crates/ra_ide_db/src/symbol_index.rs +++ b/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; - #[salsa::input] - fn library_symbols(&self, id: SourceRootId) -> Arc; + fn library_symbols(&self) -> Arc>; /// 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>; } +fn library_symbols( + db: &(impl SymbolsDatabase + ParallelDatabase), +) -> Arc> { + 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::>(); + 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 { db.check_canceled(); let parse = db.parse(file_id); @@ -112,9 +135,9 @@ fn file_symbols(db: &impl SymbolsDatabase, file_id: FileId) -> Arc } /// Need to wrap Snapshot to provide `Clone` impl for `map_with` -struct Snap(salsa::Snapshot); -impl Clone for Snap { - fn clone(&self) -> Snap { +struct Snap(DB); +impl Clone for Snap> { + fn clone(&self) -> Snap> { Snap(self.0.snapshot()) } } @@ -143,19 +166,11 @@ impl Clone for Snap { pub fn world_symbols(db: &RootDatabase, query: Query) -> Vec { let _p = ra_prof::profile("world_symbols").detail(|| query.query.clone()); - let buf: Vec> = 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 { } 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::>(); + tmp2.iter().map(|it| &**it).collect() }; query.search(&buf) } @@ -191,14 +203,11 @@ pub fn crate_symbols(db: &RootDatabase, krate: CrateId, query: Query) -> Vec>(); - - #[cfg(feature = "wasm")] - let buf = files.iter().map(|&file_id| snap.0.file_symbols(file_id)).collect::>(); + let buf = buf.iter().map(|it| &**it).collect::>(); 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::() } - #[cfg(not(feature = "wasm"))] pub(crate) fn for_files( files: impl ParallelIterator)>, ) -> SymbolIndex { @@ -294,16 +298,6 @@ impl SymbolIndex { SymbolIndex::new(symbols) } - #[cfg(feature = "wasm")] - pub(crate) fn for_files( - files: impl Iterator)>, - ) -> SymbolIndex { - let symbols = files - .flat_map(|(file_id, file)| source_file_to_file_symbols(&file.tree(), file_id)) - .collect::>(); - 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]) -> Vec { + pub(crate) fn search(self, indices: &[&SymbolIndex]) -> Vec { let mut op = fst::map::OpBuilder::new(); for file_symbols in indices.iter() { let automaton = fst::automaton::Subsequence::new(&self.lowercased); diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs index ef6c7d44d..ee1d37ea2 100644 --- a/crates/rust-analyzer/src/global_state.rs +++ b/crates/rust-analyzer/src/global_state.rs @@ -12,12 +12,9 @@ use crossbeam_channel::{unbounded, Receiver}; use lsp_types::Url; use parking_lot::RwLock; use ra_flycheck::{Flycheck, FlycheckConfig}; -use ra_ide::{ - Analysis, AnalysisChange, AnalysisHost, CrateGraph, FileId, LibraryData, SourceRootId, -}; +use ra_ide::{Analysis, AnalysisChange, AnalysisHost, CrateGraph, FileId, SourceRootId}; use ra_project_model::{CargoWorkspace, ProcMacroClient, ProjectWorkspace, Target}; use ra_vfs::{LineEndings, RootEntry, Vfs, VfsChange, VfsFile, VfsTask, Watch}; -use relative_path::RelativePathBuf; use stdx::format_to; use crate::{ @@ -195,32 +192,18 @@ impl GlobalState { /// Returns a vec of libraries /// FIXME: better API here - pub fn process_changes( - &mut self, - roots_scanned: &mut usize, - ) -> Option)>)>> { + pub fn process_changes(&mut self, roots_scanned: &mut usize) -> bool { let changes = self.vfs.write().commit_changes(); if changes.is_empty() { - return None; + return false; } - let mut libs = Vec::new(); let mut change = AnalysisChange::new(); for c in changes { match c { VfsChange::AddRoot { root, files } => { - let root_path = self.vfs.read().root2path(root); - let is_local = self.local_roots.iter().any(|r| root_path.starts_with(r)); - if is_local { - *roots_scanned += 1; - for (file, path, text) in files { - change.add_file(SourceRootId(root.0), FileId(file.0), path, text); - } - } else { - let files = files - .into_iter() - .map(|(vfsfile, path, text)| (FileId(vfsfile.0), path, text)) - .collect(); - libs.push((SourceRootId(root.0), files)); + *roots_scanned += 1; + for (file, path, text) in files { + change.add_file(SourceRootId(root.0), FileId(file.0), path, text); } } VfsChange::AddFile { root, file, path, text } => { @@ -235,13 +218,7 @@ impl GlobalState { } } self.analysis_host.apply_change(change); - Some(libs) - } - - pub fn add_lib(&mut self, data: LibraryData) { - let mut change = AnalysisChange::new(); - change.add_library(data); - self.analysis_host.apply_change(change); + true } pub fn snapshot(&self) -> GlobalStateSnapshot { diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs index 80cfd3c28..08b0a5a16 100644 --- a/crates/rust-analyzer/src/main_loop.rs +++ b/crates/rust-analyzer/src/main_loop.rs @@ -24,11 +24,10 @@ use lsp_types::{ WorkDoneProgressReport, }; use ra_flycheck::{CheckTask, Status}; -use ra_ide::{Canceled, FileId, LibraryData, LineIndex, SourceRootId}; +use ra_ide::{Canceled, FileId, LineIndex}; use ra_prof::profile; use ra_project_model::{PackageRoot, ProjectWorkspace}; use ra_vfs::{VfsTask, Watch}; -use relative_path::RelativePathBuf; use rustc_hash::FxHashSet; use serde::{de::DeserializeOwned, Serialize}; use threadpool::ThreadPool; @@ -174,12 +173,10 @@ pub fn main_loop(config: Config, connection: Connection) -> Result<()> { let pool = ThreadPool::default(); let (task_sender, task_receiver) = unbounded::(); - let (libdata_sender, libdata_receiver) = unbounded::(); log::info!("server initialized, serving requests"); { let task_sender = task_sender; - let libdata_sender = libdata_sender; loop { log::trace!("selecting"); let event = select! { @@ -192,7 +189,6 @@ pub fn main_loop(config: Config, connection: Connection) -> Result<()> { Ok(task) => Event::Vfs(task), Err(RecvError) => return Err("vfs died".into()), }, - recv(libdata_receiver) -> data => Event::Lib(data.unwrap()), recv(global_state.flycheck.as_ref().map_or(&never(), |it| &it.task_recv)) -> task => match task { Ok(task) => Event::CheckWatcher(task), Err(RecvError) => return Err("check watcher died".into()), @@ -203,15 +199,7 @@ pub fn main_loop(config: Config, connection: Connection) -> Result<()> { break; }; } - loop_turn( - &pool, - &task_sender, - &libdata_sender, - &connection, - &mut global_state, - &mut loop_state, - event, - )?; + loop_turn(&pool, &task_sender, &connection, &mut global_state, &mut loop_state, event)?; } } global_state.analysis_host.request_cancellation(); @@ -219,7 +207,6 @@ pub fn main_loop(config: Config, connection: Connection) -> Result<()> { task_receiver.into_iter().for_each(|task| { on_task(task, &connection.sender, &mut loop_state.pending_requests, &mut global_state) }); - libdata_receiver.into_iter().for_each(drop); log::info!("...tasks have finished"); log::info!("joining threadpool..."); pool.join(); @@ -243,7 +230,6 @@ enum Event { Msg(Message), Task(Task), Vfs(VfsTask), - Lib(LibraryData), CheckWatcher(CheckTask), } @@ -279,7 +265,6 @@ impl fmt::Debug for Event { Event::Msg(it) => fmt::Debug::fmt(it, f), Event::Task(it) => fmt::Debug::fmt(it, f), Event::Vfs(it) => fmt::Debug::fmt(it, f), - Event::Lib(it) => fmt::Debug::fmt(it, f), Event::CheckWatcher(it) => fmt::Debug::fmt(it, f), } } @@ -291,10 +276,6 @@ struct LoopState { pending_responses: FxHashSet, pending_requests: PendingRequests, subscriptions: Subscriptions, - // We try not to index more than MAX_IN_FLIGHT_LIBS libraries at the same - // time to always have a thread ready to react to input. - in_flight_libraries: usize, - pending_libraries: Vec<(SourceRootId, Vec<(FileId, RelativePathBuf, Arc)>)>, workspace_loaded: bool, roots_progress_reported: Option, roots_scanned: usize, @@ -315,7 +296,6 @@ impl LoopState { fn loop_turn( pool: &ThreadPool, task_sender: &Sender, - libdata_sender: &Sender, connection: &Connection, global_state: &mut GlobalState, loop_state: &mut LoopState, @@ -339,12 +319,6 @@ fn loop_turn( Event::Vfs(task) => { global_state.vfs.write().handle_task(task); } - Event::Lib(lib) => { - global_state.add_lib(lib); - global_state.maybe_collect_garbage(); - loop_state.in_flight_libraries -= 1; - loop_state.roots_scanned += 1; - } Event::CheckWatcher(task) => on_check_task(task, global_state, task_sender)?, Event::Msg(msg) => match msg { Message::Request(req) => on_request( @@ -390,36 +364,12 @@ fn loop_turn( }, }; - let mut state_changed = false; - if let Some(changes) = global_state.process_changes(&mut loop_state.roots_scanned) { - state_changed = true; - loop_state.pending_libraries.extend(changes); - } - - let max_in_flight_libs = pool.max_count().saturating_sub(2).max(1); - while loop_state.in_flight_libraries < max_in_flight_libs { - let (root, files) = match loop_state.pending_libraries.pop() { - Some(it) => it, - None => break, - }; - - loop_state.in_flight_libraries += 1; - let sender = libdata_sender.clone(); - pool.execute(move || { - log::info!("indexing {:?} ... ", root); - let data = LibraryData::prepare(root, files); - sender.send(data).unwrap(); - }); - } + let mut state_changed = global_state.process_changes(&mut loop_state.roots_scanned); let show_progress = !loop_state.workspace_loaded && global_state.config.client_caps.work_done_progress; - if !loop_state.workspace_loaded - && loop_state.roots_scanned == loop_state.roots_total - && loop_state.pending_libraries.is_empty() - && loop_state.in_flight_libraries == 0 - { + if !loop_state.workspace_loaded && loop_state.roots_scanned == loop_state.roots_total { state_changed = true; loop_state.workspace_loaded = true; if let Some(flycheck) = &global_state.flycheck { -- cgit v1.2.3