Skip to content

Commit 0247d6e

Browse files
committed
perf: optimize cache building and fix path normalization
## Performance Improvements ### Optimize owner/tag collection (cache.rs) - Changed from O(owners × files) to O(files) complexity - Build owners_map and tags_map in single pass through file_entries - Removed unused collect_owners and collect_tags functions from common.rs ### Fix lifetime warnings (smart_iter.rs) - Added explicit lifetime `'_` to trait and impl return types - Fixes compiler warnings about elided lifetimes ## Bug Fixes ### Fix path normalization in inspect command - Handle ./ prefix variations when looking up files in cache - Normalize both cached paths and input paths for comparison - Updated test to properly assert success with ownership verification ## Code Cleanup - Removed unused functions: collect_owners, collect_tags - Removed unused imports: Owner, Tag from common.rs
1 parent c104304 commit 0247d6e

File tree

5 files changed

+32
-55
lines changed

5 files changed

+32
-55
lines changed

ci/tests/test_cli.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -293,16 +293,16 @@ fn test_codeowners_inspect_command() {
293293
.assert()
294294
.success();
295295

296-
// Inspect command should at least accept the arguments
297-
// Note: The inspect command has path normalization issues that need to be fixed
298-
// For now, verify the command runs (even if it reports file not found)
296+
// Inspect a specific file - verify ownership info is returned
299297
ci().arg("codeowners")
300298
.arg("inspect")
301-
.arg("./src/main.rs")
299+
.arg("src/main.rs")
302300
.arg("--repo")
303301
.arg(".")
304302
.current_dir(temp_dir.path())
305-
.assert(); // Don't assert success - path normalization needs fixing
303+
.assert()
304+
.success()
305+
.stdout(predicate::str::contains("@rust-team"));
306306
}
307307

308308
#[test]

codeinput/src/core/cache.rs

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
use crate::{
22
core::{
3-
common::{collect_owners, collect_tags, get_repo_hash},
3+
common::get_repo_hash,
44
parse::parse_repo,
55
resolver::find_owners_and_tags_for_file,
66
types::{
@@ -93,27 +93,21 @@ pub fn build_cache(
9393
// Print newline after processing is complete
9494
println!("\r\x1b[K✅ Processed {} files successfully", total_files);
9595

96-
// Process each owner
97-
let owners = collect_owners(&entries);
98-
owners.iter().for_each(|owner| {
99-
let paths = owners_map.entry(owner.clone()).or_insert_with(Vec::new);
100-
for file_entry in &file_entries {
101-
if file_entry.owners.contains(owner) {
102-
paths.push(file_entry.path.clone());
103-
}
96+
// Build owner and tag maps in a single pass through file_entries - O(files) instead of O(owners × files)
97+
for file_entry in &file_entries {
98+
for owner in &file_entry.owners {
99+
owners_map
100+
.entry(owner.clone())
101+
.or_insert_with(Vec::new)
102+
.push(file_entry.path.clone());
104103
}
105-
});
106-
107-
// Process each tag
108-
let tags = collect_tags(&entries);
109-
tags.iter().for_each(|tag| {
110-
let paths = tags_map.entry(tag.clone()).or_insert_with(Vec::new);
111-
for file_entry in &file_entries {
112-
if file_entry.tags.contains(tag) {
113-
paths.push(file_entry.path.clone());
114-
}
104+
for tag in &file_entry.tags {
105+
tags_map
106+
.entry(tag.clone())
107+
.or_insert_with(Vec::new)
108+
.push(file_entry.path.clone());
115109
}
116-
});
110+
}
117111

118112
Ok(CodeownersCache {
119113
hash,

codeinput/src/core/commands/inspect.rs

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,11 +34,19 @@ pub fn run(
3434
file_path.to_path_buf()
3535
};
3636

37-
// Find the file in the cache
37+
// Create normalized path for matching (handle ./ prefix variations)
38+
let path_str = normalized_file_path.to_string_lossy();
39+
let path_without_dot = path_str.strip_prefix("./").unwrap_or(&path_str);
40+
41+
// Find the file in the cache (try both with and without ./ prefix)
3842
let file_entry = cache
3943
.files
4044
.iter()
41-
.find(|file| file.path == normalized_file_path)
45+
.find(|file| {
46+
let cache_path = file.path.to_string_lossy();
47+
let cache_path_normalized = cache_path.strip_prefix("./").unwrap_or(&cache_path);
48+
cache_path_normalized == path_without_dot
49+
})
4250
.ok_or_else(|| {
4351
Error::new(&format!(
4452
"File {} not found in cache",

codeinput/src/core/common.rs

Lines changed: 1 addition & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use ignore::Walk;
44
use sha2::{Digest, Sha256};
55
use std::path::{Path, PathBuf};
66

7-
use super::types::{CodeownersEntry, Owner, Tag};
7+
use super::types::CodeownersEntry;
88

99
/// Maximum recursion depth for finding CODEOWNERS files (prevents stack overflow on deep structures)
1010
const MAX_RECURSION_DEPTH: usize = 100;
@@ -57,31 +57,6 @@ pub fn find_files<P: AsRef<Path>>(base_path: P) -> Result<Vec<PathBuf>> {
5757
Ok(result)
5858
}
5959

60-
/// Collect all unique owners from CODEOWNERS entries
61-
pub fn collect_owners(entries: &[CodeownersEntry]) -> Vec<Owner> {
62-
let mut owners = std::collections::HashSet::new();
63-
64-
for entry in entries {
65-
for owner in &entry.owners {
66-
owners.insert(owner.clone());
67-
}
68-
}
69-
70-
owners.into_iter().collect()
71-
}
72-
73-
/// Collect all unique tags from CODEOWNERS entries
74-
pub fn collect_tags(entries: &[CodeownersEntry]) -> Vec<Tag> {
75-
let mut tags = std::collections::HashSet::new();
76-
77-
for entry in entries {
78-
for tag in &entry.tags {
79-
tags.insert(tag.clone());
80-
}
81-
}
82-
83-
tags.into_iter().collect()
84-
}
8560

8661
/// Files to exclude from the repository hash calculation (these are generated files)
8762
const HASH_EXCLUDED_PATTERNS: &[&str] = &[

codeinput/src/core/smart_iter.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,11 +3,11 @@
33
use rayon::iter::{IntoParallelRefIterator, ParallelIterator};
44

55
pub(crate) trait SmartIter<T: Send + Sync> {
6-
fn smart_iter(&self, n: usize) -> SmartIterator<T>;
6+
fn smart_iter(&self, n: usize) -> SmartIterator<'_, T>;
77
}
88

99
impl<T: Send + Sync> SmartIter<T> for [T] {
10-
fn smart_iter(&self, n: usize) -> SmartIterator<T> {
10+
fn smart_iter(&self, n: usize) -> SmartIterator<'_, T> {
1111
if self.len() <= n {
1212
SmartIterator::Sequential(self.iter())
1313
} else {

0 commit comments

Comments
 (0)