Auto merge of rust-lang#17842 - mo8it:crossbeam-channel, r=Veykril

internal: Optimize the usage of channel senders Used `Sender` directly instead of a boxed closure. There is no need to use the boxed closure. This also allows the caller to decide to do something other than `unwrap` (not a fan of it BTW).
lnicola · Aug 12, 2024 · c2d1555 · c2d1555
2 parents 757b0a5 + f7c4716
commit c2d1555
Show file tree

Hide file tree

Showing 13 changed files with 41 additions and 34 deletions.
diff --git a/src/tools/rust-analyzer/Cargo.lock b/src/tools/rust-analyzer/Cargo.lock
@@ -2317,6 +2317,7 @@ checksum = "49874b5167b65d7193b8aba1567f5c7d93d001cafc34600cee003eda787e483f"
 name = "vfs"
 version = "0.0.0"
 dependencies = [
+ "crossbeam-channel",
  "fst",
  "indexmap",
  "nohash-hasher",

diff --git a/src/tools/rust-analyzer/crates/ide-db/Cargo.toml b/src/tools/rust-analyzer/crates/ide-db/Cargo.toml
@@ -14,7 +14,7 @@ doctest = false
 
 [dependencies]
 cov-mark = "2.0.0-pre.1"
-crossbeam-channel = "0.5.5"
+crossbeam-channel.workspace = true
 tracing.workspace = true
 rayon.workspace = true
 fst = { version = "0.4.7", default-features = false }

diff --git a/src/tools/rust-analyzer/crates/load-cargo/src/lib.rs b/src/tools/rust-analyzer/crates/load-cargo/src/lib.rs
@@ -64,8 +64,7 @@ pub fn load_workspace(
     let (sender, receiver) = unbounded();
     let mut vfs = vfs::Vfs::default();
     let mut loader = {
-        let loader =
-            vfs_notify::NotifyHandle::spawn(Box::new(move |msg| sender.send(msg).unwrap()));
+        let loader = vfs_notify::NotifyHandle::spawn(sender);
         Box::new(loader)
     };
 

diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/Cargo.toml b/src/tools/rust-analyzer/crates/rust-analyzer/Cargo.toml
@@ -21,7 +21,7 @@ path = "src/bin/main.rs"
 
 [dependencies]
 anyhow.workspace = true
-crossbeam-channel = "0.5.5"
+crossbeam-channel.workspace = true
 dirs = "5.0.1"
 dissimilar.workspace = true
 itertools.workspace = true
@@ -90,13 +90,13 @@ jemalloc = ["jemallocator", "profile/jemalloc"]
 force-always-assert = ["always-assert/force"]
 sysroot-abi = []
 in-rust-tree = [
-    "sysroot-abi",
-    "syntax/in-rust-tree",
-    "parser/in-rust-tree",
-    "hir/in-rust-tree",
-    "hir-def/in-rust-tree",
-    "hir-ty/in-rust-tree",
-    "load-cargo/in-rust-tree",
+  "sysroot-abi",
+  "syntax/in-rust-tree",
+  "parser/in-rust-tree",
+  "hir/in-rust-tree",
+  "hir-def/in-rust-tree",
+  "hir-ty/in-rust-tree",
+  "load-cargo/in-rust-tree",
 ]
 
 [lints]

diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/flycheck.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/flycheck.rs
@@ -109,7 +109,7 @@ pub(crate) struct FlycheckHandle {
 impl FlycheckHandle {
     pub(crate) fn spawn(
         id: usize,
-        sender: Box<dyn Fn(FlycheckMessage) + Send>,
+        sender: Sender<FlycheckMessage>,
         config: FlycheckConfig,
         sysroot_root: Option<AbsPathBuf>,
         workspace_root: AbsPathBuf,
@@ -199,7 +199,7 @@ enum StateChange {
 struct FlycheckActor {
     /// The workspace id of this flycheck instance.
     id: usize,
-    sender: Box<dyn Fn(FlycheckMessage) + Send>,
+    sender: Sender<FlycheckMessage>,
     config: FlycheckConfig,
     manifest_path: Option<AbsPathBuf>,
     /// Either the workspace root of the workspace we are flychecking,
@@ -235,7 +235,7 @@ pub(crate) const SAVED_FILE_PLACEHOLDER: &str = "$saved_file";
 impl FlycheckActor {
     fn new(
         id: usize,
-        sender: Box<dyn Fn(FlycheckMessage) + Send>,
+        sender: Sender<FlycheckMessage>,
         config: FlycheckConfig,
         sysroot_root: Option<AbsPathBuf>,
         workspace_root: AbsPathBuf,
@@ -478,8 +478,9 @@ impl FlycheckActor {
         Some(cmd)
     }
 
+    #[track_caller]
     fn send(&self, check_task: FlycheckMessage) {
-        (self.sender)(check_task);
+        self.sender.send(check_task).unwrap();
     }
 }
 

diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/global_state.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/global_state.rs
@@ -185,8 +185,7 @@ impl GlobalState {
     pub(crate) fn new(sender: Sender<lsp_server::Message>, config: Config) -> GlobalState {
         let loader = {
             let (sender, receiver) = unbounded::<vfs::loader::Message>();
-            let handle: vfs_notify::NotifyHandle =
-                vfs::loader::Handle::spawn(Box::new(move |msg| sender.send(msg).unwrap()));
+            let handle: vfs_notify::NotifyHandle = vfs::loader::Handle::spawn(sender);
             let handle = Box::new(handle) as Box<dyn vfs::loader::Handle>;
             Handle { handle, receiver }
         };
@@ -564,8 +563,9 @@ impl GlobalState {
         self.req_queue.incoming.is_completed(&request.id)
     }
 
+    #[track_caller]
     fn send(&self, message: lsp_server::Message) {
-        self.sender.send(message).unwrap()
+        self.sender.send(message).unwrap();
     }
 
     pub(crate) fn publish_diagnostics(

diff --git a/src/tools/rust-analyzer/crates/rust-analyzer/src/reload.rs b/src/tools/rust-analyzer/crates/rust-analyzer/src/reload.rs
@@ -758,7 +758,7 @@ impl GlobalState {
         self.flycheck = match invocation_strategy {
             crate::flycheck::InvocationStrategy::Once => vec![FlycheckHandle::spawn(
                 0,
-                Box::new(move |msg| sender.send(msg).unwrap()),
+                sender,
                 config,
                 None,
                 self.config.root_path().clone(),
@@ -793,10 +793,9 @@ impl GlobalState {
                         ))
                     })
                     .map(|(id, (root, manifest_path), sysroot_root)| {
-                        let sender = sender.clone();
                         FlycheckHandle::spawn(
                             id,
-                            Box::new(move |msg| sender.send(msg).unwrap()),
+                            sender.clone(),
                             config.clone(),
                             sysroot_root,
                             root.to_path_buf(),

diff --git a/src/tools/rust-analyzer/crates/stdx/Cargo.toml b/src/tools/rust-analyzer/crates/stdx/Cargo.toml
@@ -17,7 +17,7 @@ backtrace = { version = "0.3.67", optional = true }
 always-assert = { version = "0.2.0", features = ["tracing"] }
 jod-thread = "0.1.2"
 libc.workspace = true
-crossbeam-channel = "0.5.5"
+crossbeam-channel.workspace = true
 itertools.workspace = true
 # Think twice before adding anything here
 

diff --git a/src/tools/rust-analyzer/crates/vfs-notify/Cargo.toml b/src/tools/rust-analyzer/crates/vfs-notify/Cargo.toml
@@ -15,7 +15,7 @@ doctest = false
 [dependencies]
 tracing.workspace = true
 walkdir = "2.3.2"
-crossbeam-channel = "0.5.5"
+crossbeam-channel.workspace = true
 notify = "6.1.1"
 rayon = "1.10.0"
 

diff --git a/src/tools/rust-analyzer/crates/vfs-notify/src/lib.rs b/src/tools/rust-analyzer/crates/vfs-notify/src/lib.rs
@@ -119,8 +119,7 @@ impl NotifyActor {
                         self.watched_dir_entries.clear();
                         self.watched_file_entries.clear();
 
-                        let send = |msg| (self.sender)(msg);
-                        send(loader::Message::Progress {
+                        self.send(loader::Message::Progress {
                             n_total,
                             n_done: LoadingProgress::Started,
                             config_version,
@@ -130,7 +129,8 @@ impl NotifyActor {
                         let (entry_tx, entry_rx) = unbounded();
                         let (watch_tx, watch_rx) = unbounded();
                         let processed = AtomicUsize::new(0);
-                        config.load.into_par_iter().enumerate().for_each(move |(i, entry)| {
+
+                        config.load.into_par_iter().enumerate().for_each(|(i, entry)| {
                             let do_watch = config.watch.contains(&i);
                             if do_watch {
                                 _ = entry_tx.send(entry.clone());
@@ -140,18 +140,18 @@ impl NotifyActor {
                                 entry,
                                 do_watch,
                                 |file| {
-                                    send(loader::Message::Progress {
+                                    self.send(loader::Message::Progress {
                                         n_total,
                                         n_done: LoadingProgress::Progress(
                                             processed.load(std::sync::atomic::Ordering::Relaxed),
                                         ),
                                         dir: Some(file),
                                         config_version,
-                                    })
+                                    });
                                 },
                             );
-                            send(loader::Message::Loaded { files });
-                            send(loader::Message::Progress {
+                            self.send(loader::Message::Loaded { files });
+                            self.send(loader::Message::Progress {
                                 n_total,
                                 n_done: LoadingProgress::Progress(
                                     processed.fetch_add(1, std::sync::atomic::Ordering::AcqRel) + 1,
@@ -160,9 +160,13 @@ impl NotifyActor {
                                 dir: None,
                             });
                         });
+
+                        drop(watch_tx);
                         for path in watch_rx {
                             self.watch(&path);
                         }
+
+                        drop(entry_tx);
                         for entry in entry_rx {
                             match entry {
                                 loader::Entry::Files(files) => {
@@ -173,6 +177,7 @@ impl NotifyActor {
                                 }
                             }
                         }
+
                         self.send(loader::Message::Progress {
                             n_total,
                             n_done: LoadingProgress::Finished,
@@ -316,8 +321,9 @@ impl NotifyActor {
         }
     }
 
+    #[track_caller]
     fn send(&self, msg: loader::Message) {
-        (self.sender)(msg);
+        self.sender.send(msg).unwrap();
     }
 }
 

diff --git a/src/tools/rust-analyzer/crates/vfs/Cargo.toml b/src/tools/rust-analyzer/crates/vfs/Cargo.toml
@@ -18,6 +18,7 @@ tracing.workspace = true
 fst = "0.4.7"
 indexmap.workspace = true
 nohash-hasher.workspace = true
+crossbeam-channel.workspace = true
 
 paths.workspace = true
 stdx.workspace = true

diff --git a/src/tools/rust-analyzer/crates/vfs/src/loader.rs b/src/tools/rust-analyzer/crates/vfs/src/loader.rs
@@ -72,7 +72,7 @@ pub enum Message {
 }
 
 /// Type that will receive [`Messages`](Message) from a [`Handle`].
-pub type Sender = Box<dyn Fn(Message) + Send + Sync>;
+pub type Sender = crossbeam_channel::Sender<Message>;
 
 /// Interface for reading and watching files.
 pub trait Handle: fmt::Debug {

diff --git a/src/tools/rust-analyzer/lib/lsp-server/Cargo.toml b/src/tools/rust-analyzer/lib/lsp-server/Cargo.toml
@@ -10,11 +10,11 @@ edition = "2021"
 log = "0.4.17"
 serde_json = "1.0.108"
 serde = { version = "1.0.192", features = ["derive"] }
-crossbeam-channel = "0.5.8"
+crossbeam-channel.workspace = true
 
 [dev-dependencies]
 lsp-types = "=0.95"
 ctrlc = "3.4.1"
 
 [lints]
-workspace = true
+workspace = true