Skip to content

Commit 886804b

Browse files
authored
validate performance optimizations (#64)
1 parent 4473bf5 commit 886804b

File tree

4 files changed

+73
-16
lines changed

4 files changed

+73
-16
lines changed

Cargo.lock

Lines changed: 12 additions & 3 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "codeowners"
3-
version = "0.2.4"
3+
version = "0.2.5"
44
edition = "2024"
55

66
[profile.release]
@@ -14,7 +14,7 @@ clap = { version = "4.5.20", features = ["derive"] }
1414
clap_derive = "4.5.18"
1515
error-stack = "0.5.0"
1616
enum_dispatch = "0.3.13"
17-
fast-glob = "0.4.0"
17+
fast-glob = "1.0.0"
1818
glob = "0.3.2"
1919
ignore = "0.4.23"
2020
itertools = "0.14.0"

src/project.rs

Lines changed: 33 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -178,9 +178,7 @@ impl Project {
178178
}
179179

180180
pub fn relative_path<'a>(&'a self, absolute_path: &'a Path) -> &'a Path {
181-
absolute_path
182-
.strip_prefix(&self.base_path)
183-
.expect("Could not generate relative path")
181+
absolute_path.strip_prefix(&self.base_path).unwrap_or(absolute_path)
184182
}
185183

186184
pub fn get_team(&self, name: &str) -> Option<Team> {
@@ -197,3 +195,35 @@ impl Project {
197195
result
198196
}
199197
}
198+
199+
#[cfg(test)]
200+
mod tests {
201+
use super::*;
202+
203+
#[test]
204+
fn test_vendored_gem_by_name_maps_all_gems() {
205+
let vg1 = VendoredGem {
206+
path: PathBuf::from("vendored/a"),
207+
name: "a".to_string(),
208+
};
209+
let vg2 = VendoredGem {
210+
path: PathBuf::from("vendored/b"),
211+
name: "b".to_string(),
212+
};
213+
let project = Project {
214+
base_path: PathBuf::from("."),
215+
files: vec![],
216+
packages: vec![],
217+
vendored_gems: vec![vg1.clone(), vg2.clone()],
218+
teams: vec![],
219+
codeowners_file_path: PathBuf::from(".github/CODEOWNERS"),
220+
directory_codeowner_files: vec![],
221+
teams_by_name: HashMap::new(),
222+
};
223+
224+
let map = project.vendored_gem_by_name();
225+
assert_eq!(map.len(), 2);
226+
assert_eq!(map.get("a").unwrap().name, vg1.name);
227+
assert_eq!(map.get("b").unwrap().name, vg2.name);
228+
}
229+
}

src/project_builder.rs

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,12 @@
11
use std::{
22
fs::File,
33
path::{Path, PathBuf},
4+
sync::{Arc, Mutex},
45
};
56

67
use error_stack::{Result, ResultExt};
78
use fast_glob::glob_match;
8-
use ignore::WalkBuilder;
9+
use ignore::{WalkBuilder, WalkParallel, WalkState};
910
use rayon::iter::{IntoParallelIterator, ParallelIterator};
1011
use tracing::{instrument, warn};
1112

@@ -53,12 +54,29 @@ impl<'a> ProjectBuilder<'a> {
5354
let mut entry_types = Vec::with_capacity(INITIAL_VECTOR_CAPACITY);
5455
let mut builder = WalkBuilder::new(&self.base_path);
5556
builder.hidden(false);
56-
let walkdir = builder.build();
57+
let walk_parallel: WalkParallel = builder.build_parallel();
5758

58-
for entry in walkdir {
59-
let entry = entry.change_context(Error::Io)?;
59+
let collected = Arc::new(Mutex::new(Vec::with_capacity(INITIAL_VECTOR_CAPACITY)));
60+
let collected_for_threads = Arc::clone(&collected);
61+
62+
walk_parallel.run(move || {
63+
let collected = Arc::clone(&collected_for_threads);
64+
Box::new(move |res| {
65+
if let Ok(entry) = res {
66+
if let Ok(mut v) = collected.lock() {
67+
v.push(entry);
68+
}
69+
}
70+
WalkState::Continue
71+
})
72+
});
73+
74+
// Process sequentially with &mut self
75+
let collected_entries = Arc::try_unwrap(collected).unwrap().into_inner().unwrap();
76+
for entry in collected_entries {
6077
entry_types.push(self.build_entry_type(entry)?);
6178
}
79+
6280
self.build_project_from_entry_types(entry_types)
6381
}
6482

@@ -216,7 +234,10 @@ impl<'a> ProjectBuilder<'a> {
216234
}
217235

218236
fn matches_globs(path: &Path, globs: &[String]) -> bool {
219-
globs.iter().any(|glob| glob_match(glob, path.to_str().unwrap()))
237+
match path.to_str() {
238+
Some(s) => globs.iter().any(|glob| glob_match(glob, s)),
239+
None => false,
240+
}
220241
}
221242

222243
fn ruby_package_owner(path: &Path) -> Result<Option<String>, Error> {
@@ -241,14 +262,11 @@ mod tests {
241262

242263
#[test]
243264
fn test_matches_globs() {
244-
// should fail because hidden directories are ignored by glob patterns unless explicitly included
245265
assert!(matches_globs(Path::new("script/.eslintrc.js"), &[OWNED_GLOB.to_string()]));
246266
}
247267

248268
#[test]
249269
fn test_glob_match() {
250-
// Exposes bug in glob-match https://github.com/devongovett/glob-match/issues/9
251-
// should fail because hidden directories are ignored by glob patterns unless explicitly included
252270
assert!(glob_match(OWNED_GLOB, "script/.eslintrc.js"));
253271
}
254272
}

0 commit comments

Comments
 (0)