Skip to content
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.

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

Clean package perf improvements #13818

Merged
merged 8 commits into from
May 2, 2024
71 changes: 55 additions & 16 deletions src/cargo/ops/cargo_clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,10 @@ use crate::util::interning::InternedString;
use crate::util::{human_readable_bytes, GlobalContext, Progress, ProgressStyle};
use anyhow::bail;
use cargo_util::paths;
use std::collections::{HashMap, HashSet};
use std::fs;
use std::path::{Path, PathBuf};
use std::rc::Rc;

pub struct CleanOptions<'gctx> {
pub gctx: &'gctx GlobalContext,
Expand Down Expand Up @@ -169,6 +171,9 @@ fn clean_specs(

clean_ctx.progress = Box::new(CleaningPackagesBar::new(clean_ctx.gctx, packages.len()));

// 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();
for pkg in packages {
let pkg_dir = format!("{}-*", pkg.name());
clean_ctx.progress.on_cleaning_package(&pkg.name())?;
Expand All @@ -192,7 +197,9 @@ fn clean_specs(
}
continue;
}
let crate_name = target.crate_name();
let crate_name: Rc<str> = target.crate_name().into();
let path_dot: Rc<str> = format!("{crate_name}.").into();
let path_dash: Rc<str> = format!("{crate_name}-").into();
osiewicz marked this conversation as resolved.
Show resolved Hide resolved
for &mode in &[
CompileMode::Build,
CompileMode::Test,
Expand All @@ -212,28 +219,15 @@ fn clean_specs(
TargetKind::Test | TargetKind::Bench => (layout.deps(), None),
_ => (layout.deps(), Some(layout.dest())),
};
let mut dir_glob_str = escape_glob_path(dir)?;
let dir_glob = Path::new(&dir_glob_str);
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 unhashed_name = file_type.output_filename(target, None);
let dir_glob = escape_glob_path(dir)?;
let dir_glob = Path::new(&dir_glob);

clean_ctx.rm_rf_glob(&dir_glob.join(&hashed_name))?;
clean_ctx.rm_rf(&dir.join(&unhashed_name))?;
// Remove dep-info file generated by rustc. It is not tracked in
// file_types. It does not have a prefix.
let hashed_dep_info = dir_glob.join(format!("{}-*.d", crate_name));
clean_ctx.rm_rf_glob(&hashed_dep_info)?;
let unhashed_dep_info = dir.join(format!("{}.d", crate_name));
clean_ctx.rm_rf(&unhashed_dep_info)?;
// Remove split-debuginfo files generated by rustc.
let split_debuginfo_obj = dir_glob.join(format!("{}.*.o", crate_name));
clean_ctx.rm_rf_glob(&split_debuginfo_obj)?;
let split_debuginfo_dwo = dir_glob.join(format!("{}.*.dwo", crate_name));
clean_ctx.rm_rf_glob(&split_debuginfo_dwo)?;
let split_debuginfo_dwp = dir_glob.join(format!("{}.*.dwp", crate_name));
clean_ctx.rm_rf_glob(&split_debuginfo_dwp)?;

// Remove the uplifted copy.
if let Some(uplift_dir) = uplift_dir {
Expand All @@ -244,6 +238,31 @@ fn clean_specs(
clean_ctx.rm_rf(&dep_info)?;
}
}
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())
.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.clone(), ".d"),
// Remove split-debuginfo files generated by rustc.
(path_dot.clone(), ".o"),
(path_dot.clone(), ".dwo"),
(path_dot.clone(), ".dwp"),
];
clean_ctx.rm_rf_prefix_list(&dir_glob_str, &paths)?;
}

// TODO: what to do about build_script_build?
let dir = escape_glob_path(layout.incremental())?;
let incremental = Path::new(&dir).join(format!("{}-*", crate_name));
Expand Down Expand Up @@ -322,6 +341,26 @@ impl<'gctx> CleanContext<'gctx> {
Ok(())
}

/// Iterates over files matching a glob (`pattern`), removing any files whose filenames start and end with provided prefix/suffix pair.
/// Compared to multiple separate calls to [`Self::rm_rf_glob`], this method iterates over the directory just once, which is why
/// it may be preferable for working with multiple prefix/suffix pairs.
osiewicz marked this conversation as resolved.
Show resolved Hide resolved
fn rm_rf_prefix_list(
&mut self,
pattern: &str,
path_matchers: &[(Rc<str>, &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)?;
}
}
Ok(())
}

pub fn rm_rf(&mut self, path: &Path) -> CargoResult<()> {
let meta = match fs::symlink_metadata(path) {
Ok(meta) => meta,
Expand Down