From c76860f678f9b2886bc658a38758b60752a37b6d Mon Sep 17 00:00:00 2001 From: Perry Hertler Date: Mon, 11 Aug 2025 11:07:50 -0500 Subject: [PATCH] validate performance optimizations --- Cargo.lock | 15 ++++++++++++--- Cargo.toml | 4 ++-- src/project.rs | 36 +++++++++++++++++++++++++++++++++--- src/project_builder.rs | 34 ++++++++++++++++++++++++++-------- 4 files changed, 73 insertions(+), 16 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d9cf231..f97d18a 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -77,6 +77,12 @@ version = "1.0.83" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "25bdb32cbbdce2b519a9cd7df3a678443100e265d5e25ca763b7572a5104f5f3" +[[package]] +name = "arrayvec" +version = "0.7.6" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "7c02d123df017efcdfbd739ef81735b36c5ba83ec3c59c80a9d7ecc718f92e50" + [[package]] name = "assert_cmd" version = "2.0.16" @@ -173,7 +179,7 @@ checksum = "1462739cb27611015575c0c11df5df7601141071f07518d56fcc1be504cbec97" [[package]] name = "codeowners" -version = "0.2.4" +version = "0.2.5" dependencies = [ "assert_cmd", "clap", @@ -314,9 +320,12 @@ dependencies = [ [[package]] name = "fast-glob" -version = "0.4.0" +version = "1.0.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "bb10ed0f8a3dca52477be37ac0fb8f9d1fd4cd8d311b4484bdd45c1c56e0c9ec" +checksum = "3d26eec0ae9682c457cb0f85de67ad417b716ae852736a5d94c2ad6e92a997c9" +dependencies = [ + "arrayvec", +] [[package]] name = "fastrand" diff --git a/Cargo.toml b/Cargo.toml index df37c08..f61c3da 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "codeowners" -version = "0.2.4" +version = "0.2.5" edition = "2024" [profile.release] @@ -14,7 +14,7 @@ clap = { version = "4.5.20", features = ["derive"] } clap_derive = "4.5.18" error-stack = "0.5.0" enum_dispatch = "0.3.13" -fast-glob = "0.4.0" +fast-glob = "1.0.0" glob = "0.3.2" ignore = "0.4.23" itertools = "0.14.0" diff --git a/src/project.rs b/src/project.rs index e405e90..5179371 100644 --- a/src/project.rs +++ b/src/project.rs @@ -178,9 +178,7 @@ impl Project { } pub fn relative_path<'a>(&'a self, absolute_path: &'a Path) -> &'a Path { - absolute_path - .strip_prefix(&self.base_path) - .expect("Could not generate relative path") + absolute_path.strip_prefix(&self.base_path).unwrap_or(absolute_path) } pub fn get_team(&self, name: &str) -> Option { @@ -197,3 +195,35 @@ impl Project { result } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn test_vendored_gem_by_name_maps_all_gems() { + let vg1 = VendoredGem { + path: PathBuf::from("vendored/a"), + name: "a".to_string(), + }; + let vg2 = VendoredGem { + path: PathBuf::from("vendored/b"), + name: "b".to_string(), + }; + let project = Project { + base_path: PathBuf::from("."), + files: vec![], + packages: vec![], + vendored_gems: vec![vg1.clone(), vg2.clone()], + teams: vec![], + codeowners_file_path: PathBuf::from(".github/CODEOWNERS"), + directory_codeowner_files: vec![], + teams_by_name: HashMap::new(), + }; + + let map = project.vendored_gem_by_name(); + assert_eq!(map.len(), 2); + assert_eq!(map.get("a").unwrap().name, vg1.name); + assert_eq!(map.get("b").unwrap().name, vg2.name); + } +} diff --git a/src/project_builder.rs b/src/project_builder.rs index 54f3ac3..c0bb1ad 100644 --- a/src/project_builder.rs +++ b/src/project_builder.rs @@ -1,11 +1,12 @@ use std::{ fs::File, path::{Path, PathBuf}, + sync::{Arc, Mutex}, }; use error_stack::{Result, ResultExt}; use fast_glob::glob_match; -use ignore::WalkBuilder; +use ignore::{WalkBuilder, WalkParallel, WalkState}; use rayon::iter::{IntoParallelIterator, ParallelIterator}; use tracing::{instrument, warn}; @@ -53,12 +54,29 @@ impl<'a> ProjectBuilder<'a> { let mut entry_types = Vec::with_capacity(INITIAL_VECTOR_CAPACITY); let mut builder = WalkBuilder::new(&self.base_path); builder.hidden(false); - let walkdir = builder.build(); + let walk_parallel: WalkParallel = builder.build_parallel(); - for entry in walkdir { - let entry = entry.change_context(Error::Io)?; + let collected = Arc::new(Mutex::new(Vec::with_capacity(INITIAL_VECTOR_CAPACITY))); + let collected_for_threads = Arc::clone(&collected); + + walk_parallel.run(move || { + let collected = Arc::clone(&collected_for_threads); + Box::new(move |res| { + if let Ok(entry) = res { + if let Ok(mut v) = collected.lock() { + v.push(entry); + } + } + WalkState::Continue + }) + }); + + // Process sequentially with &mut self + let collected_entries = Arc::try_unwrap(collected).unwrap().into_inner().unwrap(); + for entry in collected_entries { entry_types.push(self.build_entry_type(entry)?); } + self.build_project_from_entry_types(entry_types) } @@ -216,7 +234,10 @@ impl<'a> ProjectBuilder<'a> { } fn matches_globs(path: &Path, globs: &[String]) -> bool { - globs.iter().any(|glob| glob_match(glob, path.to_str().unwrap())) + match path.to_str() { + Some(s) => globs.iter().any(|glob| glob_match(glob, s)), + None => false, + } } fn ruby_package_owner(path: &Path) -> Result, Error> { @@ -241,14 +262,11 @@ mod tests { #[test] fn test_matches_globs() { - // should fail because hidden directories are ignored by glob patterns unless explicitly included assert!(matches_globs(Path::new("script/.eslintrc.js"), &[OWNED_GLOB.to_string()])); } #[test] fn test_glob_match() { - // Exposes bug in glob-match https://github.com/devongovett/glob-match/issues/9 - // should fail because hidden directories are ignored by glob patterns unless explicitly included assert!(glob_match(OWNED_GLOB, "script/.eslintrc.js")); } }