clean: Optimize clean with multiple -p specifiers

This commit optimizes implementation of `cargo clean -p` by reducing the amount of directory walks that take place.
We now batch calls to `rm_rf_prefix_list`, thus potentially avoiding multiple walks over a single subdirectory. In practice this helps us significantly reduce the runtime for clearing large workspaces (as implemented in #16263); for Zed, `cargo clean --workspace` went down from 73 seconds to 3 seconds.
We have 216 workspace members.

Co-authored-by: dino <dinojoaocosta@gmail.com>

Author: osiewicz

src/cargo/core/compiler/build_context/target_info.rs

@@ -140,6 +140,13 @@ impl FileType {
             should_replace_hyphens: true,
         }
     }
+
+    pub fn output_prefix_suffix(&self, target: &Target) -> (String, String) {
+        (
+            format!("{}{}-", self.prefix, target.crate_name()),
+            self.suffix.clone(),
+        )
+    }
 }
 
 impl TargetInfo {

src/cargo/ops/cargo_clean.rs

@@ -7,10 +7,11 @@ use crate::util::edit_distance;
 use crate::util::errors::CargoResult;
 use crate::util::interning::InternedString;
 use crate::util::{GlobalContext, Progress, ProgressStyle};
-use anyhow::bail;
+use anyhow::{Context as _, bail};
 use cargo_util::paths;
 use indexmap::IndexSet;
-use std::collections::{HashMap, HashSet};
+use std::borrow::Cow;
+use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
 use std::fs;
 use std::path::{Path, PathBuf};
 use std::rc::Rc;
@@ -195,6 +196,8 @@ fn clean_specs(
     // Try to reduce the amount of times we iterate over the same target directory by storing away
     // the directories we've iterated over (and cleaned for a given package).
     let mut cleaned_packages: HashMap<_, HashSet<_>> = HashMap::default();
+    // Root directory -> (prefix, [suffixes])
+    let mut packages_to_clean: BTreeMap<Rc<Path>, BTreeMap<_, BTreeSet<_>>> = Default::default();
     for pkg in packages {
         let pkg_dir = format!("{}-*", pkg.name());
         clean_ctx.progress.on_cleaning_package(&pkg.name())?;
@@ -267,8 +270,8 @@ fn clean_specs(
                 continue;
             }
             let crate_name: Rc<str> = target.crate_name().into();
-            let path_dot: &str = &format!("{crate_name}.");
-            let path_dash: &str = &format!("{crate_name}-");
+            let path_dot = format!("{crate_name}.");
+            let path_dash = format!("{crate_name}-");
             for &mode in &[
                 CompileMode::Build,
                 CompileMode::Test,
@@ -292,14 +295,18 @@ fn clean_specs(
                         }
                         _ => (layout.build_dir().legacy_deps(), Some(artifact_dir.dest())),
                     };
-                    let mut dir_glob_str = escape_glob_path(dir)?;
-                    let dir_glob = Path::new(&dir_glob_str);
+                    let dir = Rc::<Path>::from(dir);
                     for file_type in file_types {
                         // Some files include a hash in the filename, some don't.
-                        let hashed_name = file_type.output_filename(target, Some("*"));
+                        let (prefix, suffix) = file_type.output_prefix_suffix(target);
                         let unhashed_name = file_type.output_filename(target, None);
+                        packages_to_clean
+                            .entry(dir.clone())
+                            .or_default()
+                            .entry(prefix)
+                            .or_default()
+                            .extend([Cow::Owned(suffix)]);
 
-                        clean_ctx.rm_rf_glob(&dir_glob.join(&hashed_name))?;
                         clean_ctx.rm_rf(&dir.join(&unhashed_name))?;
 
                         // Remove the uplifted copy.
@@ -314,37 +321,44 @@ fn clean_specs(
                     let unhashed_dep_info = dir.join(format!("{}.d", crate_name));
                     clean_ctx.rm_rf(&unhashed_dep_info)?;
 
-                    if !dir_glob_str.ends_with(std::path::MAIN_SEPARATOR) {
-                        dir_glob_str.push(std::path::MAIN_SEPARATOR);
-                    }
-                    dir_glob_str.push('*');
-                    let dir_glob_str: Rc<str> = dir_glob_str.into();
                     if cleaned_packages
-                        .entry(dir_glob_str.clone())
+                        .entry(dir.clone())
                         .or_default()
                         .insert(crate_name.clone())
                     {
-                        let paths = [
-                            // Remove dep-info file generated by rustc. It is not tracked in
-                            // file_types. It does not have a prefix.
-                            (path_dash, ".d"),
-                            // Remove split-debuginfo files generated by rustc.
-                            (path_dot, ".o"),
-                            (path_dot, ".dwo"),
-                            (path_dot, ".dwp"),
-                        ];
-                        clean_ctx.rm_rf_prefix_list(&dir_glob_str, &paths)?;
+                        let to_clean = packages_to_clean.entry(dir).or_default();
+                        // Remove dep-info file generated by rustc. It is not tracked in
+                        // file_types. It does not have a prefix.
+                        to_clean
+                            .entry(path_dash.clone())
+                            .or_default()
+                            .extend([Cow::Borrowed(".d")]);
+                        // Remove split-debuginfo files generated by rustc.
+                        to_clean.entry(path_dot.clone()).or_default().extend([
+                            Cow::Borrowed(".o"),
+                            Cow::Borrowed(".dwo"),
+                            Cow::Borrowed(".dwp"),
+                        ]);
                     }
 
                     // TODO: what to do about build_script_build?
-                    let dir = escape_glob_path(layout.build_dir().incremental())?;
-                    let incremental = Path::new(&dir).join(format!("{}-*", crate_name));
-                    clean_ctx.rm_rf_glob(&incremental)?;
+                    let dir = Rc::from(layout.build_dir().incremental());
+
+                    packages_to_clean
+                        .entry(dir)
+                        .or_default()
+                        .entry(path_dash.clone())
+                        .or_default()
+                        .extend([Cow::Borrowed("")]);
                 }
             }
         }
     }
 
+    for (base_dir, entries_to_clean) in packages_to_clean {
+        clean_ctx.rm_rf_prefix_list(&base_dir, &entries_to_clean)?;
+    }
+
     Ok(())
 }
 
@@ -403,36 +417,28 @@ impl<'gctx> CleanContext<'gctx> {
         Ok(())
     }
 
-    fn rm_rf_glob(&mut self, pattern: &Path) -> CargoResult<()> {
-        // TODO: Display utf8 warning to user?  Or switch to globset?
-        let pattern = pattern
-            .to_str()
-            .ok_or_else(|| anyhow::anyhow!("expected utf-8 path"))?;
-        for path in glob::glob(pattern)? {
-            self.rm_rf(&path?)?;
-        }
-        Ok(())
-    }
-
-    /// Removes files matching a glob and any of the provided filename patterns (prefix/suffix pairs).
-    ///
-    /// This function iterates over files matching a glob (`pattern`) and removes those whose
-    /// filenames start and end with specific prefix/suffix pairs. It should be more efficient for
-    /// operations involving multiple prefix/suffix pairs, as it iterates over the directory
-    /// only once, unlike making multiple calls to [`Self::rm_rf_glob`].
+    /// Removes children of `root_directory` that start with any of the provided prefix/suffix pairs.
     fn rm_rf_prefix_list(
         &mut self,
-        pattern: &str,
-        path_matchers: &[(&str, &str)],
+        root_directory: &Path,
+        path_matchers: &BTreeMap<String, BTreeSet<Cow<'static, str>>>,
     ) -> CargoResult<()> {
-        for path in glob::glob(pattern)? {
-            let path = path?;
-            let filename = path.file_name().and_then(|name| name.to_str()).unwrap();
-            if path_matchers
-                .iter()
-                .any(|(prefix, suffix)| filename.starts_with(prefix) && filename.ends_with(suffix))
-            {
-                self.rm_rf(&path)?;
+        let Ok(dir) = std::fs::read_dir(root_directory) else {
+            return Ok(());
+        };
+        for entry in dir {
+            let entry = entry?;
+            let filename = entry.file_name();
+            let filename = filename
+                .to_str()
+                .context("Expected file name to be a valid UTF-8 string")?;
+            if path_matchers.iter().any(|(prefix, suffixes)| {
+                filename.starts_with(prefix)
+                    && suffixes
+                        .iter()
+                        .any(|suffix| filename.ends_with(suffix.as_ref()))
+            }) {
+                self.rm_rf(&entry.path())?;
             }
         }
         Ok(())