3 files changed, 89 insertions, 53 deletions

diff --git a/crates/ra_lsp_server/src/main_loop.rs b/crates/ra_lsp_server/src/main_loop.rs
index 7822be2e2..d850ded37 100644
--- a/crates/ra_lsp_server/src/main_loop.rs
+++ b/crates/ra_lsp_server/src/main_loop.rs
@@ -5,11 +5,18 @@ mod handlers;
 mod subscriptions;
 pub(crate) mod pending_requests;
 
-use std::{error::Error, fmt, panic, path::PathBuf, sync::Arc, time::Instant};
+use std::{
+    env,
+    error::Error,
+    fmt, panic,
+    path::PathBuf,
+    sync::Arc,
+    time::{Duration, Instant},
+};
 
 use crossbeam_channel::{select, unbounded, RecvError, Sender};
 use lsp_server::{Connection, ErrorCode, Message, Notification, Request, RequestId, Response};
-use lsp_types::{ClientCapabilities, NumberOrString, Url};
+use lsp_types::{ClientCapabilities, NumberOrString};
 use ra_cargo_watch::{CheckOptions, CheckTask};
 use ra_ide::{Canceled, FeatureFlags, FileId, LibraryData, SourceRootId};
 use ra_prof::profile;
@@ -29,9 +36,6 @@ use crate::{
     Result, ServerConfig,
 };
 
-const THREADPOOL_SIZE: usize = 8;
-const MAX_IN_FLIGHT_LIBS: usize = THREADPOOL_SIZE - 3;
-
 #[derive(Debug)]
 pub struct LspError {
     pub code: i32,
@@ -60,6 +64,25 @@ pub fn main_loop(
 ) -> Result<()> {
     log::info!("server_config: {:#?}", config);
 
+    // Windows scheduler implements priority boosts: if thread waits for an
+    // event (like a condvar), and event fires, priority of the thread is
+    // temporary bumped. This optimization backfires in our case: each time the
+    // `main_loop` schedules a task to run on a threadpool, the worker threads
+    // gets a higher priority, and (on a machine with fewer cores) displaces the
+    // main loop! We work-around this by marking the main loop as a
+    // higher-priority thread.
+    //
+    // https://docs.microsoft.com/en-us/windows/win32/procthread/scheduling-priorities
+    // https://docs.microsoft.com/en-us/windows/win32/procthread/priority-boosts
+    // https://github.com/rust-analyzer/rust-analyzer/issues/2835
+    #[cfg(windows)]
+    unsafe {
+        use winapi::um::processthreadsapi::*;
+        let thread = GetCurrentThread();
+        let thread_priority_above_normal = 1;
+        SetThreadPriority(thread, thread_priority_above_normal);
+    }
+
     let mut loop_state = LoopState::default();
     let mut world_state = {
         let feature_flags = {
@@ -168,7 +191,7 @@ pub fn main_loop(
         )
     };
 
-    let pool = ThreadPool::new(THREADPOOL_SIZE);
+    let pool = ThreadPool::default();
     let (task_sender, task_receiver) = unbounded::<Task>();
     let (libdata_sender, libdata_receiver) = unbounded::<LibraryData>();
 
@@ -210,7 +233,7 @@ pub fn main_loop(
             )?;
         }
     }
-
+    world_state.analysis_host.request_cancellation();
     log::info!("waiting for tasks to finish...");
     task_receiver.into_iter().for_each(|task| {
         on_task(task, &connection.sender, &mut loop_state.pending_requests, &mut world_state)
@@ -336,7 +359,7 @@ fn loop_turn(
             world_state.maybe_collect_garbage();
             loop_state.in_flight_libraries -= 1;
         }
-        Event::CheckWatcher(task) => on_check_task(task, world_state, task_sender)?,
+        Event::CheckWatcher(task) => on_check_task(pool, task, world_state, task_sender)?,
         Event::Msg(msg) => match msg {
             Message::Request(req) => on_request(
                 world_state,
@@ -371,7 +394,8 @@ fn loop_turn(
         loop_state.pending_libraries.extend(changes);
     }
 
-    while loop_state.in_flight_libraries < MAX_IN_FLIGHT_LIBS
+    let max_in_flight_libs = pool.max_count().saturating_sub(2).max(1);
+    while loop_state.in_flight_libraries < max_in_flight_libs
         && !loop_state.pending_libraries.is_empty()
     {
         let (root, files) = loop_state.pending_libraries.pop().unwrap();
@@ -408,6 +432,19 @@ fn loop_turn(
             loop_state.subscriptions.subscriptions(),
         )
     }
+
+    let loop_duration = loop_start.elapsed();
+    if loop_duration > Duration::from_millis(100) {
+        log::error!("overly long loop turn: {:?}", loop_duration);
+        if env::var("RA_PROFILE").is_ok() {
+            show_message(
+                req::MessageType::Error,
+                format!("overly long loop turn: {:?}", loop_duration),
+                &connection.sender,
+            );
+        }
+    }
+
     Ok(())
 }
 
@@ -435,7 +472,7 @@ fn on_request(
     world: &mut WorldState,
     pending_requests: &mut PendingRequests,
     pool: &ThreadPool,
-    sender: &Sender<Task>,
+    task_sender: &Sender<Task>,
     msg_sender: &Sender<Message>,
     request_received: Instant,
     req: Request,
@@ -444,7 +481,7 @@ fn on_request(
         req: Some(req),
         pool,
         world,
-        sender,
+        task_sender,
         msg_sender,
         pending_requests,
         request_received,
@@ -585,31 +622,23 @@ fn on_notification(
 }
 
 fn on_check_task(
+    pool: &ThreadPool,
     task: CheckTask,
-    world_state: &WorldState,
+    world_state: &mut WorldState,
     task_sender: &Sender<Task>,
 ) -> Result<()> {
-    match task {
+    let urls = match task {
         CheckTask::ClearDiagnostics => {
-            let cleared_files = world_state.check_watcher.state.write().clear();
-
-            // Send updated diagnostics for each cleared file
-            for url in cleared_files {
-                publish_diagnostics_for_url(&url, world_state, task_sender)?;
-            }
+            let state = Arc::get_mut(&mut world_state.check_watcher.state)
+                .expect("couldn't get check watcher state as mutable");
+            state.clear()
         }
 
         CheckTask::AddDiagnostic(url, diagnostic) => {
-            world_state
-                .check_watcher
-                .state
-                .write()
-                .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
-            // receive updated diagnostics.
-            publish_diagnostics_for_url(&url, world_state, task_sender)?;
+            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);
+            vec![url]
         }
 
         CheckTask::Status(progress) => {
@@ -619,22 +648,30 @@ fn on_check_task(
             };
             let not = notification_new::<req::Progress>(params);
             task_sender.send(Task::Notify(not)).unwrap();
+            Vec::new()
         }
-    }
-    Ok(())
-}
+    };
+
+    let subscriptions = urls
+        .into_iter()
+        .map(|url| {
+            let path = url.to_file_path().map_err(|()| format!("invalid uri: {}", url))?;
+            Ok(world_state.vfs.read().path2file(&path).map(|it| FileId(it.0)))
+        })
+        .filter_map(|res| res.transpose())
+        .collect::<Result<Vec<_>>>()?;
+
+    // We manually send a diagnostic update when the watcher asks
+    // us to, to avoid the issue of having to change the file to
+    // receive updated diagnostics.
+    update_file_notifications_on_threadpool(
+        pool,
+        world_state.snapshot(),
+        false,
+        task_sender.clone(),
+        subscriptions,
+    );
 
-fn publish_diagnostics_for_url(
-    url: &Url,
-    world_state: &WorldState,
-    task_sender: &Sender<Task>,
-) -> Result<()> {
-    let path = url.to_file_path().map_err(|()| format!("invalid uri: {}", url))?;
-    if let Some(file_id) = world_state.vfs.read().path2file(&path) {
-        let params = handlers::publish_diagnostics(&world_state.snapshot(), FileId(file_id.0))?;
-        let not = notification_new::<req::PublishDiagnostics>(params);
-        task_sender.send(Task::Notify(not)).unwrap();
-    }
     Ok(())
 }
 
@@ -644,7 +681,7 @@ struct PoolDispatcher<'a> {
     world: &'a mut WorldState,
     pending_requests: &'a mut PendingRequests,
     msg_sender: &'a Sender<Message>,
-    sender: &'a Sender<Task>,
+    task_sender: &'a Sender<Task>,
     request_received: Instant,
 }
 
@@ -691,7 +728,7 @@ impl<'a> PoolDispatcher<'a> {
 
         self.pool.execute({
             let world = self.world.snapshot();
-            let sender = self.sender.clone();
+            let sender = self.task_sender.clone();
             move || {
                 let result = f(world, params);
                 let task = result_to_task::<R>(id, result);
@@ -769,7 +806,7 @@ fn update_file_notifications_on_threadpool(
     pool: &ThreadPool,
     world: WorldSnapshot,
     publish_decorations: bool,
-    sender: Sender<Task>,
+    task_sender: Sender<Task>,
     subscriptions: Vec<FileId>,
 ) {
     log::trace!("updating notifications for {:?}", subscriptions);
@@ -785,7 +822,7 @@ fn update_file_notifications_on_threadpool(
                     }
                     Ok(params) => {
                         let not = notification_new::<req::PublishDiagnostics>(params);
-                        sender.send(Task::Notify(not)).unwrap();
+                        task_sender.send(Task::Notify(not)).unwrap();
                     }
                 }
             }
@@ -798,7 +835,7 @@ fn update_file_notifications_on_threadpool(
                     }
                     Ok(params) => {
                         let not = notification_new::<req::PublishDecorations>(params);
-                        sender.send(Task::Notify(not)).unwrap();
+                        task_sender.send(Task::Notify(not)).unwrap();
                     }
                 }
             }
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(&params.text_document.uri).into_iter().flatten()
-    {
+    for fix in world.check_watcher.fixes_for(&params.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<Vec<ProjectWorkspace>>,
     pub analysis: Analysis,
     pub latest_requests: Arc<RwLock<LatestRequests>>,
-    pub check_watcher: Arc<RwLock<CheckState>>,
+    pub check_watcher: CheckState,
     vfs: Arc<RwLock<Vfs>>,
 }
 
@@ -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(),
         }
     }