refactor(walk): extract entry filtering and normalization logic

[parneetsingh022]

This PR refactors the directory traversal logic in WorkerState to improve readability and maintainability. Previously, the evaluation of filtering constraints (depth, patterns, extensions, size, time) was implemented as a large block of inline conditionals within the core walk loop.

This change extracts that logic into a dedicated EntryFilter struct, cleanly separating the traversal mechanics from the filtering rules. All existing traversal behavior and the execution order of the filter checks are strictly preserved.

Changes

This PR cleans up the walk entry processing so the main loop is easier to read and reason about.

• Moved file filtering logic into a new EntryFilter helper, with separate methods for pattern, size, owner, and other checks.

• Added normalize_walk_entry to handle ignore::DirEntry conversion, broken symlink recovery, and walker errors outside of the main loop.

• Moved Unix-specific owner-check logic into check_owner, keeping the main path less cluttered with platform-specific conditionals.

[tmccombs]

Have you done any benchmarking or comparison of assembly on the release build to see if this has any negative impact on performance?

I would hope that it the wrapping and unwrapping of options can mostly be optimized away, but I'm not sure, and since this is in the hot path, I am slightly worried about it.

[tmccombs]

I wonder if it would be worth using a macro like

macro_rules! entry_filter {
  ($e:expr) => {
    let result = $e;
    if result.is_some() {
      return result;
    }
  }
}

Then each of these checks becomes entry_filter!(self.check_min_depth(entry)).

Or alternatively, have this return Result<(), WalkState>, and then this becomes

self.check_min_depth(entry)?;
self.check_patterns(entry)?;
//...

[parneetsingh022]

Internal functions now return a bool, I guess this might be better implementation than using a macro (which would have been effective on previous implementation).

if self.is_below_min_depth(entry)
    || self.fails_pattern_filter(entry)
    || self.fails_extension_filter(entry)
    || self.matches_ignored_file_type(entry)
    || fails_owner_constraints
    || self.fails_size_constraints(entry)
    || self.fails_modification_time_constraints(entry)
{
    return Err(WalkState::Continue);
}

[tmccombs]

Most of these functions are actually just checking a condition, and return WalkState::Continue. I wonder if i instead of these returning Option, it woudl be simpler to just have them return bool, and do the return Some(WalkState::Continue)` in the evaluate method

[parneetsingh022]

I guess you are right. There is no point in returning Option<WalkState> on every function. I've refactored the code to return bool, and now only evaluate and evaluate_raw functions use Result<(), WalkState>.

[tmccombs]

I would hope this inline annotation isn't necessary

[tmccombs]

but it might be better to just not have this defined for non-unix, and have a #[cfg(unix)] on the call as well.

[parneetsingh022]

Thanks, that makes sense. I removed the extra non-Unix helper implementation.

Instead, the owner constraint check is now only called on Unix, and non-Unix builds just set fails_owner_constraints to false inside evaluate:

#[cfg(unix)]
let fails_owner_constraints = self.fails_owner_constraints(entry);

#[cfg(not(unix))]
let fails_owner_constraints = false;

[parneetsingh022]

Have you done any benchmarking or comparison of assembly on the release build to see if this has any negative impact on performance?

I would hope that it the wrapping and unwrapping of options can mostly be optimized away, but I'm not sure, and since this is in the hot path, I am slightly worried about it.

I’ve tried to address the concerns mentioned in the PR comments/review.

Regarding the performance concern: I benchmarked this PR against master using release builds and hyperfine.

Benchmark setup:

• OS: Windows
• Benchmark tool: hyperfine
• Baseline: master
• PR branch: this PR
• Benchmark directory: official Rust repository checkout

I used a local checkout of the official Rust repository as the benchmark directory because it is large enough to make traversal/filtering costs measurable and should give a more useful signal than benchmarking only on the fd repository itself.

I do not see any measurable performance impact from this PR. All benchmark results are within noise, with master and this PR performing effectively the same across the tested cases.

That matches my expectation, since this PR does not change the underlying logic. It is primarily a refactor that splits the existing logic into smaller functions to improve readability and maintainability.

I ran local benchmarks on Windows using hyperfine --warmup 10 --runs 50.

Benchmark	Master	PR	Result
Broad traversal	498.4 ms ± 27.9 ms	497.4 ms ± 30.2 ms	PR 1.00 ± 0.08× faster
Name pattern	479.6 ms ± 19.8 ms	481.6 ms ± 21.3 ms	Master 1.00 ± 0.06× faster
Extension filter	491.5 ms ± 18.0 ms	491.0 ms ± 21.9 ms	PR 1.00 ± 0.06× faster
Type filter	493.2 ms ± 17.2 ms	490.4 ms ± 17.3 ms	PR 1.01 ± 0.05× faster
Size filter	1.720 s ± 0.051 s	1.728 s ± 0.040 s	Master 1.00 ± 0.04× faster

Overall, the results look effectively within benchmark noise rather than showing a clear regression.