Auto merge of #15028 - Veykril:rustfmt-thread, r=Veykril

internal: Give rustfmt jobs a separate thread

Some light testing suggests that this fixes the waiting on formatting popup in vscode when the project is still building (which is usually the way for me to encounter it, as r-a is either waiting or getting little resources causing the tasks to block formatting)

4 files changed, 54 insertions, 29 deletions

diff --git a/crates/rust-analyzer/src/dispatch.rs b/crates/rust-analyzer/src/dispatch.rs
index ebe77b8dfe7..8a0ad88b654 100644
--- a/crates/rust-analyzer/src/dispatch.rs
+++ b/crates/rust-analyzer/src/dispatch.rs
@@ -135,7 +135,7 @@ impl<'a> RequestDispatcher<'a> {
         R::Params: DeserializeOwned + panic::UnwindSafe + Send + fmt::Debug,
         R::Result: Serialize,
     {
-        self.on_with_thread_intent::<R>(ThreadIntent::Worker, f)
+        self.on_with_thread_intent::<true, R>(ThreadIntent::Worker, f)
     }
 
     /// Dispatches a latency-sensitive request onto the thread pool.
@@ -148,7 +148,22 @@ impl<'a> RequestDispatcher<'a> {
         R::Params: DeserializeOwned + panic::UnwindSafe + Send + fmt::Debug,
         R::Result: Serialize,
     {
-        self.on_with_thread_intent::<R>(ThreadIntent::LatencySensitive, f)
+        self.on_with_thread_intent::<true, R>(ThreadIntent::LatencySensitive, f)
+    }
+
+    /// Formatting requests should never block on waiting a for task thread to open up, editors will wait
+    /// on the response and a late formatting update might mess with the document and user.
+    /// We can't run this on the main thread though as we invoke rustfmt which may take arbitrary time to complete!
+    pub(crate) fn on_fmt_thread<R>(
+        &mut self,
+        f: fn(GlobalStateSnapshot, R::Params) -> Result<R::Result>,
+    ) -> &mut Self
+    where
+        R: lsp_types::request::Request + 'static,
+        R::Params: DeserializeOwned + panic::UnwindSafe + Send + fmt::Debug,
+        R::Result: Serialize,
+    {
+        self.on_with_thread_intent::<false, R>(ThreadIntent::LatencySensitive, f)
     }
 
     pub(crate) fn finish(&mut self) {
@@ -163,7 +178,7 @@ impl<'a> RequestDispatcher<'a> {
         }
     }
 
-    fn on_with_thread_intent<R>(
+    fn on_with_thread_intent<const MAIN_POOL: bool, R>(
         &mut self,
         intent: ThreadIntent,
         f: fn(GlobalStateSnapshot, R::Params) -> Result<R::Result>,
@@ -178,17 +193,20 @@ impl<'a> RequestDispatcher<'a> {
             None => return self,
         };
 
-        self.global_state.task_pool.handle.spawn(intent, {
-            let world = self.global_state.snapshot();
-            move || {
-                let result = panic::catch_unwind(move || {
-                    let _pctx = stdx::panic_context::enter(panic_context);
-                    f(world, params)
-                });
-                match thread_result_to_response::<R>(req.id.clone(), result) {
-                    Ok(response) => Task::Response(response),
-                    Err(_) => Task::Retry(req),
-                }
+        let world = self.global_state.snapshot();
+        if MAIN_POOL {
+            &mut self.global_state.task_pool.handle
+        } else {
+            &mut self.global_state.fmt_pool.handle
+        }
+        .spawn(intent, move || {
+            let result = panic::catch_unwind(move || {
+                let _pctx = stdx::panic_context::enter(panic_context);
+                f(world, params)
+            });
+            match thread_result_to_response::<R>(req.id.clone(), result) {
+                Ok(response) => Task::Response(response),
+                Err(_) => Task::Retry(req),
             }
         });
 
diff --git a/crates/rust-analyzer/src/global_state.rs b/crates/rust-analyzer/src/global_state.rs
index 9f4dc444020..9f2fda02535 100644
--- a/crates/rust-analyzer/src/global_state.rs
+++ b/crates/rust-analyzer/src/global_state.rs
@@ -54,6 +54,7 @@ pub(crate) struct GlobalState {
     req_queue: ReqQueue,
 
     pub(crate) task_pool: Handle<TaskPool<Task>, Receiver<Task>>,
+    pub(crate) fmt_pool: Handle<TaskPool<Task>, Receiver<Task>>,
 
     pub(crate) config: Arc<Config>,
     pub(crate) config_errors: Option<ConfigError>,
@@ -151,6 +152,11 @@ impl GlobalState {
             let handle = TaskPool::new_with_threads(sender, config.main_loop_num_threads());
             Handle { handle, receiver }
         };
+        let fmt_pool = {
+            let (sender, receiver) = unbounded();
+            let handle = TaskPool::new_with_threads(sender, 1);
+            Handle { handle, receiver }
+        };
 
         let mut analysis_host = AnalysisHost::new(config.lru_parse_query_capacity());
         if let Some(capacities) = config.lru_query_capacities() {
@@ -161,6 +167,7 @@ impl GlobalState {
             sender,
             req_queue: ReqQueue::default(),
             task_pool,
+            fmt_pool,
             loader,
             config: Arc::new(config.clone()),
             analysis_host,
diff --git a/crates/rust-analyzer/src/handlers/request.rs b/crates/rust-analyzer/src/handlers/request.rs
index 52a85054ea3..fc1076b65b0 100644
--- a/crates/rust-analyzer/src/handlers/request.rs
+++ b/crates/rust-analyzer/src/handlers/request.rs
@@ -18,12 +18,11 @@ use lsp_server::ErrorCode;
 use lsp_types::{
     CallHierarchyIncomingCall, CallHierarchyIncomingCallsParams, CallHierarchyItem,
     CallHierarchyOutgoingCall, CallHierarchyOutgoingCallsParams, CallHierarchyPrepareParams,
-    CodeLens, CompletionItem, DocumentFormattingParams, FoldingRange, FoldingRangeParams,
-    HoverContents, InlayHint, InlayHintParams, Location, LocationLink, Position,
-    PrepareRenameResponse, Range, RenameParams, SemanticTokensDeltaParams,
-    SemanticTokensFullDeltaResult, SemanticTokensParams, SemanticTokensRangeParams,
-    SemanticTokensRangeResult, SemanticTokensResult, SymbolInformation, SymbolTag,
-    TextDocumentIdentifier, Url, WorkspaceEdit,
+    CodeLens, CompletionItem, FoldingRange, FoldingRangeParams, HoverContents, InlayHint,
+    InlayHintParams, Location, LocationLink, Position, PrepareRenameResponse, Range, RenameParams,
+    SemanticTokensDeltaParams, SemanticTokensFullDeltaResult, SemanticTokensParams,
+    SemanticTokensRangeParams, SemanticTokensRangeResult, SemanticTokensResult, SymbolInformation,
+    SymbolTag, TextDocumentIdentifier, Url, WorkspaceEdit,
 };
 use project_model::{ManifestPath, ProjectWorkspace, TargetKind};
 use serde_json::json;
@@ -1077,7 +1076,7 @@ pub(crate) fn handle_references(
 
 pub(crate) fn handle_formatting(
     snap: GlobalStateSnapshot,
-    params: DocumentFormattingParams,
+    params: lsp_types::DocumentFormattingParams,
 ) -> Result<Option<Vec<lsp_types::TextEdit>>> {
     let _p = profile::span("handle_formatting");
 
diff --git a/crates/rust-analyzer/src/main_loop.rs b/crates/rust-analyzer/src/main_loop.rs
index 12bc638929d..eb5d7f495f7 100644
--- a/crates/rust-analyzer/src/main_loop.rs
+++ b/crates/rust-analyzer/src/main_loop.rs
@@ -177,6 +177,9 @@ impl GlobalState {
             recv(self.task_pool.receiver) -> task =>
                 Some(Event::Task(task.unwrap())),
 
+            recv(self.fmt_pool.receiver) -> task =>
+                Some(Event::Task(task.unwrap())),
+
             recv(self.loader.receiver) -> task =>
                 Some(Event::Vfs(task.unwrap())),
 
@@ -678,6 +681,12 @@ impl GlobalState {
             .on_sync::<lsp_types::request::SelectionRangeRequest>(handlers::handle_selection_range)
             .on_sync::<lsp_ext::MatchingBrace>(handlers::handle_matching_brace)
             .on_sync::<lsp_ext::OnTypeFormatting>(handlers::handle_on_type_formatting)
+            // Formatting should be done immediately as the editor might wait on it, but we can't
+            // put it on the main thread as we do not want the main thread to block on rustfmt.
+            // So we have an extra thread just for formatting requests to make sure it gets handled
+            // as fast as possible.
+            .on_fmt_thread::<lsp_types::request::Formatting>(handlers::handle_formatting)
+            .on_fmt_thread::<lsp_types::request::RangeFormatting>(handlers::handle_range_formatting)
             // We can’t run latency-sensitive request handlers which do semantic
             // analysis on the main thread because that would block other
             // requests. Instead, we run these request handlers on higher priority
@@ -695,14 +704,6 @@ impl GlobalState {
             .on_latency_sensitive::<lsp_types::request::SemanticTokensRangeRequest>(
                 handlers::handle_semantic_tokens_range,
             )
-            // Formatting is not caused by the user typing,
-            // but it does qualify as latency-sensitive
-            // because a delay before formatting is applied
-            // can be confusing for the user.
-            .on_latency_sensitive::<lsp_types::request::Formatting>(handlers::handle_formatting)
-            .on_latency_sensitive::<lsp_types::request::RangeFormatting>(
-                handlers::handle_range_formatting,
-            )
             // All other request handlers
             .on::<lsp_ext::FetchDependencyList>(handlers::fetch_dependency_list)
             .on::<lsp_ext::AnalyzerStatus>(handlers::handle_analyzer_status)