diff options
author | Emil Lauridsen <[email protected]> | 2020-01-31 18:23:25 +0000 |
---|---|---|
committer | Emil Lauridsen <[email protected]> | 2020-02-03 10:34:24 +0000 |
commit | 790788d5f4013d8d92f110bc12a581d18cf4b6ae (patch) | |
tree | 311e11529c7546b7a09486d5c161039d8bd8f975 /crates/ra_lsp_server/src/main_loop.rs | |
parent | 52456c44901c8c38c8bcb742ebe305484af8f36f (diff) |
Rework how we send diagnostics to client.
The previous way of sending from the thread pool suffered from stale
diagnostics due to being canceled before we could clear the old ones.
The key change is moving to sending diagnostics from the main loop
thread, but doing all the hard work in the thread pool. This should
provide the best of both worlds, with little to no of the downsides.
This should hopefully fix a lot of issues, but we'll need testing in
each individual issue to be sure.
Diffstat (limited to 'crates/ra_lsp_server/src/main_loop.rs')
-rw-r--r-- | crates/ra_lsp_server/src/main_loop.rs | 76 |
1 files changed, 40 insertions, 36 deletions
diff --git a/crates/ra_lsp_server/src/main_loop.rs b/crates/ra_lsp_server/src/main_loop.rs index 508fe08c0..12961ba37 100644 --- a/crates/ra_lsp_server/src/main_loop.rs +++ b/crates/ra_lsp_server/src/main_loop.rs | |||
@@ -17,16 +17,17 @@ use std::{ | |||
17 | use crossbeam_channel::{select, unbounded, RecvError, Sender}; | 17 | use crossbeam_channel::{select, unbounded, RecvError, Sender}; |
18 | use lsp_server::{Connection, ErrorCode, Message, Notification, Request, RequestId, Response}; | 18 | use lsp_server::{Connection, ErrorCode, Message, Notification, Request, RequestId, Response}; |
19 | use lsp_types::{ClientCapabilities, NumberOrString}; | 19 | use lsp_types::{ClientCapabilities, NumberOrString}; |
20 | use ra_cargo_watch::{CheckOptions, CheckTask}; | 20 | use ra_cargo_watch::{url_from_path_with_drive_lowercasing, CheckOptions, CheckTask}; |
21 | use ra_ide::{Canceled, FeatureFlags, FileId, LibraryData, SourceRootId}; | 21 | use ra_ide::{Canceled, FeatureFlags, FileId, LibraryData, SourceRootId}; |
22 | use ra_prof::profile; | 22 | use ra_prof::profile; |
23 | use ra_vfs::{VfsTask, Watch}; | 23 | use ra_vfs::{VfsFile, VfsTask, Watch}; |
24 | use relative_path::RelativePathBuf; | 24 | use relative_path::RelativePathBuf; |
25 | use rustc_hash::FxHashSet; | 25 | use rustc_hash::FxHashSet; |
26 | use serde::{de::DeserializeOwned, Serialize}; | 26 | use serde::{de::DeserializeOwned, Serialize}; |
27 | use threadpool::ThreadPool; | 27 | use threadpool::ThreadPool; |
28 | 28 | ||
29 | use crate::{ | 29 | use crate::{ |
30 | diagnostics::DiagnosticTask, | ||
30 | main_loop::{ | 31 | main_loop::{ |
31 | pending_requests::{PendingRequest, PendingRequests}, | 32 | pending_requests::{PendingRequest, PendingRequests}, |
32 | subscriptions::Subscriptions, | 33 | subscriptions::Subscriptions, |
@@ -254,6 +255,7 @@ pub fn main_loop( | |||
254 | enum Task { | 255 | enum Task { |
255 | Respond(Response), | 256 | Respond(Response), |
256 | Notify(Notification), | 257 | Notify(Notification), |
258 | Diagnostic(DiagnosticTask), | ||
257 | } | 259 | } |
258 | 260 | ||
259 | enum Event { | 261 | enum Event { |
@@ -359,7 +361,7 @@ fn loop_turn( | |||
359 | world_state.maybe_collect_garbage(); | 361 | world_state.maybe_collect_garbage(); |
360 | loop_state.in_flight_libraries -= 1; | 362 | loop_state.in_flight_libraries -= 1; |
361 | } | 363 | } |
362 | Event::CheckWatcher(task) => on_check_task(pool, task, world_state, task_sender)?, | 364 | Event::CheckWatcher(task) => on_check_task(task, world_state, task_sender)?, |
363 | Event::Msg(msg) => match msg { | 365 | Event::Msg(msg) => match msg { |
364 | Message::Request(req) => on_request( | 366 | Message::Request(req) => on_request( |
365 | world_state, | 367 | world_state, |
@@ -464,6 +466,7 @@ fn on_task( | |||
464 | Task::Notify(n) => { | 466 | Task::Notify(n) => { |
465 | msg_sender.send(n.into()).unwrap(); | 467 | msg_sender.send(n.into()).unwrap(); |
466 | } | 468 | } |
469 | Task::Diagnostic(task) => on_diagnostic_task(task, msg_sender, state), | ||
467 | } | 470 | } |
468 | } | 471 | } |
469 | 472 | ||
@@ -621,23 +624,26 @@ fn on_notification( | |||
621 | } | 624 | } |
622 | 625 | ||
623 | fn on_check_task( | 626 | fn on_check_task( |
624 | pool: &ThreadPool, | ||
625 | task: CheckTask, | 627 | task: CheckTask, |
626 | world_state: &mut WorldState, | 628 | world_state: &mut WorldState, |
627 | task_sender: &Sender<Task>, | 629 | task_sender: &Sender<Task>, |
628 | ) -> Result<()> { | 630 | ) -> Result<()> { |
629 | let urls = match task { | 631 | match task { |
630 | CheckTask::ClearDiagnostics => { | 632 | CheckTask::ClearDiagnostics => { |
631 | let state = Arc::get_mut(&mut world_state.check_watcher.state) | 633 | task_sender.send(Task::Diagnostic(DiagnosticTask::ClearCheck))?; |
632 | .expect("couldn't get check watcher state as mutable"); | ||
633 | state.clear() | ||
634 | } | 634 | } |
635 | 635 | ||
636 | CheckTask::AddDiagnostic(url, diagnostic) => { | 636 | CheckTask::AddDiagnostic { url, diagnostic, fixes } => { |
637 | let state = Arc::get_mut(&mut world_state.check_watcher.state) | 637 | let path = url.to_file_path().map_err(|()| format!("invalid uri: {}", url))?; |
638 | .expect("couldn't get check watcher state as mutable"); | 638 | let file_id = world_state |
639 | state.add_diagnostic_with_fixes(url.clone(), diagnostic); | 639 | .vfs |
640 | vec![url] | 640 | .read() |
641 | .path2file(&path) | ||
642 | .map(|it| FileId(it.0)) | ||
643 | .ok_or_else(|| format!("unknown file: {}", path.to_string_lossy()))?; | ||
644 | |||
645 | task_sender | ||
646 | .send(Task::Diagnostic(DiagnosticTask::AddCheck(file_id, diagnostic, fixes)))?; | ||
641 | } | 647 | } |
642 | 648 | ||
643 | CheckTask::Status(progress) => { | 649 | CheckTask::Status(progress) => { |
@@ -647,31 +653,30 @@ fn on_check_task( | |||
647 | }; | 653 | }; |
648 | let not = notification_new::<req::Progress>(params); | 654 | let not = notification_new::<req::Progress>(params); |
649 | task_sender.send(Task::Notify(not)).unwrap(); | 655 | task_sender.send(Task::Notify(not)).unwrap(); |
650 | Vec::new() | ||
651 | } | 656 | } |
652 | }; | 657 | }; |
653 | 658 | ||
654 | let subscriptions = urls | 659 | Ok(()) |
655 | .into_iter() | 660 | } |
656 | .map(|url| { | ||
657 | let path = url.to_file_path().map_err(|()| format!("invalid uri: {}", url))?; | ||
658 | Ok(world_state.vfs.read().path2file(&path).map(|it| FileId(it.0))) | ||
659 | }) | ||
660 | .filter_map(|res| res.transpose()) | ||
661 | .collect::<Result<Vec<_>>>()?; | ||
662 | 661 | ||
663 | // We manually send a diagnostic update when the watcher asks | 662 | fn on_diagnostic_task(task: DiagnosticTask, msg_sender: &Sender<Message>, state: &mut WorldState) { |
664 | // us to, to avoid the issue of having to change the file to | 663 | let subscriptions = state.diagnostics.handle_task(task); |
665 | // receive updated diagnostics. | ||
666 | update_file_notifications_on_threadpool( | ||
667 | pool, | ||
668 | world_state.snapshot(), | ||
669 | false, | ||
670 | task_sender.clone(), | ||
671 | subscriptions, | ||
672 | ); | ||
673 | 664 | ||
674 | Ok(()) | 665 | for file_id in subscriptions { |
666 | let path = state.vfs.read().file2path(VfsFile(file_id.0)); | ||
667 | let uri = match url_from_path_with_drive_lowercasing(&path) { | ||
668 | Ok(uri) => uri, | ||
669 | Err(err) => { | ||
670 | log::error!("Couldn't convert path to url ({}): {:?}", err, path.to_string_lossy()); | ||
671 | continue; | ||
672 | } | ||
673 | }; | ||
674 | |||
675 | let diagnostics = state.diagnostics.diagnostics_for(file_id).cloned().collect(); | ||
676 | let params = req::PublishDiagnosticsParams { uri, diagnostics, version: None }; | ||
677 | let not = notification_new::<req::PublishDiagnostics>(params); | ||
678 | msg_sender.send(not.into()).unwrap(); | ||
679 | } | ||
675 | } | 680 | } |
676 | 681 | ||
677 | struct PoolDispatcher<'a> { | 682 | struct PoolDispatcher<'a> { |
@@ -819,9 +824,8 @@ fn update_file_notifications_on_threadpool( | |||
819 | log::error!("failed to compute diagnostics: {:?}", e); | 824 | log::error!("failed to compute diagnostics: {:?}", e); |
820 | } | 825 | } |
821 | } | 826 | } |
822 | Ok(params) => { | 827 | Ok(task) => { |
823 | let not = notification_new::<req::PublishDiagnostics>(params); | 828 | task_sender.send(Task::Diagnostic(task)).unwrap(); |
824 | task_sender.send(Task::Notify(not)).unwrap(); | ||
825 | } | 829 | } |
826 | } | 830 | } |
827 | } | 831 | } |