Speed up tidy again

src/bootstrap/Cargo.toml

@@ -95,6 +95,7 @@ insta = "1.43"
 # dependencies, only bootstrap itself.
 [profile.dev]
 debug = 0
+debug-assertions = false
 
 [profile.dev.package]
 # Only use debuginfo=1 to further reduce compile times.

src/bootstrap/src/core/build_steps/test.rs

@@ -1720,7 +1720,10 @@ HELP: to skip test's attempt to check tidiness, pass `--skip src/tools/tidy` to
         }
 
         builder.info("tidy check");
-        cmd.delay_failure().run(builder);
+        let ctx = builder.as_ref();
+        let mut cmd = cmd.delay_failure();
+        // spawn child in background so we can do additional work in parallel
+        let tidy = cmd.start(ctx);
 
         builder.info("x.py completions check");
         let completion_paths = get_completion_paths(builder);
@@ -1736,6 +1739,9 @@ HELP: to skip test's attempt to check tidiness, pass `--skip src/tools/tidy` to
             helpers::exit_process(1);
         }
 
+        // now wait for the child to complete
+        tidy.wait_for_output(ctx);
+
         builder.info("x.py help check");
         if builder.config.cmd.bless() {
             builder.ensure(crate::core::build_steps::run::GenerateHelp);

src/bootstrap/src/utils/exec.rs

@@ -331,6 +331,11 @@ impl<'a> BootstrapCommand {
         self
     }
 
+    #[track_caller]
+    pub fn start(&mut self, exec_ctx: impl AsRef<ExecutionContext>) -> DeferredCommand<'_> {
+        exec_ctx.as_ref().start(self, OutputMode::Capture, OutputMode::Capture)
+    }
+
     /// Run the command, while printing stdout and stderr.
     /// Returns true if the command has succeeded.
     #[track_caller]

src/tools/tidy/src/bins.rs

@@ -111,7 +111,7 @@ mod os_impl {
 
     #[cfg(unix)]
     pub fn check(path: &Path, tidy_ctx: TidyCtx) {
-        let mut check = tidy_ctx.start_check("bins");
+        let check = tidy_ctx.start_check("bins");
 
         use std::ffi::OsStr;
 

src/tools/tidy/src/codegen.rs

@@ -20,7 +20,7 @@ fn has_supported_extension(path: &Path) -> bool {
 }
 
 pub fn check(path: &Path, tidy_ctx: TidyCtx) {
-    let mut check = tidy_ctx.start_check(CheckId::new("codegen").path(path));
+    let check = tidy_ctx.start_check(CheckId::new("codegen").path(path));
 
     fn skip(path: &Path, is_dir: bool) -> bool {
         if path.file_name().is_some_and(|name| name.to_string_lossy().starts_with(".#")) {

src/tools/tidy/src/debug_artifacts.rs

@@ -8,7 +8,7 @@ use crate::walk::{filter_dirs, filter_not_rust, walk};
 const GRAPHVIZ_POSTFLOW_MSG: &str = "`borrowck_graphviz_postflow` attribute in test";
 
 pub fn check(test_dir: &Path, tidy_ctx: TidyCtx) {
-    let mut check = tidy_ctx.start_check(CheckId::new("debug_artifacts").path(test_dir));
+    let check = tidy_ctx.start_check(CheckId::new("debug_artifacts").path(test_dir));
 
     walk(
         test_dir,

src/tools/tidy/src/deps.rs

@@ -3,8 +3,10 @@
 use std::collections::{BTreeSet, HashMap, HashSet};
 use std::fmt::{Display, Formatter};
 use std::fs::{self, read_dir};
-use std::io;
 use std::path::Path;
+use std::sync::atomic::{AtomicBool, Ordering};
+use std::thread::Scope;
+use std::{io, thread};
 
 use cargo_metadata::semver::Version;
 use cargo_metadata::{Metadata, Package, PackageId};
@@ -630,62 +632,80 @@ const PERMITTED_CRANELIFT_DEPENDENCIES: &[&str] = &[
 ///
 /// `root` is path to the directory with the root `Cargo.toml` (for the workspace). `cargo` is path
 /// to the cargo executable.
-pub fn check(root: &Path, cargo: &Path, tidy_ctx: TidyCtx) {
+pub fn check(
+    root: &Path,
+    cargo: &Path,
+    _scope: &'_ Scope<'_, '_>,
+    sem: &'_ crate::Semaphore,
+    tidy_ctx: TidyCtx,
+) {
     let mut check = tidy_ctx.start_check("deps");
     let bless = tidy_ctx.is_bless_enabled();
+    let is_ci = tidy_ctx.is_running_on_ci();
 
-    let mut checked_runtime_licenses = false;
+    let checked_runtime_licenses = AtomicBool::new(false);
 
     check_proc_macro_dep_list(root, cargo, bless, &mut check);
 
-    for &WorkspaceInfo { path, exceptions, crates_and_deps, submodules } in WORKSPACES {
-        if has_missing_submodule(root, submodules, tidy_ctx.is_running_on_ci()) {
-            continue;
-        }
+    thread::scope(|s| {
+        for (i, WorkspaceInfo { path, exceptions, crates_and_deps, submodules }) in
+            WORKSPACES.iter().enumerate()
+        {
+            let check = &check;
+            let checked_runtime_licenses = &checked_runtime_licenses;
+            let guard = if i > 0 { Some(sem.acquire()) } else { None };
+            s.spawn(move || {
+                if has_missing_submodule(root, submodules, is_ci) {
+                    return;
+                }
 
-        if !root.join(path).join("Cargo.lock").exists() {
-            check.error(format!("the `{path}` workspace doesn't have a Cargo.lock"));
-            continue;
-        }
+                if !root.join(path).join("Cargo.lock").exists() {
+                    check.error(format!("the `{path}` workspace doesn't have a Cargo.lock"));
+                    return;
+                }
 
-        let mut cmd = cargo_metadata::MetadataCommand::new();
-        cmd.cargo_path(cargo)
-            .manifest_path(root.join(path).join("Cargo.toml"))
-            .features(cargo_metadata::CargoOpt::AllFeatures)
-            .other_options(vec!["--locked".to_owned()]);
-        let metadata = t!(cmd.exec());
-
-        // Check for packages which have been moved into a different workspace and not updated
-        let absolute_root =
-            if path == "." { root.to_path_buf() } else { t!(std::path::absolute(root.join(path))) };
-        let absolute_root_real = t!(std::path::absolute(&metadata.workspace_root));
-        if absolute_root_real != absolute_root {
-            check.error(format!("{path} is part of another workspace ({} != {}), remove from `WORKSPACES` ({WORKSPACE_LOCATION})", absolute_root.display(), absolute_root_real.display()));
-        }
-        check_license_exceptions(&metadata, path, exceptions, &mut check);
-        if let Some((crates, permitted_deps, location)) = crates_and_deps {
-            let descr = crates.get(0).unwrap_or(&path);
-            check_permitted_dependencies(
-                &metadata,
-                descr,
-                permitted_deps,
-                crates,
-                location,
-                &mut check,
-            );
-        }
+                let mut cmd = cargo_metadata::MetadataCommand::new();
+                cmd.cargo_path(cargo)
+                    .manifest_path(root.join(path).join("Cargo.toml"))
+                    .features(cargo_metadata::CargoOpt::AllFeatures)
+                    .other_options(vec!["--locked".to_owned()]);
+                let metadata = t!(cmd.exec());
+
+                // Check for packages which have been moved into a different workspace and not updated
+                let absolute_root =
+                    if *path == "." { root.to_path_buf() } else { t!(std::path::absolute(root.join(path))) };
+                let absolute_root_real = t!(std::path::absolute(&metadata.workspace_root));
+                if absolute_root_real != absolute_root {
+                    check.error(format!("{path} is part of another workspace ({} != {}), remove from `WORKSPACES` ({WORKSPACE_LOCATION})", absolute_root.display(), absolute_root_real.display()));
+                }
+                check_license_exceptions(&metadata, path, exceptions, &check);
+                if let Some((crates, permitted_deps, location)) = &crates_and_deps {
+                    let descr = crates.get(0).unwrap_or(&path);
+                    check_permitted_dependencies(
+                        &metadata,
+                        descr,
+                        permitted_deps,
+                        crates,
+                        location,
+                        &check,
+                    );
+                }
 
-        if path == "library" {
-            check_runtime_license_exceptions(&metadata, &mut check);
-            check_runtime_no_duplicate_dependencies(&metadata, &mut check);
-            check_runtime_no_proc_macros(&metadata, &mut check);
-            checked_runtime_licenses = true;
+                if *path == "library" {
+                    check_runtime_license_exceptions(&metadata, &check);
+                    check_runtime_no_duplicate_dependencies(&metadata, &check);
+                    check_runtime_no_proc_macros(&metadata, &check);
+                    checked_runtime_licenses.store(true, Ordering::Relaxed);
+                }
+
+                let _guard = guard;
+            });
         }
-    }
+    });
 
     // Sanity check to ensure we don't accidentally remove the workspace containing the runtime
     // crates.
-    assert!(checked_runtime_licenses);
+    assert!(checked_runtime_licenses.load(Ordering::Relaxed));
 }
 
 /// Ensure the list of proc-macro crate transitive dependencies is up to date
@@ -788,7 +808,7 @@ pub fn has_missing_submodule(root: &Path, submodules: &[&str], is_ci: bool) -> b
 ///
 /// Unlike for tools we don't allow exceptions to the `LICENSES` list for the runtime with the sole
 /// exception of `fortanix-sgx-abi` which is only used on x86_64-fortanix-unknown-sgx.
-fn check_runtime_license_exceptions(metadata: &Metadata, check: &mut RunningCheck) {
+fn check_runtime_license_exceptions(metadata: &Metadata, check: &RunningCheck) {
     for pkg in &metadata.packages {
         if pkg.source.is_none() {
             // No need to check local packages.
@@ -823,7 +843,7 @@ fn check_license_exceptions(
     metadata: &Metadata,
     workspace: &str,
     exceptions: &[(&str, &str)],
-    check: &mut RunningCheck,
+    check: &RunningCheck,
 ) {
     // Validate the EXCEPTIONS list hasn't changed.
     for (name, license) in exceptions {
@@ -890,7 +910,7 @@ fn check_license_exceptions(
     }
 }
 
-fn check_runtime_no_duplicate_dependencies(metadata: &Metadata, check: &mut RunningCheck) {
+fn check_runtime_no_duplicate_dependencies(metadata: &Metadata, check: &RunningCheck) {
     let mut seen_pkgs = HashSet::new();
     for pkg in &metadata.packages {
         if pkg.source.is_none() {
@@ -906,7 +926,7 @@ fn check_runtime_no_duplicate_dependencies(metadata: &Metadata, check: &mut Runn
     }
 }
 
-fn check_runtime_no_proc_macros(metadata: &Metadata, check: &mut RunningCheck) {
+fn check_runtime_no_proc_macros(metadata: &Metadata, check: &RunningCheck) {
     for pkg in &metadata.packages {
         if pkg.targets.iter().any(|target| target.is_proc_macro()) {
             check.error(format!(
@@ -928,8 +948,8 @@ fn check_permitted_dependencies(
     descr: &str,
     permitted_dependencies: &[&'static str],
     restricted_dependency_crates: &[&'static str],
-    permitted_location: ListLocation,
-    check: &mut RunningCheck,
+    permitted_location: &ListLocation,
+    check: &RunningCheck,
 ) {
     let mut has_permitted_dep_error = false;
     let mut deps = HashSet::new();

src/tools/tidy/src/diagnostics.rs

@@ -2,6 +2,7 @@ use std::collections::HashSet;
 use std::fmt::{Display, Formatter};
 use std::io;
 use std::path::{Path, PathBuf};
+use std::sync::atomic::{AtomicBool, Ordering};
 use std::sync::{Arc, Mutex};
 
 use build_helper::ci::CiEnv;
@@ -86,10 +87,10 @@ impl TidyCtx {
         ctx.start_check(id.clone());
         RunningCheck {
             id,
-            bad: false,
+            bad: AtomicBool::new(false),
             ctx: self.diag_ctx.clone(),
             #[cfg(test)]
-            errors: vec![],
+            errors: Mutex::new(vec![]),
         }
     }
 
@@ -228,10 +229,10 @@ impl FinishedCheck {
 /// Represents a single tidy check, identified by its `name`, running.
 pub struct RunningCheck {
     id: CheckId,
-    bad: bool,
+    bad: AtomicBool,
     ctx: Arc<Mutex<DiagCtxInner>>,
     #[cfg(test)]
-    errors: Vec<String>,
+    errors: Mutex<Vec<String>>,
 }
 
 impl RunningCheck {
@@ -245,34 +246,34 @@ impl RunningCheck {
     }
 
     /// Immediately output an error and mark the check as failed.
-    pub fn error<T: Display>(&mut self, msg: T) {
+    pub fn error<T: Display>(&self, msg: T) {
         self.mark_as_bad();
         let msg = msg.to_string();
         output_message(&msg, Some(&self.id), Some(COLOR_ERROR));
         #[cfg(test)]
-        self.errors.push(msg);
+        self.errors.lock().unwrap().push(msg);
     }
 
     /// Immediately output a warning.
-    pub fn warning<T: Display>(&mut self, msg: T) {
+    pub fn warning<T: Display>(&self, msg: T) {
         output_message(&msg.to_string(), Some(&self.id), Some(COLOR_WARNING));
     }
 
     /// Output an informational message
-    pub fn message<T: Display>(&mut self, msg: T) {
+    pub fn message<T: Display>(&self, msg: T) {
         output_message(&msg.to_string(), Some(&self.id), None);
     }
 
     /// Output a message only if verbose output is enabled.
-    pub fn verbose_msg<T: Display>(&mut self, msg: T) {
+    pub fn verbose_msg<T: Display>(&self, msg: T) {
         if self.is_verbose_enabled() {
             self.message(msg);
         }
     }
 
     /// Has an error already occurred for this check?
     pub fn is_bad(&self) -> bool {
-        self.bad
+        self.bad.load(Ordering::Relaxed)
     }
 
     /// Is verbose output enabled?
@@ -282,17 +283,20 @@ impl RunningCheck {
 
     #[cfg(test)]
     pub fn get_errors(&self) -> Vec<String> {
-        self.errors.clone()
+        self.errors.lock().unwrap().clone()
     }
 
-    fn mark_as_bad(&mut self) {
-        self.bad = true;
+    fn mark_as_bad(&self) {
+        self.bad.store(true, Ordering::Relaxed);
     }
 }
 
 impl Drop for RunningCheck {
     fn drop(&mut self) {
-        self.ctx.lock().unwrap().finish_check(FinishedCheck { id: self.id.clone(), bad: self.bad })
+        self.ctx.lock().unwrap().finish_check(FinishedCheck {
+            id: self.id.clone(),
+            bad: self.bad.load(Ordering::Relaxed),
+        })
     }
 }
 

src/tools/tidy/src/edition.rs

@@ -6,7 +6,7 @@ use crate::diagnostics::{CheckId, TidyCtx};
 use crate::walk::{filter_dirs, walk};
 
 pub fn check(path: &Path, tidy_ctx: TidyCtx) {
-    let mut check = tidy_ctx.start_check(CheckId::new("edition").path(path));
+    let check = tidy_ctx.start_check(CheckId::new("edition").path(path));
     walk(path, |path, _is_dir| filter_dirs(path), &mut |entry, contents| {
         let file = entry.path();
         let filename = file.file_name().unwrap();

src/tools/tidy/src/extdeps.rs

@@ -16,7 +16,7 @@ const ALLOWED_SOURCES: &[&str] = &[
 /// Checks for external package sources. `root` is the path to the directory that contains the
 /// workspace `Cargo.toml`.
 pub fn check(root: &Path, tidy_ctx: TidyCtx) {
-    let mut check = tidy_ctx.start_check("extdeps");
+    let check = tidy_ctx.start_check("extdeps");
 
     for &WorkspaceInfo { path, submodules, .. } in crate::deps::WORKSPACES {
         if crate::deps::has_missing_submodule(root, submodules, tidy_ctx.is_running_on_ci()) {