Rollup merge of #108772 - jyn514:faster-tidy, r=the8472

Speed up tidy quite a lot

I highly recommend reviewing this commit-by-commit. Based on #106440 for convenience.

## Timings

These were collected by running `x test tidy -v` to copy paste the command, then using [`samply record`](https://github.com/mstange/samply).

before (8 threads)
![image](https://user-images.githubusercontent.com/23638587/222965319-352ad2c8-367c-4d74-960a-e4bb161a6aff.png)

after (8 threads) ![image](https://user-images.githubusercontent.com/23638587/222965323-fa846f4e-727a-4bf8-8e3b-1b7b40505cc3.png)

before (64 threads) ![image](https://user-images.githubusercontent.com/23638587/222965302-dc88020c-19e9-49d9-a87d-cad054d717f3.png)
after (64 threads) ![image](https://user-images.githubusercontent.com/23638587/222965335-e73d7622-59de-41d2-9cc4-1bd67042a349.png)

The last commit makes tidy use more threads, so comparing "before (8 threads)" to "after (64 threads)" is IMO the most realistic comparison. Locally, that brings the time for me to run tidy down from 4 to .9 seconds, i.e. the majority of the time for `x test tidy` is now spend running `fmt --check`.

r? `@the8472`

9 files changed, 178 insertions, 163 deletions

diff --git a/src/bootstrap/test.rs b/src/bootstrap/test.rs
index f5d680df113..baddc9da48d 100644
--- a/src/bootstrap/test.rs
+++ b/src/bootstrap/test.rs
@@ -1118,7 +1118,11 @@ impl Step for Tidy {
         cmd.arg(&builder.src);
         cmd.arg(&builder.initial_cargo);
         cmd.arg(&builder.out);
-        cmd.arg(builder.jobs().to_string());
+        // Tidy is heavily IO constrained. Still respect `-j`, but use a higher limit if `jobs` hasn't been configured.
+        let jobs = builder.config.jobs.unwrap_or_else(|| {
+            8 * std::thread::available_parallelism().map_or(1, std::num::NonZeroUsize::get) as u32
+        });
+        cmd.arg(jobs.to_string());
         if builder.is_verbose() {
             cmd.arg("--verbose");
         }
diff --git a/src/tools/tidy/src/bins.rs b/src/tools/tidy/src/bins.rs
index 2d6abe59343..070ce93f97c 100644
--- a/src/tools/tidy/src/bins.rs
+++ b/src/tools/tidy/src/bins.rs
@@ -103,7 +103,7 @@ mod os_impl {
 
         // FIXME: we don't need to look at all binaries, only files that have been modified in this branch
         // (e.g. using `git ls-files`).
-        walk_no_read(path, |path| filter_dirs(path) || path.ends_with("src/etc"), &mut |entry| {
+        walk_no_read(&[path], |path| filter_dirs(path) || path.ends_with("src/etc"), &mut |entry| {
             let file = entry.path();
             let extension = file.extension();
             let scripts = ["py", "sh", "ps1"];
diff --git a/src/tools/tidy/src/debug_artifacts.rs b/src/tools/tidy/src/debug_artifacts.rs
index 2241375eaae..84b13306805 100644
--- a/src/tools/tidy/src/debug_artifacts.rs
+++ b/src/tools/tidy/src/debug_artifacts.rs
@@ -1,21 +1,15 @@
 //! Tidy check to prevent creation of unnecessary debug artifacts while running tests.
 
-use crate::walk::{filter_dirs, walk};
+use crate::walk::{filter_dirs, filter_not_rust, walk};
 use std::path::Path;
 
 const GRAPHVIZ_POSTFLOW_MSG: &str = "`borrowck_graphviz_postflow` attribute in test";
 
 pub fn check(test_dir: &Path, bad: &mut bool) {
-    walk(test_dir, filter_dirs, &mut |entry, contents| {
-        let filename = entry.path();
-        let is_rust = filename.extension().map_or(false, |ext| ext == "rs");
-        if !is_rust {
-            return;
-        }
-
+    walk(test_dir, |path| filter_dirs(path) || filter_not_rust(path), &mut |entry, contents| {
         for (i, line) in contents.lines().enumerate() {
             if line.contains("borrowck_graphviz_postflow") {
-                tidy_error!(bad, "{}:{}: {}", filename.display(), i + 1, GRAPHVIZ_POSTFLOW_MSG);
+                tidy_error!(bad, "{}:{}: {}", entry.path().display(), i + 1, GRAPHVIZ_POSTFLOW_MSG);
             }
         }
     });
diff --git a/src/tools/tidy/src/features.rs b/src/tools/tidy/src/features.rs
index 6d94417a10f..f18feda533c 100644
--- a/src/tools/tidy/src/features.rs
+++ b/src/tools/tidy/src/features.rs
@@ -9,8 +9,9 @@
 //! * All unstable lang features have tests to ensure they are actually unstable.
 //! * Language features in a group are sorted by feature name.
 
-use crate::walk::{filter_dirs, walk, walk_many};
+use crate::walk::{filter_dirs, filter_not_rust, walk, walk_many};
 use std::collections::hash_map::{Entry, HashMap};
+use std::ffi::OsStr;
 use std::fmt;
 use std::fs;
 use std::num::NonZeroU32;
@@ -101,17 +102,15 @@ pub fn check(
             &tests_path.join("rustdoc-ui"),
             &tests_path.join("rustdoc"),
         ],
-        filter_dirs,
+        |path| {
+            filter_dirs(path)
+                || filter_not_rust(path)
+                || path.file_name() == Some(OsStr::new("features.rs"))
+                || path.file_name() == Some(OsStr::new("diagnostic_list.rs"))
+        },
         &mut |entry, contents| {
             let file = entry.path();
             let filename = file.file_name().unwrap().to_string_lossy();
-            if !filename.ends_with(".rs")
-                || filename == "features.rs"
-                || filename == "diagnostic_list.rs"
-            {
-                return;
-            }
-
             let filen_underscore = filename.replace('-', "_").replace(".rs", "");
             let filename_is_gate_test = test_filen_gate(&filen_underscore, &mut features);
 
diff --git a/src/tools/tidy/src/main.rs b/src/tools/tidy/src/main.rs
index d98758ace4f..f59406c404b 100644
--- a/src/tools/tidy/src/main.rs
+++ b/src/tools/tidy/src/main.rs
@@ -13,7 +13,7 @@ use std::path::PathBuf;
 use std::process;
 use std::str::FromStr;
 use std::sync::atomic::{AtomicBool, Ordering};
-use std::thread::{scope, ScopedJoinHandle};
+use std::thread::{self, scope, ScopedJoinHandle};
 
 fn main() {
     let root_path: PathBuf = env::args_os().nth(1).expect("need path to root of repo").into();
@@ -55,16 +55,28 @@ fn main() {
             VecDeque::with_capacity(concurrency.get());
 
         macro_rules! check {
-            ($p:ident $(, $args:expr)* ) => {
+            ($p:ident) => {
+                check!(@ $p, name=format!("{}", stringify!($p)));
+            };
+            ($p:ident, $path:expr $(, $args:expr)* ) => {
+                let shortened = $path.strip_prefix(&root_path).unwrap();
+                let name = if shortened == std::path::Path::new("") {
+                    format!("{} (.)", stringify!($p))
+                } else {
+                    format!("{} ({})", stringify!($p), shortened.display())
+                };
+                check!(@ $p, name=name, $path $(,$args)*);
+            };
+            (@ $p:ident, name=$name:expr $(, $args:expr)* ) => {
                 drain_handles(&mut handles);
 
-                let handle = s.spawn(|| {
+                let handle = thread::Builder::new().name($name).spawn_scoped(s, || {
                     let mut flag = false;
                     $p::check($($args, )* &mut flag);
                     if (flag) {
                         bad.store(true, Ordering::Relaxed);
                     }
-                });
+                }).unwrap();
                 handles.push_back(handle);
             }
         }
@@ -108,7 +120,6 @@ fn main() {
         check!(edition, &library_path);
 
         check!(alphabetical, &src_path);
-        check!(alphabetical, &tests_path);
         check!(alphabetical, &compiler_path);
         check!(alphabetical, &library_path);
 
diff --git a/src/tools/tidy/src/style.rs b/src/tools/tidy/src/style.rs
index 75a4586cb7f..a965c98f484 100644
--- a/src/tools/tidy/src/style.rs
+++ b/src/tools/tidy/src/style.rs
@@ -19,7 +19,7 @@
 
 use crate::walk::{filter_dirs, walk};
 use regex::{Regex, RegexSet};
-use std::path::Path;
+use std::{ffi::OsStr, path::Path};
 
 /// Error code markdown is restricted to 80 columns because they can be
 /// displayed on the console with --example.
@@ -228,21 +228,33 @@ fn is_unexplained_ignore(extension: &str, line: &str) -> bool {
 
 pub fn check(path: &Path, bad: &mut bool) {
     fn skip(path: &Path) -> bool {
-        filter_dirs(path) || skip_markdown_path(path)
+        if path.file_name().map_or(false, |name| name.to_string_lossy().starts_with(".#")) {
+            // vim or emacs temporary file
+            return true;
+        }
+
+        if filter_dirs(path) || skip_markdown_path(path) {
+            return true;
+        }
+
+        let extensions = ["rs", "py", "js", "sh", "c", "cpp", "h", "md", "css", "ftl", "goml"];
+        if extensions.iter().all(|e| path.extension() != Some(OsStr::new(e))) {
+            return true;
+        }
+
+        // We only check CSS files in rustdoc.
+        path.extension().map_or(false, |e| e == "css") && !is_in(path, "src", "librustdoc")
     }
+
     let problematic_consts_strings: Vec<String> = (PROBLEMATIC_CONSTS.iter().map(u32::to_string))
         .chain(PROBLEMATIC_CONSTS.iter().map(|v| format!("{:x}", v)))
         .chain(PROBLEMATIC_CONSTS.iter().map(|v| format!("{:X}", v)))
         .collect();
     let problematic_regex = RegexSet::new(problematic_consts_strings.as_slice()).unwrap();
+
     walk(path, skip, &mut |entry, contents| {
         let file = entry.path();
         let filename = file.file_name().unwrap().to_string_lossy();
-        let extensions =
-            [".rs", ".py", ".js", ".sh", ".c", ".cpp", ".h", ".md", ".css", ".ftl", ".goml"];
-        if extensions.iter().all(|e| !filename.ends_with(e)) || filename.starts_with(".#") {
-            return;
-        }
 
         let is_style_file = filename.ends_with(".css");
         let under_rustfmt = filename.ends_with(".rs") &&
@@ -253,11 +265,6 @@ pub fn check(path: &Path, bad: &mut bool) {
                     a.ends_with("src/doc/book")
             });
 
-        if is_style_file && !is_in(file, "src", "librustdoc") {
-            // We only check CSS files in rustdoc.
-            return;
-        }
-
         if contents.is_empty() {
             tidy_error!(bad, "{}: empty file", file.display());
         }
diff --git a/src/tools/tidy/src/target_specific_tests.rs b/src/tools/tidy/src/target_specific_tests.rs
index f41fa4fcce1..e0fa6aceb85 100644
--- a/src/tools/tidy/src/target_specific_tests.rs
+++ b/src/tools/tidy/src/target_specific_tests.rs
@@ -4,6 +4,8 @@
 use std::collections::BTreeMap;
 use std::path::Path;
 
+use crate::walk::filter_not_rust;
+
 const COMMENT: &str = "//";
 const LLVM_COMPONENTS_HEADER: &str = "needs-llvm-components:";
 const COMPILE_FLAGS_HEADER: &str = "compile-flags:";
@@ -35,61 +37,57 @@ struct RevisionInfo<'a> {
 }
 
 pub fn check(path: &Path, bad: &mut bool) {
-    crate::walk::walk(
-        path,
-        |path| path.extension().map(|p| p == "rs") == Some(false),
-        &mut |entry, content| {
-            let file = entry.path().display();
-            let mut header_map = BTreeMap::new();
-            iter_header(content, &mut |cfg, directive| {
-                if let Some(value) = directive.strip_prefix(LLVM_COMPONENTS_HEADER) {
-                    let info = header_map.entry(cfg).or_insert(RevisionInfo::default());
-                    let comp_vec = info.llvm_components.get_or_insert(Vec::new());
-                    for component in value.split(' ') {
-                        let component = component.trim();
-                        if !component.is_empty() {
-                            comp_vec.push(component);
-                        }
-                    }
-                } else if directive.starts_with(COMPILE_FLAGS_HEADER) {
-                    let compile_flags = &directive[COMPILE_FLAGS_HEADER.len()..];
-                    if let Some((_, v)) = compile_flags.split_once("--target") {
-                        if let Some((arch, _)) =
-                            v.trim_start_matches(|c| c == ' ' || c == '=').split_once("-")
-                        {
-                            let info = header_map.entry(cfg).or_insert(RevisionInfo::default());
-                            info.target_arch.replace(arch);
-                        } else {
-                            eprintln!("{file}: seems to have a malformed --target value");
-                            *bad = true;
-                        }
+    crate::walk::walk(path, filter_not_rust, &mut |entry, content| {
+        let file = entry.path().display();
+        let mut header_map = BTreeMap::new();
+        iter_header(content, &mut |cfg, directive| {
+            if let Some(value) = directive.strip_prefix(LLVM_COMPONENTS_HEADER) {
+                let info = header_map.entry(cfg).or_insert(RevisionInfo::default());
+                let comp_vec = info.llvm_components.get_or_insert(Vec::new());
+                for component in value.split(' ') {
+                    let component = component.trim();
+                    if !component.is_empty() {
+                        comp_vec.push(component);
                     }
                 }
-            });
-            for (rev, RevisionInfo { target_arch, llvm_components }) in &header_map {
-                let rev = rev.unwrap_or("[unspecified]");
-                match (target_arch, llvm_components) {
-                    (None, None) => {}
-                    (Some(_), None) => {
-                        eprintln!(
-                            "{}: revision {} should specify `{}` as it has `--target` set",
-                            file, rev, LLVM_COMPONENTS_HEADER
-                        );
+            } else if directive.starts_with(COMPILE_FLAGS_HEADER) {
+                let compile_flags = &directive[COMPILE_FLAGS_HEADER.len()..];
+                if let Some((_, v)) = compile_flags.split_once("--target") {
+                    if let Some((arch, _)) =
+                        v.trim_start_matches(|c| c == ' ' || c == '=').split_once("-")
+                    {
+                        let info = header_map.entry(cfg).or_insert(RevisionInfo::default());
+                        info.target_arch.replace(arch);
+                    } else {
+                        eprintln!("{file}: seems to have a malformed --target value");
                         *bad = true;
                     }
-                    (None, Some(_)) => {
-                        eprintln!(
-                            "{}: revision {} should not specify `{}` as it doesn't need `--target`",
-                            file, rev, LLVM_COMPONENTS_HEADER
-                        );
-                        *bad = true;
-                    }
-                    (Some(_), Some(_)) => {
-                        // FIXME: check specified components against the target architectures we
-                        // gathered.
-                    }
                 }
             }
-        },
-    );
+        });
+        for (rev, RevisionInfo { target_arch, llvm_components }) in &header_map {
+            let rev = rev.unwrap_or("[unspecified]");
+            match (target_arch, llvm_components) {
+                (None, None) => {}
+                (Some(_), None) => {
+                    eprintln!(
+                        "{}: revision {} should specify `{}` as it has `--target` set",
+                        file, rev, LLVM_COMPONENTS_HEADER
+                    );
+                    *bad = true;
+                }
+                (None, Some(_)) => {
+                    eprintln!(
+                        "{}: revision {} should not specify `{}` as it doesn't need `--target`",
+                        file, rev, LLVM_COMPONENTS_HEADER
+                    );
+                    *bad = true;
+                }
+                (Some(_), Some(_)) => {
+                    // FIXME: check specified components against the target architectures we
+                    // gathered.
+                }
+            }
+        }
+    });
 }
diff --git a/src/tools/tidy/src/ui_tests.rs b/src/tools/tidy/src/ui_tests.rs
index 15c36923e88..66f5c932be2 100644
--- a/src/tools/tidy/src/ui_tests.rs
+++ b/src/tools/tidy/src/ui_tests.rs
@@ -3,87 +3,83 @@
 //! - there are no stray `.stderr` files
 
 use ignore::Walk;
-use ignore::WalkBuilder;
+use std::collections::HashMap;
 use std::fs;
-use std::path::Path;
+use std::path::{Path, PathBuf};
 
 const ENTRY_LIMIT: usize = 1000;
 // FIXME: The following limits should be reduced eventually.
 const ROOT_ENTRY_LIMIT: usize = 940;
 const ISSUES_ENTRY_LIMIT: usize = 1978;
 
-fn check_entries(path: &Path, bad: &mut bool) {
-    for dir in Walk::new(&path.join("ui")) {
+fn check_entries(tests_path: &Path, bad: &mut bool) {
+    let mut directories: HashMap<PathBuf, usize> = HashMap::new();
+
+    for dir in Walk::new(&tests_path.join("ui")) {
         if let Ok(entry) = dir {
-            if entry.file_type().map(|ft| ft.is_dir()).unwrap_or(false) {
-                let dir_path = entry.path();
-                // Use special values for these dirs.
-                let is_root = path.join("ui") == dir_path;
-                let is_issues_dir = path.join("ui/issues") == dir_path;
-                let limit = if is_root {
-                    ROOT_ENTRY_LIMIT
-                } else if is_issues_dir {
-                    ISSUES_ENTRY_LIMIT
-                } else {
-                    ENTRY_LIMIT
-                };
+            let parent = entry.path().parent().unwrap().to_path_buf();
+            *directories.entry(parent).or_default() += 1;
+        }
+    }
 
-                let count = WalkBuilder::new(&dir_path)
-                    .max_depth(Some(1))
-                    .build()
-                    .into_iter()
-                    .collect::<Vec<_>>()
-                    .len()
-                    - 1; // remove the dir itself
+    for (dir_path, count) in directories {
+        // Use special values for these dirs.
+        let is_root = tests_path.join("ui") == dir_path;
+        let is_issues_dir = tests_path.join("ui/issues") == dir_path;
+        let limit = if is_root {
+            ROOT_ENTRY_LIMIT
+        } else if is_issues_dir {
+            ISSUES_ENTRY_LIMIT
+        } else {
+            ENTRY_LIMIT
+        };
 
-                if count > limit {
-                    tidy_error!(
-                        bad,
-                        "following path contains more than {} entries, \
-                            you should move the test to some relevant subdirectory (current: {}): {}",
-                        limit,
-                        count,
-                        dir_path.display()
-                    );
-                }
-            }
+        if count > limit {
+            tidy_error!(
+                bad,
+                "following path contains more than {} entries, \
+                    you should move the test to some relevant subdirectory (current: {}): {}",
+                limit,
+                count,
+                dir_path.display()
+            );
         }
     }
 }
 
 pub fn check(path: &Path, bad: &mut bool) {
     check_entries(&path, bad);
-    for path in &[&path.join("ui"), &path.join("ui-fulldeps")] {
-        crate::walk::walk_no_read(path, |_| false, &mut |entry| {
-            let file_path = entry.path();
-            if let Some(ext) = file_path.extension() {
-                if ext == "stderr" || ext == "stdout" {
-                    // Test output filenames have one of the formats:
-                    // ```
-                    // $testname.stderr
-                    // $testname.$mode.stderr
-                    // $testname.$revision.stderr
-                    // $testname.$revision.$mode.stderr
-                    // ```
-                    //
-                    // For now, just make sure that there is a corresponding
-                    // `$testname.rs` file.
-                    //
-                    // NB: We do not use file_stem() as some file names have multiple `.`s and we
-                    // must strip all of them.
-                    let testname =
-                        file_path.file_name().unwrap().to_str().unwrap().split_once('.').unwrap().0;
-                    if !file_path.with_file_name(testname).with_extension("rs").exists() {
-                        tidy_error!(bad, "Stray file with UI testing output: {:?}", file_path);
-                    }
+    let (ui, ui_fulldeps) = (path.join("ui"), path.join("ui-fulldeps"));
+    let paths = [ui.as_path(), ui_fulldeps.as_path()];
+    crate::walk::walk_no_read(&paths, |_| false, &mut |entry| {
+        let file_path = entry.path();
+        if let Some(ext) = file_path.extension() {
+            if ext == "stderr" || ext == "stdout" {
+                // Test output filenames have one of the formats:
+                // ```
+                // $testname.stderr
+                // $testname.$mode.stderr
+                // $testname.$revision.stderr
+                // $testname.$revision.$mode.stderr
+                // ```
+                //
+                // For now, just make sure that there is a corresponding
+                // `$testname.rs` file.
+                //
+                // NB: We do not use file_stem() as some file names have multiple `.`s and we
+                // must strip all of them.
+                let testname =
+                    file_path.file_name().unwrap().to_str().unwrap().split_once('.').unwrap().0;
+                if !file_path.with_file_name(testname).with_extension("rs").exists() {
+                    tidy_error!(bad, "Stray file with UI testing output: {:?}", file_path);
+                }
 
-                    if let Ok(metadata) = fs::metadata(file_path) {
-                        if metadata.len() == 0 {
-                            tidy_error!(bad, "Empty file with UI testing output: {:?}", file_path);
-                        }
+                if let Ok(metadata) = fs::metadata(file_path) {
+                    if metadata.len() == 0 {
+                        tidy_error!(bad, "Empty file with UI testing output: {:?}", file_path);
                     }
                 }
             }
-        });
-    }
+        }
+    });
 }
diff --git a/src/tools/tidy/src/walk.rs b/src/tools/tidy/src/walk.rs
index 94152e75168..2ade22c209f 100644
--- a/src/tools/tidy/src/walk.rs
+++ b/src/tools/tidy/src/walk.rs
@@ -1,6 +1,6 @@
 use ignore::DirEntry;
 
-use std::{fs::File, io::Read, path::Path};
+use std::{ffi::OsStr, fs::File, io::Read, path::Path};
 
 /// The default directory filter.
 pub fn filter_dirs(path: &Path) -> bool {
@@ -33,23 +33,26 @@ pub fn filter_dirs(path: &Path) -> bool {
     skip.iter().any(|p| path.ends_with(p))
 }
 
-pub fn walk_many(
-    paths: &[&Path],
+/// Filter for only files that end in `.rs`.
+pub fn filter_not_rust(path: &Path) -> bool {
+    path.extension() != Some(OsStr::new("rs")) && !path.is_dir()
+}
+
+pub fn walk(
+    path: &Path,
     skip: impl Clone + Send + Sync + 'static + Fn(&Path) -> bool,
     f: &mut dyn FnMut(&DirEntry, &str),
 ) {
-    for path in paths {
-        walk(path, skip.clone(), f);
-    }
+    walk_many(&[path], skip, f);
 }
 
-pub fn walk(
-    path: &Path,
-    skip: impl Send + Sync + 'static + Fn(&Path) -> bool,
+pub fn walk_many(
+    paths: &[&Path],
+    skip: impl Clone + Send + Sync + 'static + Fn(&Path) -> bool,
     f: &mut dyn FnMut(&DirEntry, &str),
 ) {
     let mut contents = Vec::new();
-    walk_no_read(path, skip, &mut |entry| {
+    walk_no_read(paths, skip, &mut |entry| {
         contents.clear();
         let mut file = t!(File::open(entry.path()), entry.path());
         t!(file.read_to_end(&mut contents), entry.path());
@@ -62,11 +65,14 @@ pub fn walk(
 }
 
 pub(crate) fn walk_no_read(
-    path: &Path,
+    paths: &[&Path],
     skip: impl Send + Sync + 'static + Fn(&Path) -> bool,
     f: &mut dyn FnMut(&DirEntry),
 ) {
-    let mut walker = ignore::WalkBuilder::new(path);
+    let mut walker = ignore::WalkBuilder::new(paths[0]);
+    for path in &paths[1..] {
+        walker.add(path);
+    }
     let walker = walker.filter_entry(move |e| !skip(e.path()));
     for entry in walker.build() {
         if let Ok(entry) = entry {