Skip to content

Commit 3f7d871

Browse files
osiewiczdinocosta
andcommitted
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 rust-lang#16263); for Zed, `cargo clean --workspace` went down from 73 seconds to 3 seconds. We have 216 workspace members. Co-authored-by: dino <[email protected]>
1 parent 8363559 commit 3f7d871

File tree

2 files changed

+65
-29
lines changed

2 files changed

+65
-29
lines changed

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

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,13 @@ impl FileType {
140140
should_replace_hyphens: true,
141141
}
142142
}
143+
144+
pub fn output_prefix_suffix(&self, target: &Target) -> (String, String) {
145+
(
146+
format!("{}{}-", self.prefix, target.crate_name()),
147+
self.suffix.clone(),
148+
)
149+
}
143150
}
144151

145152
impl TargetInfo {

src/cargo/ops/cargo_clean.rs

Lines changed: 58 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ use crate::util::interning::InternedString;
99
use crate::util::{GlobalContext, Progress, ProgressStyle};
1010
use anyhow::bail;
1111
use cargo_util::paths;
12-
use std::collections::{HashMap, HashSet};
12+
use std::borrow::Cow;
13+
use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
1314
use std::fs;
1415
use std::path::{Path, PathBuf};
1516
use std::rc::Rc;
@@ -194,6 +195,7 @@ fn clean_specs(
194195
// Try to reduce the amount of times we iterate over the same target directory by storing away
195196
// the directories we've iterated over (and cleaned for a given package).
196197
let mut cleaned_packages: HashMap<_, HashSet<_>> = HashMap::default();
198+
let mut packages_to_clean: BTreeMap<Rc<str>, BTreeMap<_, BTreeSet<_>>> = Default::default();
197199
for pkg in packages {
198200
let pkg_dir = format!("{}-*", pkg.name());
199201
clean_ctx.progress.on_cleaning_package(&pkg.name())?;
@@ -218,8 +220,8 @@ fn clean_specs(
218220
continue;
219221
}
220222
let crate_name: Rc<str> = target.crate_name().into();
221-
let path_dot: &str = &format!("{crate_name}.");
222-
let path_dash: &str = &format!("{crate_name}-");
223+
let path_dot = format!("{crate_name}.");
224+
let path_dash = format!("{crate_name}-");
223225
for &mode in &[
224226
CompileMode::Build,
225227
CompileMode::Test,
@@ -251,13 +253,23 @@ fn clean_specs(
251253
),
252254
};
253255
let mut dir_glob_str = escape_glob_path(dir)?;
254-
let dir_glob = Path::new(&dir_glob_str);
256+
if !dir_glob_str.ends_with(std::path::MAIN_SEPARATOR) {
257+
dir_glob_str.push(std::path::MAIN_SEPARATOR);
258+
}
259+
dir_glob_str.push('*');
260+
let dir_glob_str: Rc<str> = dir_glob_str.into();
261+
255262
for file_type in file_types {
256263
// Some files include a hash in the filename, some don't.
257-
let hashed_name = file_type.output_filename(target, Some("*"));
264+
let (prefix, suffix) = file_type.output_prefix_suffix(target);
258265
let unhashed_name = file_type.output_filename(target, None);
266+
packages_to_clean
267+
.entry(Rc::from(dir_glob_str.as_ref()))
268+
.or_default()
269+
.entry(prefix)
270+
.or_default()
271+
.extend([Cow::Owned(suffix)]);
259272

260-
clean_ctx.rm_rf_glob(&dir_glob.join(&hashed_name))?;
261273
clean_ctx.rm_rf(&dir.join(&unhashed_name))?;
262274

263275
// Remove the uplifted copy.
@@ -272,37 +284,47 @@ fn clean_specs(
272284
let unhashed_dep_info = dir.join(format!("{}.d", crate_name));
273285
clean_ctx.rm_rf(&unhashed_dep_info)?;
274286

275-
if !dir_glob_str.ends_with(std::path::MAIN_SEPARATOR) {
276-
dir_glob_str.push(std::path::MAIN_SEPARATOR);
277-
}
278-
dir_glob_str.push('*');
279-
let dir_glob_str: Rc<str> = dir_glob_str.into();
280287
if cleaned_packages
281288
.entry(dir_glob_str.clone())
282289
.or_default()
283290
.insert(crate_name.clone())
284291
{
285-
let paths = [
286-
// Remove dep-info file generated by rustc. It is not tracked in
287-
// file_types. It does not have a prefix.
288-
(path_dash, ".d"),
289-
// Remove split-debuginfo files generated by rustc.
290-
(path_dot, ".o"),
291-
(path_dot, ".dwo"),
292-
(path_dot, ".dwp"),
293-
];
294-
clean_ctx.rm_rf_prefix_list(&dir_glob_str, &paths)?;
292+
let to_clean = packages_to_clean.entry(dir_glob_str).or_default();
293+
// Remove dep-info file generated by rustc. It is not tracked in
294+
// file_types. It does not have a prefix.
295+
to_clean
296+
.entry(path_dash.clone())
297+
.or_default()
298+
.extend([Cow::Borrowed(".d")]);
299+
// Remove split-debuginfo files generated by rustc.
300+
to_clean.entry(path_dot.clone()).or_default().extend([
301+
Cow::Borrowed(".o"),
302+
Cow::Borrowed(".dwo"),
303+
Cow::Borrowed(".dwp"),
304+
]);
295305
}
296306

297307
// TODO: what to do about build_script_build?
298-
let dir = escape_glob_path(layout.build_dir().incremental())?;
299-
let incremental = Path::new(&dir).join(format!("{}-*", crate_name));
300-
clean_ctx.rm_rf_glob(&incremental)?;
308+
let mut dir = escape_glob_path(layout.build_dir().incremental())?;
309+
if !dir.ends_with(std::path::MAIN_SEPARATOR) {
310+
dir.push(std::path::MAIN_SEPARATOR);
311+
}
312+
dir.push('*');
313+
packages_to_clean
314+
.entry(dir.into())
315+
.or_default()
316+
.entry(path_dash.clone())
317+
.or_default()
318+
.extend([Cow::Borrowed("")]);
301319
}
302320
}
303321
}
304322
}
305323

324+
for (dir_glob_str, entries_to_clean) in packages_to_clean {
325+
clean_ctx.rm_rf_prefix_list(&dir_glob_str, &entries_to_clean)?;
326+
}
327+
306328
Ok(())
307329
}
308330

@@ -381,15 +403,22 @@ impl<'gctx> CleanContext<'gctx> {
381403
fn rm_rf_prefix_list(
382404
&mut self,
383405
pattern: &str,
384-
path_matchers: &[(&str, &str)],
406+
path_matchers: &BTreeMap<String, BTreeSet<Cow<'static, str>>>,
385407
) -> CargoResult<()> {
386408
for path in glob::glob(pattern)? {
387409
let path = path?;
388410
let filename = path.file_name().and_then(|name| name.to_str()).unwrap();
389-
if path_matchers
390-
.iter()
391-
.any(|(prefix, suffix)| filename.starts_with(prefix) && filename.ends_with(suffix))
392-
{
411+
let first_char = filename.chars().next();
412+
let mut subrange = path_matchers.iter();
413+
// .range(..filename.to_owned())
414+
// .rev()
415+
// .take_while(|(prefix, _suffixes)| first_char == prefix.chars().next());
416+
if subrange.any(|(prefix, suffixes)| {
417+
filename.starts_with(prefix)
418+
&& suffixes
419+
.iter()
420+
.any(|suffix| filename.ends_with(suffix.as_ref()))
421+
}) {
393422
self.rm_rf(&path)?;
394423
}
395424
}

0 commit comments

Comments
 (0)