Rollup merge of #144511 - lolbinarycat:tidy-extra-checks-opt, r=Kobzol

tidy: increase performance of auto extra checks feature

Removes the repeated calls to git diff.

Halves the overhead of the tidy extra checks feature from 0.1 seconds to 0.05 on my machine, but probably will be more significant on systems on slow disks or less memory for i/o cache.

r? ``@Kobzol``

Author: Zalathar

src/tools/tidy/src/extra_checks/mod.rs

@@ -86,7 +86,7 @@ fn check_impl(
         std::env::var("TIDY_PRINT_DIFF").is_ok_and(|v| v.eq_ignore_ascii_case("true") || v == "1");
 
     // Split comma-separated args up
-    let lint_args = match extra_checks {
+    let mut lint_args = match extra_checks {
         Some(s) => s
             .strip_prefix("--extra-checks=")
             .unwrap()
@@ -99,11 +99,7 @@ fn check_impl(
             })
             .filter_map(|(res, src)| match res {
                 Ok(arg) => {
-                    if arg.is_inactive_auto(ci_info) {
-                        None
-                    } else {
-                        Some(arg)
-                    }
+                    Some(arg)
                 }
                 Err(err) => {
                     // only warn because before bad extra checks would be silently ignored.
@@ -114,6 +110,11 @@ fn check_impl(
             .collect(),
         None => vec![],
     };
+    if lint_args.iter().any(|ck| ck.auto) {
+        crate::files_modified_batch_filter(ci_info, &mut lint_args, |ck, path| {
+            ck.is_non_auto_or_matches(path)
+        });
+    }
 
     macro_rules! extra_check {
         ($lang:ident, $kind:ident) => {
@@ -721,23 +722,26 @@ impl ExtraCheckArg {
         self.lang == lang && self.kind.map(|k| k == kind).unwrap_or(true)
     }
 
-    /// Returns `true` if this is an auto arg and the relevant files are not modified.
-    fn is_inactive_auto(&self, ci_info: &CiInfo) -> bool {
+    /// Returns `false` if this is an auto arg and the passed filename does not trigger the auto rule
+    fn is_non_auto_or_matches(&self, filepath: &str) -> bool {
         if !self.auto {
-            return false;
+            return true;
         }
         let ext = match self.lang {
             ExtraCheckLang::Py => ".py",
             ExtraCheckLang::Cpp => ".cpp",
             ExtraCheckLang::Shell => ".sh",
             ExtraCheckLang::Js => ".js",
             ExtraCheckLang::Spellcheck => {
-                return !crate::files_modified(ci_info, |s| {
-                    SPELLCHECK_DIRS.iter().any(|dir| Path::new(s).starts_with(dir))
-                });
+                for dir in SPELLCHECK_DIRS {
+                    if Path::new(filepath).starts_with(dir) {
+                        return true;
+                    }
+                }
+                return false;
             }
         };
-        !crate::files_modified(ci_info, |s| s.ends_with(ext))
+        filepath.ends_with(ext)
     }
 
     fn has_supported_kind(&self) -> bool {

src/tools/tidy/src/lib.rs

@@ -125,40 +125,61 @@ pub fn git_diff<S: AsRef<OsStr>>(base_commit: &str, extra_arg: S) -> Option<Stri
     Some(String::from_utf8_lossy(&output.stdout).into())
 }
 
-/// Returns true if any modified file matches the predicate, if we are in CI, or if unable to list modified files.
-pub fn files_modified(ci_info: &CiInfo, pred: impl Fn(&str) -> bool) -> bool {
+/// Similar to `files_modified`, but only involves a single call to `git`.
+///
+/// removes all elements from `items` that do not cause any match when `pred` is called with the list of modifed files.
+///
+/// if in CI, no elements will be removed.
+pub fn files_modified_batch_filter<T>(
+    ci_info: &CiInfo,
+    items: &mut Vec<T>,
+    pred: impl Fn(&T, &str) -> bool,
+) {
     if CiEnv::is_ci() {
         // assume everything is modified on CI because we really don't want false positives there.
-        return true;
+        return;
     }
     let Some(base_commit) = &ci_info.base_commit else {
         eprintln!("No base commit, assuming all files are modified");
-        return true;
+        return;
     };
     match crate::git_diff(base_commit, "--name-status") {
         Some(output) => {
-            let modified_files = output.lines().filter_map(|ln| {
-                let (status, name) = ln
-                    .trim_end()
-                    .split_once('\t')
-                    .expect("bad format from `git diff --name-status`");
-                if status == "M" { Some(name) } else { None }
-            });
-            for modified_file in modified_files {
-                if pred(modified_file) {
-                    return true;
+            let modified_files: Vec<_> = output
+                .lines()
+                .filter_map(|ln| {
+                    let (status, name) = ln
+                        .trim_end()
+                        .split_once('\t')
+                        .expect("bad format from `git diff --name-status`");
+                    if status == "M" { Some(name) } else { None }
+                })
+                .collect();
+            items.retain(|item| {
+                for modified_file in &modified_files {
+                    if pred(item, modified_file) {
+                        // at least one predicate matches, keep this item.
+                        return true;
+                    }
                 }
-            }
-            false
+                // no predicates matched, remove this item.
+                false
+            });
         }
         None => {
             eprintln!("warning: failed to run `git diff` to check for changes");
             eprintln!("warning: assuming all files are modified");
-            true
         }
     }
 }
 
+/// Returns true if any modified file matches the predicate, if we are in CI, or if unable to list modified files.
+pub fn files_modified(ci_info: &CiInfo, pred: impl Fn(&str) -> bool) -> bool {
+    let mut v = vec![()];
+    files_modified_batch_filter(ci_info, &mut v, |_, p| pred(p));
+    !v.is_empty()
+}
+
 pub mod alphabetical;
 pub mod bins;
 pub mod debug_artifacts;