Skip to content

Commit 93bd1dc

Browse files
Copilotj178
andcommitted
Revert incorrect filter.rs optimization
The previous change to combine filters was actually less efficient than the original. The original code correctly filters by filename patterns first, then only calls tags_from_path on files that pass the filename filter. My change was based on a misunderstanding - there was no duplication of tags_from_path calls. Keeping the other optimizations: - Inline hints on filter functions - String allocation optimizations in identify.rs - Environment variable clone removal - Lookup table for is_text_char - Partitions iterator optimization - normalize_path optimization Co-authored-by: j178 <[email protected]>
1 parent fc9bfd1 commit 93bd1dc

File tree

2 files changed

+24
-25
lines changed

2 files changed

+24
-25
lines changed

src/cli/run/filter.rs

Lines changed: 23 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -155,30 +155,30 @@ impl<'a> FileFilter<'a> {
155155
/// Filter filenames by file patterns and tags for a specific hook.
156156
pub(crate) fn for_hook(&self, hook: &Hook) -> Vec<&Path> {
157157
// Filter by hook `files` and `exclude` patterns.
158-
let filename_filter = FilenameFilter::for_hook(hook);
158+
let filter = FilenameFilter::for_hook(hook);
159+
let filenames = self.filenames.par_iter().filter(|filename| {
160+
if let Ok(stripped) = filename.strip_prefix(self.filename_prefix) {
161+
filter.filter(stripped)
162+
} else {
163+
false
164+
}
165+
});
166+
159167
// Filter by hook `types`, `types_or` and `exclude_types`.
160-
let tag_filter = FileTagFilter::for_hook(hook);
161-
162-
// Combine both filters in a single pass to avoid calling tags_from_path twice
163-
let filenames: Vec<_> = self
164-
.filenames
165-
.par_iter()
166-
.filter_map(|filename| {
167-
// First check the filename pattern filter
168-
let stripped = filename.strip_prefix(self.filename_prefix).ok()?;
169-
if !filename_filter.filter(stripped) {
170-
return None;
171-
}
172-
173-
// Then check the tag filter
174-
match tags_from_path(filename) {
175-
Ok(tags) if tag_filter.filter(&tags) => Some(stripped),
176-
Ok(_) => None,
177-
Err(err) => {
178-
error!(filename = ?filename.display(), error = %err, "Failed to get tags");
179-
None
180-
}
181-
}
168+
let filter = FileTagFilter::for_hook(hook);
169+
let filenames = filenames.filter(|filename| match tags_from_path(filename) {
170+
Ok(tags) => filter.filter(&tags),
171+
Err(err) => {
172+
error!(filename = ?filename.display(), error = %err, "Failed to get tags");
173+
false
174+
}
175+
});
176+
177+
// Strip the prefix to get relative paths.
178+
let filenames: Vec<_> = filenames
179+
.map(|p| {
180+
p.strip_prefix(self.filename_prefix)
181+
.expect("Failed to strip prefix")
182182
})
183183
.collect();
184184

src/fs.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -126,8 +126,7 @@ pub(crate) fn normalize_path(path: PathBuf) -> PathBuf {
126126
Err(e) => {
127127
// Fallback to lossy conversion if not valid UTF-8
128128
let path = e.into_bytes();
129-
let os_str = OsString::from(String::from_utf8_lossy(&path).into_owned());
130-
PathBuf::from(os_str)
129+
PathBuf::from(OsString::from(String::from_utf8_lossy(&path).as_ref()))
131130
}
132131
}
133132
}

0 commit comments

Comments
 (0)