Improve speed of cargo dev fmt.

Jarcho · Jarcho · commit 229374322864 · 2025-05-21T10:06:30.000-04:00
diff --git a/clippy_dev/src/fmt.rs b/clippy_dev/src/fmt.rs
@@ -1,5 +1,5 @@
 use crate::utils::{
-    ClippyInfo, ErrAction, FileUpdater, UpdateMode, UpdateStatus, panic_action, run_with_args_split, run_with_output,
+    ErrAction, FileUpdater, UpdateMode, UpdateStatus, panic_action, run_with_output, split_args_for_threads,
 };
 use itertools::Itertools;
 use rustc_lexer::{TokenKind, tokenize};
@@ -260,50 +260,37 @@ fn fmt_syms(update_mode: UpdateMode) {
     );
 }
 
-fn run_rustfmt(clippy: &ClippyInfo, update_mode: UpdateMode) {
+fn run_rustfmt(update_mode: UpdateMode) {
     let mut rustfmt_path = String::from_utf8(run_with_output(
         "rustup which rustfmt",
         Command::new("rustup").args(["which", "rustfmt"]),
     ))
     .expect("invalid rustfmt path");
     rustfmt_path.truncate(rustfmt_path.trim_end().len());
 
-    let mut cargo_path = String::from_utf8(run_with_output(
-        "rustup which cargo",
-        Command::new("rustup").args(["which", "cargo"]),
-    ))
-    .expect("invalid cargo path");
-    cargo_path.truncate(cargo_path.trim_end().len());
-
-    // Start all format jobs first before waiting on the results.
-    let mut children = Vec::with_capacity(16);
-    for &path in &[
-        ".",
-        "clippy_config",
-        "clippy_dev",
-        "clippy_lints",
-        "clippy_lints_internal",
-        "clippy_utils",
-        "rustc_tools_util",
-        "lintcheck",
-    ] {
-        let mut cmd = Command::new(&cargo_path);
-        cmd.current_dir(clippy.path.join(path))
-            .args(["fmt"])
-            .env("RUSTFMT", &rustfmt_path)
-            .stdout(Stdio::null())
-            .stdin(Stdio::null())
-            .stderr(Stdio::piped());
-        if update_mode.is_check() {
-            cmd.arg("--check");
-        }
-        match cmd.spawn() {
-            Ok(x) => children.push(("cargo fmt", x)),
-            Err(ref e) => panic_action(&e, ErrAction::Run, "cargo fmt".as_ref()),
-        }
-    }
+    let args: Vec<_> = WalkDir::new(".")
+        .into_iter()
+        .filter_entry(|e|
+        // Skip traversing some of the larger directories.
+        e.path()
+            .as_os_str()
+            .as_encoded_bytes()
+            .get(2..)
+            .is_none_or(|x| x != "target".as_bytes() && x != ".git".as_bytes())
+        && e.path().file_name().is_none_or(|x| x != "skip_rustfmt"))
+        // e.path().file_name().is_none_or(|x| x != "skip_rustfmt"))
+        .filter_map(|e| {
+            let e = e.expect("error reading `tests`");
+            e.path()
+                .as_os_str()
+                .as_encoded_bytes()
+                .ends_with(b".rs")
+                .then(|| e.into_path().into_os_string())
+        })
+        .collect();
 
-    run_with_args_split(
+    let mut children: Vec<_> = split_args_for_threads(
+        32,
         || {
             let mut cmd = Command::new(&rustfmt_path);
             if update_mode.is_check() {
@@ -312,27 +299,23 @@ fn run_rustfmt(clippy: &ClippyInfo, update_mode: UpdateMode) {
             cmd.stdout(Stdio::null())
                 .stdin(Stdio::null())
                 .stderr(Stdio::piped())
-                .args(["--config", "show_parse_errors=false"]);
+                .args([
+                    "--config",
+                    "show_parse_errors=false",
+                    "--unstable-features",
+                    "--skip-children",
+                ]);
             cmd
         },
-        |cmd| match cmd.spawn() {
-            Ok(x) => children.push(("rustfmt", x)),
-            Err(ref e) => panic_action(&e, ErrAction::Run, "rustfmt".as_ref()),
-        },
-        WalkDir::new("tests")
-            .into_iter()
-            .filter_entry(|p| p.path().file_name().is_none_or(|x| x != "skip_rustfmt"))
-            .filter_map(|e| {
-                let e = e.expect("error reading `tests`");
-                e.path()
-                    .as_os_str()
-                    .as_encoded_bytes()
-                    .ends_with(b".rs")
-                    .then(|| e.into_path().into_os_string())
-            }),
-    );
+        args.iter(),
+    )
+    .map(|mut cmd| match cmd.spawn() {
+        Ok(x) => x,
+        Err(ref e) => panic_action(&e, ErrAction::Run, "rustfmt".as_ref()),
+    })
+    .collect();
 
-    for (name, child) in &mut children {
+    for child in &mut children {
         match child.wait() {
             Ok(status) => match (update_mode, status.exit_ok()) {
                 (UpdateMode::Check | UpdateMode::Change, Ok(())) => {},
@@ -353,25 +336,17 @@ fn run_rustfmt(clippy: &ClippyInfo, update_mode: UpdateMode) {
                     {
                         eprintln!("{s}");
                     }
-                    panic_action(&e, ErrAction::Run, name.as_ref());
+                    panic_action(&e, ErrAction::Run, "rustfmt".as_ref());
                 },
             },
-            Err(ref e) => panic_action(e, ErrAction::Run, name.as_ref()),
+            Err(ref e) => panic_action(e, ErrAction::Run, "rustfmt".as_ref()),
         }
     }
 }
 
 // the "main" function of cargo dev fmt
-pub fn run(clippy: &ClippyInfo, update_mode: UpdateMode) {
-    if clippy.has_intellij_hook {
-        eprintln!(
-            "error: a local rustc repo is enabled as path dependency via `cargo dev setup intellij`.\n\
-            Not formatting because that would format the local repo as well!\n\
-            Please revert the changes to `Cargo.toml`s with `cargo dev remove intellij`."
-        );
-        return;
-    }
-    run_rustfmt(clippy, update_mode);
+pub fn run(update_mode: UpdateMode) {
+    run_rustfmt(update_mode);
     fmt_syms(update_mode);
     if let Err(e) = fmt_conf(update_mode.is_check()) {
         e.display();
diff --git a/clippy_dev/src/main.rs b/clippy_dev/src/main.rs
@@ -26,7 +26,7 @@ fn main() {
             allow_staged,
             allow_no_vcs,
         } => dogfood::dogfood(fix, allow_dirty, allow_staged, allow_no_vcs),
-        DevCommand::Fmt { check } => fmt::run(&clippy, utils::UpdateMode::from_check(check)),
+        DevCommand::Fmt { check } => fmt::run(utils::UpdateMode::from_check(check)),
         DevCommand::UpdateLints { check } => update_lints::update(utils::UpdateMode::from_check(check)),
         DevCommand::NewLint {
             pass,
diff --git a/clippy_dev/src/utils.rs b/clippy_dev/src/utils.rs
@@ -3,12 +3,12 @@ use core::ops::Range;
 use core::slice;
 use core::str::FromStr;
 use rustc_lexer::{self as lexer, FrontmatterAllowed};
-use std::env;
 use std::ffi::OsStr;
 use std::fs::{self, OpenOptions};
 use std::io::{self, Read as _, Seek as _, SeekFrom, Write};
 use std::path::{Path, PathBuf};
 use std::process::{self, Command, ExitStatus, Stdio};
+use std::{env, thread};
 
 #[cfg(not(windows))]
 static CARGO_CLIPPY_EXE: &str = "cargo-clippy";
@@ -682,25 +682,57 @@ pub fn run_with_output(path: &(impl AsRef<Path> + ?Sized), cmd: &mut Command) ->
     f(path.as_ref(), cmd)
 }
 
-pub fn run_with_args_split(
-    mut make_cmd: impl FnMut() -> Command,
-    mut run_cmd: impl FnMut(&mut Command),
-    args: impl Iterator<Item: AsRef<OsStr>>,
-) {
-    let mut cmd = make_cmd();
-    let mut len = 0;
-    for arg in args {
-        len += arg.as_ref().len();
-        cmd.arg(arg);
-        // Very conservative limit
-        if len > 10000 {
-            run_cmd(&mut cmd);
-            cmd = make_cmd();
-            len = 0;
+/// Splits an argument list across multiple `Command` invocations.
+///
+/// The argument list will be split into a number of batches based on
+/// `thread::available_parallelism`, with `min_batch_size` setting a lower bound on the size of each
+/// batch.
+///
+/// If the size of the arguments would exceed the system limit additional batches will be created.
+pub fn split_args_for_threads(
+    min_batch_size: usize,
+    make_cmd: impl FnMut() -> Command,
+    args: impl ExactSizeIterator<Item: AsRef<OsStr>>,
+) -> impl Iterator<Item = Command> {
+    struct Iter<F, I> {
+        make_cmd: F,
+        args: I,
+        batch_size: usize,
+        thread_count: usize,
+    }
+    impl<F, I> Iterator for Iter<F, I>
+    where
+        F: FnMut() -> Command,
+        I: ExactSizeIterator<Item: AsRef<OsStr>>,
+    {
+        type Item = Command;
+        fn next(&mut self) -> Option<Self::Item> {
+            if self.thread_count > 1 {
+                self.thread_count -= 1;
+            }
+            let mut cmd = (self.make_cmd)();
+            let mut cmd_len = 0usize;
+            for arg in self.args.by_ref().take(self.batch_size) {
+                cmd.arg(arg.as_ref());
+                cmd_len += arg.as_ref().len();
+                cmd_len += 8;
+
+                // Windows has a command length limit of 32767; stop before we hit that.
+                if cmd_len > 30000 {
+                    self.batch_size = (self.args.len() / self.thread_count).max(32);
+                    break;
+                }
+            }
+            (cmd_len != 0).then_some(cmd)
         }
     }
-    if len != 0 {
-        run_cmd(&mut cmd);
+    let thread_count = thread::available_parallelism().map_or(1, |x| x.get());
+    let batch_size = (args.len() / thread_count).max(min_batch_size);
+    Iter {
+        make_cmd,
+        args,
+        batch_size,
+        thread_count,
     }
 }
 
