Skip to content

clean: Optimize (legacy) clean with multiple -p specifiers #16264

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+68 −51
Open

clean: Optimize (legacy) clean with multiple -p specifiers #16264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions src/cargo/core/compiler/build_context/target_info.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
112 changes: 61 additions & 51 deletions src/cargo/ops/cargo_clean.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -248,6 +249,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();
let mut directories_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())?;
Expand All @@ -274,8 +277,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,
Expand All @@ -299,14 +302,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);
Copy link
Contributor

@epage epage Dec 3, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How much of a benefit are we seeing from the use of Rc and Cow. I doubt memory allocations are our biggest concern in here.

Copy link
Contributor Author

@osiewicz osiewicz Dec 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haven't really measured it, but I've figured that it wouldn't hurt since we'd be mostly copying static suffixes anyways.
I'm not too attached to that, so I'd be happy to get rid of them.

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);
directories_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.
Expand All @@ -322,36 +329,42 @@ 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 = directories_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());

directories_to_clean
.entry(dir)
.or_default()
.entry(path_dash.clone())
.or_default()
.extend([Cow::Borrowed("")]);
}
}
}
}
for (base_dir, entries_to_clean) in directories_to_clean {
clean_ctx.rm_rf_prefix_list(&base_dir, &entries_to_clean)?;
}
}

Ok(())
Expand Down Expand Up @@ -412,38 +425,35 @@ 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();
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, suffix)| filename.starts_with(prefix) && filename.ends_with(suffix))
.take_while(|(prefix, _)| filename >= prefix.as_str())
.any(|(prefix, suffixes)| {
filename.starts_with(prefix)
&& suffixes
.iter()
.any(|suffix| filename.ends_with(suffix.as_ref()))
})
{
self.rm_rf(&path)?;
self.rm_rf(&entry.path())?;
}
}

Ok(())
}

Expand Down