Use ignore::WalkParallel for native parallelization instead of rayon (#…

…1412) This potentially fixes a deadlock with `rayon::par_bridge` which relies on a shared mutex. If we recursively rely par_bridge through recursive walking, it's _possible_ that this creates a deadlock situation. Thankfully ignore implements its own parallelized walker which we should be using instead of par_bridge. I could not detect if this deadlock is actually caused by this line of code, but I believe these stalls did start happening more often when we added more parallelization to CLI.
qltysh · Jan 9, 2025 · d3a8ddf · d3a8ddf
1 parent 31e59ba
commit d3a8ddf
Show file tree

Hide file tree

Showing 4 changed files with 26 additions and 14 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/qlty-analysis/Cargo.toml b/qlty-analysis/Cargo.toml
@@ -21,8 +21,9 @@ git2.workspace = true
 glob.workspace = true
 globset.workspace = true
 ignore.workspace = true
-md5.workspace = true
 lazy_static.workspace = true
+md5.workspace = true
+num_cpus.workspace = true
 path-absolutize.workspace = true
 pbjson-types.workspace = true
 qlty-config.workspace = true

diff --git a/qlty-analysis/src/walker.rs b/qlty-analysis/src/walker.rs
@@ -1,7 +1,7 @@
 use ignore::overrides::Override;
 use ignore::overrides::OverrideBuilder;
-pub use ignore::Walk;
 use ignore::WalkBuilder;
+pub use ignore::WalkParallel;
 use std::path::PathBuf;
 
 /// Maximum allowable file size (in bytes) for processing.
@@ -23,7 +23,7 @@ impl WalkerBuilder {
     }
 
     /// Constructs a `Walk` object based on the builder's paths and ignores.
-    pub fn build(&self, paths: &[PathBuf]) -> Walk {
+    pub fn build(&self, paths: &[PathBuf]) -> WalkParallel {
         let mut builder = WalkBuilder::new(&paths[0]);
 
         for path in &paths[1..] {
@@ -34,8 +34,9 @@ impl WalkerBuilder {
             .follow_links(false)
             .max_filesize(Some(MAX_FILE_SIZE as u64))
             .hidden(false)
-            .overrides(self.overrides());
-        builder.build()
+            .overrides(self.overrides())
+            .threads(num_cpus::get());
+        builder.build_parallel()
     }
 
     fn overrides(&self) -> Override {

diff --git a/qlty-analysis/src/workspace_entries/args_source.rs b/qlty-analysis/src/workspace_entries/args_source.rs
@@ -1,8 +1,11 @@
 use crate::{walker::WalkerBuilder, WorkspaceEntry, WorkspaceEntryKind, WorkspaceEntrySource};
 use core::fmt;
+use ignore::WalkState;
 use path_absolutize::Absolutize;
-use rayon::prelude::*;
-use std::{path::PathBuf, sync::Arc};
+use std::{
+    path::PathBuf,
+    sync::{Arc, Mutex},
+};
 
 // qlty-ignore(semgrep/derive-debug): manual Debug impl below
 pub struct ArgsSource {
@@ -27,10 +30,11 @@ impl ArgsSource {
     }
 
     fn build(root: &PathBuf, paths: &Vec<PathBuf>) -> Vec<WorkspaceEntry> {
-        WalkerBuilder::new()
-            .build(paths)
-            .par_bridge()
-            .map(|entry| {
+        let workspace_entries = Arc::new(Mutex::new(vec![]));
+
+        WalkerBuilder::new().build(paths).run(|| {
+            let entries = workspace_entries.clone();
+            Box::new(move |entry| {
                 let entry = entry.unwrap();
                 let path = entry.path();
 
@@ -50,15 +54,20 @@ impl ArgsSource {
                     .to_path_buf();
                 let metadata = entry.metadata().ok().unwrap();
 
-                WorkspaceEntry {
+                entries.lock().unwrap().push(WorkspaceEntry {
                     path: clean_path,
                     content_modified: metadata.modified().ok().unwrap(),
                     contents_size: metadata.len(),
                     kind: workspace_entry_kind,
                     language_name: None,
-                }
+                });
+
+                WalkState::Continue
             })
-            .collect::<Vec<_>>()
+        });
+
+        let guard = workspace_entries.lock().unwrap();
+        guard.to_vec()
     }
 }