From 05aa5b854b230c2f6ba182c0f2c94f3de9a47204 Mon Sep 17 00:00:00 2001 From: Emil Lauridsen Date: Thu, 23 Jan 2020 09:26:08 +0100 Subject: Remove RWLock from check watcher. @matklad mentioned this might be a good idea. So the general idea is that we don't really need the lock, as we can just clone the check watcher state when creating a snapshot. We can then use `Arc::get_mut` to get mutable access to the state from `WorldState` when needed. Running with this it seems to improve responsiveness a bit while cargo is running, but I have no hard numbers to prove it. In any case, a serialization point less is always better when we're trying to be responsive. --- crates/ra_cargo_watch/src/conv.rs | 2 +- crates/ra_cargo_watch/src/lib.rs | 9 ++++----- crates/ra_lsp_server/src/main_loop.rs | 14 +++++++------- crates/ra_lsp_server/src/main_loop/handlers.rs | 5 ++--- crates/ra_lsp_server/src/world.rs | 4 ++-- 5 files changed, 16 insertions(+), 18 deletions(-) (limited to 'crates') diff --git a/crates/ra_cargo_watch/src/conv.rs b/crates/ra_cargo_watch/src/conv.rs index ac0f1d28a..8fba400ae 100644 --- a/crates/ra_cargo_watch/src/conv.rs +++ b/crates/ra_cargo_watch/src/conv.rs @@ -117,7 +117,7 @@ fn is_deprecated(rd: &RustDiagnostic) -> bool { } } -#[derive(Debug)] +#[derive(Clone, Debug)] pub struct SuggestedFix { pub title: String, pub location: Location, diff --git a/crates/ra_cargo_watch/src/lib.rs b/crates/ra_cargo_watch/src/lib.rs index 7f4c9280c..bbe634603 100644 --- a/crates/ra_cargo_watch/src/lib.rs +++ b/crates/ra_cargo_watch/src/lib.rs @@ -7,7 +7,6 @@ use lsp_types::{ Diagnostic, Url, WorkDoneProgress, WorkDoneProgressBegin, WorkDoneProgressEnd, WorkDoneProgressReport, }; -use parking_lot::RwLock; use std::{ collections::HashMap, path::PathBuf, @@ -38,7 +37,7 @@ pub struct CheckOptions { #[derive(Debug)] pub struct CheckWatcher { pub task_recv: Receiver, - pub state: Arc>, + pub state: Arc, cmd_send: Option>, handle: Option>, } @@ -46,7 +45,7 @@ pub struct CheckWatcher { impl CheckWatcher { pub fn new(options: &CheckOptions, workspace_root: PathBuf) -> CheckWatcher { let options = options.clone(); - let state = Arc::new(RwLock::new(CheckState::new())); + let state = Arc::new(CheckState::new()); let (task_send, task_recv) = unbounded::(); let (cmd_send, cmd_recv) = unbounded::(); @@ -59,7 +58,7 @@ impl CheckWatcher { /// Returns a CheckWatcher that doesn't actually do anything pub fn dummy() -> CheckWatcher { - let state = Arc::new(RwLock::new(CheckState::new())); + let state = Arc::new(CheckState::new()); CheckWatcher { task_recv: never(), cmd_send: None, handle: None, state } } @@ -87,7 +86,7 @@ impl std::ops::Drop for CheckWatcher { } } -#[derive(Debug)] +#[derive(Clone, Debug)] pub struct CheckState { diagnostic_collection: HashMap>, suggested_fix_collection: HashMap>, diff --git a/crates/ra_lsp_server/src/main_loop.rs b/crates/ra_lsp_server/src/main_loop.rs index 7822be2e2..746a8fbe9 100644 --- a/crates/ra_lsp_server/src/main_loop.rs +++ b/crates/ra_lsp_server/src/main_loop.rs @@ -586,12 +586,14 @@ fn on_notification( fn on_check_task( task: CheckTask, - world_state: &WorldState, + world_state: &mut WorldState, task_sender: &Sender, ) -> Result<()> { match task { CheckTask::ClearDiagnostics => { - let cleared_files = world_state.check_watcher.state.write().clear(); + let state = Arc::get_mut(&mut world_state.check_watcher.state) + .expect("couldn't get check watcher state as mutable"); + let cleared_files = state.clear(); // Send updated diagnostics for each cleared file for url in cleared_files { @@ -600,11 +602,9 @@ fn on_check_task( } CheckTask::AddDiagnostic(url, diagnostic) => { - world_state - .check_watcher - .state - .write() - .add_diagnostic_with_fixes(url.clone(), diagnostic); + let state = Arc::get_mut(&mut world_state.check_watcher.state) + .expect("couldn't get check watcher state as mutable"); + state.add_diagnostic_with_fixes(url.clone(), diagnostic); // We manually send a diagnostic update when the watcher asks // us to, to avoid the issue of having to change the file to diff --git a/crates/ra_lsp_server/src/main_loop/handlers.rs b/crates/ra_lsp_server/src/main_loop/handlers.rs index 8e43f0575..666f2ee29 100644 --- a/crates/ra_lsp_server/src/main_loop/handlers.rs +++ b/crates/ra_lsp_server/src/main_loop/handlers.rs @@ -674,8 +674,7 @@ pub fn handle_code_action( res.push(action.into()); } - for fix in world.check_watcher.read().fixes_for(¶ms.text_document.uri).into_iter().flatten() - { + for fix in world.check_watcher.fixes_for(¶ms.text_document.uri).into_iter().flatten() { let fix_range = fix.location.range.conv_with(&line_index); if fix_range.intersection(&range).is_none() { continue; @@ -895,7 +894,7 @@ pub fn publish_diagnostics( tags: None, }) .collect(); - if let Some(check_diags) = world.check_watcher.read().diagnostics_for(&uri) { + if let Some(check_diags) = world.check_watcher.diagnostics_for(&uri) { diagnostics.extend(check_diags.iter().cloned()); } Ok(req::PublishDiagnosticsParams { uri, diagnostics, version: None }) diff --git a/crates/ra_lsp_server/src/world.rs b/crates/ra_lsp_server/src/world.rs index e7a0acfc7..3059ef9ec 100644 --- a/crates/ra_lsp_server/src/world.rs +++ b/crates/ra_lsp_server/src/world.rs @@ -63,7 +63,7 @@ pub struct WorldSnapshot { pub workspaces: Arc>, pub analysis: Analysis, pub latest_requests: Arc>, - pub check_watcher: Arc>, + pub check_watcher: CheckState, vfs: Arc>, } @@ -220,7 +220,7 @@ impl WorldState { analysis: self.analysis_host.analysis(), vfs: Arc::clone(&self.vfs), latest_requests: Arc::clone(&self.latest_requests), - check_watcher: self.check_watcher.state.clone(), + check_watcher: (*self.check_watcher.state).clone(), } } -- cgit v1.2.3