Add fast-path in wildcard search#7515
Conversation
dotnet-policy-service
Bot
added
the
Community
| // on some virtual file systems (e.g. VirtioFS). Fall back to enumeration when not found. | ||
| if (!searchDirectory && !IsWildcardSearch(searchPath) && File.Exists(fullSearchPath)) | ||
| { | ||
| return [new SearchPathResult(fullSearchPath.Replace(Path.AltDirectorySeparatorChar, Path.DirectorySeparatorChar), isFile: true)]; |
There was a problem hiding this comment.
There's a subtle different here in that the casing of the result can be different, but idk if that matters.
There was a problem hiding this comment.
On case-insensitive filesystem, it will succeed File.Exists check and return the input casing. On case-sensitive filesystem it will fail File.Exists and continue to the fallback (existing) code.
There was a problem hiding this comment.
yes, my point was on case insensitive, it'll return the path with the casing provided in the method call, which may not match the true casing on disk and the previous implementation did just that.
There was a problem hiding this comment.
I agree that it has no functional bearing, but just curious does it really resolve the path with actual disk casing? This test fails if we try to resolve the full path before returning from this method
.There was a problem hiding this comment.
Pull request overview
Adds a fast-path to PathResolver.PerformWildcardSearch to avoid directory enumeration when the input path is already fully resolved (no wildcards), improving performance and reducing susceptibility to intermittent failures on some virtual file systems.
Changes:
- Compute a
fullSearchPathonce and reuse it for regex creation. - Add a direct
File.Existsprobe for non-directory, non-wildcard searches to avoidDirectory.GetFilesenumeration in the common “exact file” case.
| // A search path with no wildcards refers to a single file. Probe it directly instead of | ||
| // enumerating the whole directory, which is the dominant cost and can fail intermittently | ||
| // on some virtual file systems (e.g. VirtioFS). Fall back to enumeration when not found. | ||
| if (!searchDirectory && !IsWildcardSearch(searchPath) && File.Exists(fullSearchPath)) |
There was a problem hiding this comment.
It seems like an intentional decision going by line 151 below and this test case
Bug
Fixes: NuGet/Home#13572
Description
Before: when all paths are pre-resolved (no wildcards), each AddFiles still enumerated the file's entire containing directory of size M via Directory.GetFiles, so resolving N files cost O(N*M) inode lookups (the dominant file I/O, and the frame that intermittently fails on VirtioFS).
After: each pre-resolved path is confirmed with a single File.Exists stat, O(N) total with one inode touched per file, falling back to enumeration only when the exact path isn't found.
PR Checklist
Added tests(it's a performance improvement without changing the usage semantic)