[JeremyKuhne]

Summary

FileMatcher.GetFiles now selects a legacy, optimized callback, or optimized direct driver before whole-glob cache lookup. The primary target is cache-backed SDK-style project evaluation: in the final same-binary benchmark matrix, complete isolated project evaluations ran 10-13% faster and allocated 16-34% less than the Wave-disabled baseline.

The optimized path is not applied universally. Cache-backed shapes that measured neutral or slower remain on the legacy driver, while compatibility-sensitive requests are routed conservatively to either legacy or callback traversal.

The matcher is adapted from Touki at 9d925032a5e7d100c9380a7fe40b9ef64527bcab. Existing MSBuild behavior and differential tests remain the compatibility oracle.

No public API surface or new warning/error diagnostic is introduced.

Fixes #12072.

Implementation

• Add specialized matching for common zero-, one-, and anchored-globstar shapes, with a bounded NFA-style state set for complex multi-globstar patterns.
• Use optimized callback traversal with the directory-entry cache supplied by EvaluationContext.
• Restore bounded top-level parallel traversal for the optimized callback path, using one result buffer per partition.
• Add direct FileSystemEnumerator<T> traversal for built-in filesystems that explicitly advertise set-equivalent direct enumeration.
• Filter direct-enumeration entries before materializing their output paths.
• Select one driver and disposition before whole-glob cache lookup; that selection controls dispatch, tracing, and cache identity.
• Partition whole-glob caches by project, include, excludes, driver profile, case-folding mode, and legacy culture.
• Apply the same Wave 18.11 culture policy to FileMatcher expansion and the MSBuildGlob descriptors used for Include, Exclude, and Remove matching.

Dialect semantics, matcher implementation, and enumeration driver remain separate internal concepts. This avoids adding a public dialect API while leaving room for Touki-aligned compiled specifications later.

Driver selection

The cache-backed selection rule is based on whether a wildcard exclude can overlap the include root, not simply whether excludes exist.

Cache-backed shape	Selection	Sampled time vs. legacy
No excludes	Legacy fallback	0.97x-1.02x
Fixed bin/**;obj/** excludes below a src include	Legacy fallback	0.98x-1.02x
Adds recursive **/bin/**;**/obj/** excludes	Optimized callback	0.80x-0.88x
Full ten-exclude SDK-style set	Optimized callback	0.78x-0.89x

Drive-wide and drive-relative specifications, trailing-directory specifications, ancestor excludes that cannot be safely rebased, relative excludes without a project root, custom callback delegates, and non-capable filesystems use legacy traversal.

Uncached relative includes without a project directory, literal excludes, and invalid wildcard excludes use optimized callback traversal rather than direct traversal. This preserves relative output and exact-identity filtering semantics.

Compatibility

Auto resolves to the optimized implementation only when Change Wave 18.11 is enabled. Setting MSBUILDDISABLEFEATURESFROMVERSION=18.11 restores the legacy FileMatcher implementation, along with opting out of the other features in that Change Wave.

Regex-backed filesystem globs now use culture-invariant case folding by default. MSBUILDUSELEGACYCULTURESENSITIVEFILEGLOBS=1 restores current-culture folding without disabling unrelated Wave 18.11 behavior. The escape hatch is read from the current environment so reused worker nodes honor refreshed build environments.

Compatibility coverage includes DOS wildcards, trailing-dot behavior, Linux casing and literal backslashes, rooted and lexical paths, invalid excludes, injected filesystems, symlinks, I/O failures, concurrent cache population, drive-wide disposition, and Include/Remove culture consistency.

Evaluation performance

Final Windows 11 .NET 11 ShortRun results for complete project evaluation, comparing the same binary with Wave 18.11 enabled and disabled:

Scenario	Time vs. legacy	Allocation vs. legacy
32 directories, 8 nonmatching files/directory	0.89x	0.73x
32 directories, 64 nonmatching files/directory	0.90x	0.84x
128 directories, 8 nonmatching files/directory	0.87x	0.66x
128 directories, 64 nonmatching files/directory	0.90x	0.84x

Every final-code isolated sample was faster and lower-allocation. A separate full-sampling checkpoint measured 0.80x-0.90x time and 0.66x-0.88x allocation.

A targeted full shared-context run measured 1.01x legacy time and 1.02x allocation. Shared contexts are expected to remain close to neutral because the whole-glob result is already cached.

The benchmark validates every sorted @(Compile) identity before measurement, not only item counts.

Validation

• PR build 20260812.16 passed Windows Core, Windows Full, Windows Full Release without bootstrap, Linux Core, Linux multithreaded, macOS Core, managed source-build, and code coverage.
• Post-rebase FileMatcherOptimized_Tests, Release net472/x86: 107 passed, 19 expected skips.
• Post-rebase FileMatcherOptimized_Tests, Debug net11.0: 108 passed, 19 expected skips.
• Complete Framework Release tests, Windows net11.0 and net472: 2,133 passed, 70 expected skips.
• Linux matcher and optimized FileMatcher tests: 161 passed, 5 expected skips.
• MSBuildGlob and evaluation-culture tests: 202 passed on Windows and 101 passed on Linux.
• CreateItem tests, net11.0 and net472: 46 passed, 4 expected skips.
• Strict netstandard2.0 source-build compile and benchmark build with warnings as errors: passed.
• Whole-repository Release build: passed with no warnings or errors.

[JeremyKuhne]

cc: @ViktorHofer, @baronfel, @marcpopMSFT, @jaredpar

[Copilot]

Pull request overview

This PR introduces a new optimized wildcard expansion/enumeration path behind FileMatcher.GetFiles, gated by ChangeWave 18.11 (Auto selects optimized unless disabled via MSBUILDDISABLEFEATURESFROMVERSION=18.11). The implementation keeps the legacy path available as a compatibility fallback and adds a new state-set glob matcher adapted from Touki.

Changes:

• Add FileMatcherImplementation selection (Auto/Legacy/Optimized), direct-enumerator traversal for capable file systems, and cache-key partitioning by implementation.
• Introduce MSBuildPathMatcher (specialized fast paths + bounded NFA state-set) and wire it into optimized enumeration logic.
• Add benchmarks and extensive unit tests validating optimized vs legacy behavior, plus ChangeWaves documentation update.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/MSBuild.Benchmarks/MSBuildPathMatcherBenchmark.cs	Adds matcher-only microbenchmarks comparing legacy regex matching vs the new matcher.
src/MSBuild.Benchmarks/FileMatcherBenchmark.cs	Adds filesystem traversal benchmarks (cold/warm/cache-backed) comparing legacy vs optimized enumeration.
src/Framework/Utilities/FileMatcher.cs	Implements optimized enumeration path, direct traversal option, implementation selection & cache key partitioning.
src/Framework/Globbing/MSBuildPathMatcher.cs	New optimized path+filename matcher used by the optimized enumeration path.
src/Framework/FileSystem/MSBuildOnWindowsFileSystem.cs	Marks Windows filesystem as supporting direct enumeration.
src/Framework/FileSystem/ManagedFileSystem.cs	Marks managed filesystem as supporting direct enumeration.
src/Framework/FileSystem/IFileSystem.cs	Adds IDirectFileSystemEnumeration capability interface.
src/Framework/FileSystem/CachingFileSystemWrapper.cs	Propagates direct-enumeration capability through the wrapper.
src/Framework.UnitTests/Globbing/MSBuildPathMatcher_Tests.cs	Adds unit tests for directory/file matching edge cases and platform semantics.
src/Framework.UnitTests/Globbing/FileMatcherOptimized_Tests.cs	Adds differential tests validating optimized vs legacy across patterns, excludes, caching, injected file systems, and error paths.
documentation/wiki/ChangeWaves.md	Documents the new ChangeWave 18.11 feature entry.

---

💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.

[ViktorHofer]

Expert MSBuild Review — Optimized FileMatcher (1778859ef0..c889a2c8d3)

Verdict: 🔴 REQUEST CHANGES

This is high-quality, ambitious work — the matcher design is sound, the ChangeWave gate is wired correctly, the new capability interface avoids an IFileSystem breaking change, and the differential test harness is the right idea. But Wave-2 validation (independent agent, executed xUnit repros on the PR branch, harnesses deleted) confirmed 5 concrete behavioral divergences from the legacy path and measured a 2.2×–3.0× wall-clock regression for the exact shape this PR targets (evaluation-time SDK globs). Since ChangeWave 18.11 defaults the optimized path on, these ship as silent wrong-output and slower builds unless a customer knows to disable the whole wave.

Severity	Count	Notes
🔴 Critical	2	silent item-set loss; measured perf regression
🟠 High	8	4 more confirmed divergences + alloc/caching/API traps
🟡 Medium	10	design doc, diagnostics, duplication, test gaps
⚪ Low/Nit	12	naming, style, docs

Clean dimensions (6/24): #1 Backwards Compatibility (the gate itself is correct — see note), #9 Target Authoring (N/A, no .targets/.props), #13 Concurrency (only a NIT), #19 Build Infrastructure (full build.cmd green, 0 warnings, all 3 Framework TFMs), #23 Dependency Management (no package/version edits; Microsoft.IO.Redist 6.1.0 already exposes every API used), #24 Security (symlink-loop protection preserved per-directory; AttributesToSkip = 0 is deliberate parity with EnumerationOptions.Compatible, not a weakening).

Note on #1: dimension 1 is "clean" only in the mechanical sense — ResolvedImplementation is the single gate, all production call sites use Auto, the opt-out works, and the exception refactor is provably behavior-preserving. But the wave defaults on, so every Critical/High divergence below is a breaking change for anyone who doesn't set MSBUILDDISABLEFEATURESFROMVERSION=18.11.

---

🔴 Critical

C1 — Excludes are not rebased onto the include's base directory: **/obj/** silently deletes the entire result set

src/Framework/Utilities/FileMatcher.cs:2746-2752 (AddExclude), :2666-2674 (callback driver), :2985-2992 (direct driver) — vs legacy :3320-3333

Reproduced (passing xUnit repro, both drivers):

// files: <root>/obj/gen/g1.cs, <root>/obj/gen/deeper/g2.cs
matcher.GetFiles(root, @"obj\gen\**\*.cs", [@"**\obj\**"]);
// Legacy:    obj\gen\g1.cs, obj\gen\deeper\g2.cs
// Optimized: <empty>

Also confirmed for bin\Debug\**\*.cs + **\bin\**, src/obj/**/*.cs + **/obj/**, node_modules/pkg/**/*.cs + **/node_modules/**, and **/o?j/**.

Why it matters: legacy handles the **/{name}/** exclude shape via FilesSearchData.DirectoryPattern + IsInsideMatchingDirectory, rebasing the exclude onto the include's base and keeping IsInsideMatchingDirectory == false — so the obj that already sits in the include's fixed directory part is never re-examined and the exclude never fires. AddExclude instead keeps the exclude's own BaseDirectory and matches the full relative path, so the prefix obj matches and everything is dropped.

This is the single most common shape in the .NET SDK: DefaultItemExcludes is **/bin/**;**/obj/**, and source generators / CompilerGeneratedFilesOutputPath / T4 / protobuf outputs are routinely included from under obj/. Generated .cs files silently vanish from @(Compile) — no error, no warning, just a build that fails later with CS0246.

Fix: reproduce legacy's rebasing for the DirectoryPattern shape. When the exclude's base is an ancestor of the include's base, restart the exclude matcher at the include base in the "not yet inside a matching directory" state, so only directories discovered below the include base can satisfy the {pattern} segment:

// in AddExclude, when excludeState.SearchData.DirectoryPattern is not null
// and excludeState.BaseDirectory is an ancestor of state.BaseDirectory:
//   rebase to state.BaseDirectory and require the pattern segment to be
//   matched by a directory discovered during traversal, not by the fixed prefix.

Add OptimizedMatchesLegacy inline data for include-rooted-under-an-excluded-directory.

---

C2 — The "optimization" is a 2.2×–3.0× wall-clock regression on multi-core for the evaluation-time shape

src/Framework/Utilities/FileMatcher.cs:2649-2745 (EnumerateDirectory), :2626-2630 (direct driver loop) — vs legacy :1024-1070, :3384-3389

Legacy fans out one level deep with Parallel.ForEach (dop = max(1, cores/2), active when cores >= 4). Both optimized drivers are strictly single-threaded — no Parallel, Task, or Thread anywhere in the new code.

Independently measured (8 logical cores, Release net11.0, warm OS cache, median of 7 alternating runs; opt/legacy ratio, >1 = slower):

Scenario	wall	cpu	alloc
src **/*.cs, 10 SDK excludes, entry-cache (= evaluation path)	2.18–2.40×	0.8–1.3×	1.02×
⤷ same, pinned to 1 core	0.97–1.15×	1.00×	1.02×
synthetic 100 dirs × (2 .cs + 200 .txt)	2.74–3.01×	0.75×	1.37×
⤷ same, pinned to 1 core	1.04–1.34×	1.00×	1.37×
artifacts\bin **/*.dll (16 531 files)	1.09–1.39×	0.50×	0.38×
artifacts **/*.cs, no excludes (773 files)	0.57–0.90× (faster)	0.54×	0.31×

The single-core control is decisive: every multi-core regression collapses to ≈1.0× when pinned, so the loss is attributable entirely to the missing Parallel.ForEach. CPU time genuinely drops (0.5–0.6× for the direct driver) — the engine is cheaper per unit of work, it just gave up all its concurrency. The regression scales with core count, so a 32-core CI agent will be worse than these 8-core numbers.

Fix: parallelize the top-level subdirectory fan-out of EnumerateDirectory using the same TaskOptions budget legacy uses (collect into a ConcurrentStack<List<string>> as before); for the direct driver, partition over the first level of subdirectories with one FileSystemEnumerator per partition. Do not default wave 18.11 to Optimized until wall-clock parity is demonstrated on a real SDK-shaped tree at ≥8 cores.

---

🟠 High

H1 — Direct enumerator canonicalizes the root, so lexically-discarded excludes start firing

FileMatcher.cs:2915-2918 (GetFullPath), :3057-3110 — vs legacy lexical comparison :3257-3273

// files under <root>/src/
matcher.GetFiles(root, @".\**\*.cs", [@"src\**"]);
// Legacy:            .\src\a.cs, .\src\deeper\b.cs
// Optimized direct:  <empty>
// Optimized cached:  matches legacy (only the direct driver diverges)

Legacy compares exclude/include base directories lexically (IsSubdirectoryOf, Ordinal-IC), so <root>\.\ is neither equal to nor under <root>\src\ and the exclude is discarded outright. Path.GetFullPath on the enumeration root makes them match. Same for x/../x/**/*.cs shapes.

Fix: keep the lexical (un-GetFullPath'd) directory string alongside the resolved one and evaluate excludes against the lexical path, or pre-filter excludes with the same lexical IsSubdirectoryOf test legacy uses before handing them to the enumerator.

H2 — Trailing separator on projectDirectory makes the direct enumerator return absolute paths

FileMatcher.cs:3113-3122 (GetOutputRoot) — vs legacy RemoveProjectDirectory :740-770

matcher.GetFiles(root + Path.DirectorySeparatorChar, @"**\*.cs");
// Legacy:    root.cs, src\a.cs
// Optimized: C:\...\<root>\root.cs, C:\...\<root>\src\a.cs

GetFileSearchData sets BaseDirectory = Normalize(projectDirectory), which trims the trailing separator, so lexicalEnumerationRoot.StartsWith(projectDirectory, Ordinal) fails and stripping is skipped entirely. This changes item Identity/RelativeDir downstream — e.g. Copy's SourceFolders path does Path.Combine(DestinationFolder, srcName, file), which silently discards the destination when file is absolute.

Fix: trim trailing separators from projectDirectory before the StartsWith comparison, mirroring RemoveProjectDirectory. Add a test with a trailing-separator project directory.

H3 — Trailing-dot flag asymmetry between include and exclude matchers

FileMatcher.cs:2770-2772 (useTrailingDotRegex: !matchForExclusion && ...), MSBuildPathMatcher.cs:141-146 — vs legacy identical-spec short-circuit :914-920

// extension-less files README, sub\LICENSE present
matcher.GetFiles(root, @"**\*.", [@"**\*."]);
// Legacy:            <empty>
// Optimized direct:  README, sub\LICENSE

Exclude matchers are built with useTrailingDotRegex: false while include matchers use true, so *. matches the extension-less names on the include side but not on the exclude side. Legacy additionally short-circuits when the exclude spec equals the include spec.

Fix: use the same trailing-dot flag for exclude matchers as for include matchers, and/or port legacy's "exclude spec is identical to / a superset of the include spec" bail-out. Note the existing test LogicalTrailingDotExcludeDoesNotRemoveExtensionlessFile currently pins the wrong direction.

H4 — . segments are dropped from the wildcard directory part

MSBuildPathMatcher.cs:672-702 (SplitDirectoryPatterns)

matcher.GetFiles(root, @"*\.\*.cs");
// Legacy:    <empty>   (walks "." as a literal directory name — FileMatcher.cs:1048)
// Optimized: a\b.cs

Legacy's Normalize only collapses . in the fixed leading directory. SplitDirectoryPatterns drops . unconditionally, silently rewriting */./*.cs into */*.cs.

Fix: only drop . segments that precede the first wildcard segment; treat a . after a wildcard segment as a literal name. Same review applies to the unconditional consecutive-** collapse (untested — add **/**/*.cs, **/./**/*.cs to the differential corpus).

H5 — Linux: culture-sensitive directory regex ignores the case-sensitivity flag

MSBuildPathMatcher.cs:576-581 (MatchesDirectoryName), :622 (new Regex(..., RegexOptions.IgnoreCase)), :97 — set from FileMatcher.cs:2764, 2780

CreatePathMatcher passes filesystemCaseSensitive: NativeMethods.IsLinux and useCultureSensitiveMatch: RegexFileMatch is not null. Line 97 correctly computes _ignoreCaseDirectoryPatterns[index] == false for pre-globstar segments on Linux — but MatchesDirectoryName checks _cultureSensitiveDirectoryPatterns first and never consults that array; the regex is built with RegexOptions.IgnoreCase unconditionally.

Scenario (Linux): include src/*/obj/**/*.cs (multi-segment wildcard dir → legacy regex mode) with /repo/src/p/obj/a.cs and /repo/src/p/Obj/b.cs. Legacy pushes the directory pattern into EnumerateDirectories(path, "obj") → MatchCasing.PlatformDefault → case-sensitive, so Obj is never visited. Optimized returns b.cs too.

Fix: pass the case-sensitivity flag into CreateCultureSensitiveDirectoryRegex and set RegexOptions.IgnoreCase only when !filesystemCaseSensitive || globstarSeen. Add a test combining filesystemCaseSensitive: true with useCultureSensitiveMatch: true (the existing test at MSBuildPathMatcher_Tests.cs:166-180 never combines them).

H6 — Callback driver passes a null filename pattern → 12× allocation on low-selectivity directories

FileMatcher.cs:2679-2685 — cached callback :152-177, RemoveProjectDirectory :740-770

The callback driver calls _getFileSystemEntries(FileSystemEntity.Files, directory, pattern: null, ...). With pattern == null the cached callback skips the IsFileNameMatch filter (:170-172) and runs RemoveProjectDirectory(...).ToList() — a Substring per entry — over every file in the directory. Legacy passes SearchData.Filespec and filters before stripping (:1097-1105).

Measured (100 dirs × 2 .cs + 200 .txt, entry cache pre-populated to isolate filter+strip): legacy 138.5 KB → optimized 1 670.5 KB (12.1×); end-to-end with a cold cache 4.05 MB → 5.55 MB. This is an allocation regression in the path the PR claims to optimize.

(Disputed sub-claim, for the record: this does not corrupt or bloat the entry cache — the cache key is "F"/"D"/"A" + ";" + path and the factory always enumerates with "*", so the pattern is not part of the key.)

Fix: pass state.SearchData.Filespec when non-null (the matcher re-checks the name anyway), or strip lazily only for entries that passed includeMatcher.MatchesFileName.

H7 — No caching of matchers or regexes; ~4× the Regex compiles of the legacy path

MSBuildPathMatcher.cs:66-146, 622, 659; FileMatcher.cs:2755-2787 (CreatePathMatcher), :2233

A MSBuildPathMatcher is constructed per GetFiles call plus one per exclude spec, each allocating a List<string> + a string per segment (segment.ToString() allocates even for the constant "**") + the [.. patterns] copy + up to four parallel arrays. None of the new Regex(...) calls are cached — contrast the legacy s_regexCache at :73. Measured 8.3 µs / 6 216 B per matcher for **/src/**/a/** + *.cs; ~4 584 B for a typical "1 include + 10 SDK excludes" set.

Worse, GetFileSearchData still builds a Regex at :2233 that the optimized path never executes — it only tests it for null (:2767, 2772, 2780).

Fix: add a static ConcurrentDictionary cache keyed by (wildcardDirectoryPart, filePattern, flagBits) (the flags are deterministic per spec), mirroring s_regexCache; intern the "**" segment; and skip materializing the throwaway RegexFileMatch when the optimized driver is selected (pass the matchWithRegex boolean instead).

H8 — Capability-interface trap: WindowsFileSystem silently inherits SupportsDirectEnumeration => true

src/Framework/FileSystem/ManagedFileSystem.cs:17, 35; WindowsFileSystem.cs:34, 42-55

SupportsDirectEnumeration is a non-virtual property on the non-sealed ManagedFileSystem. WindowsFileSystem : ManagedFileSystem overrides EnumerateFiles/EnumerateDirectories with native FindFirstFileEx enumeration whose semantics differ (8.3 short names, "*" special-casing) — i.e. it is not a pass-through, yet it inherits the claim that it is. Unreachable today, but MSBuildOnWindowsFileSystem.cs:13 carries the TODO "Remove this class and replace with WindowsFileSystem" — executing that TODO silently enables direct enumeration for a non-conforming file system.

// ManagedFileSystem
public virtual bool SupportsDirectEnumeration => true;
// WindowsFileSystem
public override bool SupportsDirectEnumeration => false;

H9 — Result cache key encodes the resolved enum, not the effective driver

FileMatcher.cs:2056 ("l;"/"o;"), :1991-1996, :2445-2461

GetFilesForImplementation falls back to GetFilesImplementation when !SupportsOptimizedImplementation, when an entry cache is present with zero excludes, or when RequiresLegacyImplementation — all of which still write under "o;". And CreatePathMatcher deliberately configures different matching semantics per driver (filesystemCaseSensitive, useWin32FileNameMatch, useTrailingDotRegex all keyed off useCachedCallback). So under MsBuildCacheFileEnumerations (CloudBuild), the process-wide s_cachedGlobExpansions can serve a Linux case-sensitive direct-driver result to a case-insensitive callback-driver caller under the identical key.

(Fairly noted: the underlying Linux case split is pre-existing legacy behavior — the documented back-compat hack at :1088-1092 — and the old key had no discriminator either, so the PR doesn't make it worse. But the PR is what makes the key look like it discriminates.)

Fix: encode the semantic knobs (_usesFileSystemEntryCache / _allowDirectEnumeration, or a hash of the CreatePathMatcher option tuple) rather than the enum: "l;" / "oc;" / "od;".

H10 — Test gaps on exactly the code that broke

The differential/oracle strategy is excellent and the corpus reuse (GetFilesComplexGlobbingMatchingInfo) is the right call. Tests build clean and pass (61/0/7 on net11.0, 29/0 on net472). But:

Gap	Location
FailOnDriveEnumeratingWildcard (env var = "1") never tested — both hand-rolled return tuples (:2532, :2575) have zero coverage	FileMatcherOptimized_Tests.cs:399-435
Symlink loops during traversal untested — only the case where the symlink is the enumeration root; ShouldRecurseIntoEntry's loop guard (:2972) is never hit	:915-946
Result-cache hit path never executed (ClearCaches() before every query) — a key that ignored excludes or the implementation would still pass	:208-235
Differential tests can pass vacuously — 216 combinations with no independent expectation and no ShouldNotBeEmpty on the legacy oracle	:31-89, :296-360
Wave-disabled path asserts only the enum value; never calls GetFiles	:271-295
stripProjectDirectory == false (rooted include + non-null project dir) never exercised — the shape that broke in H2	:237-269
Weak assertions: ShouldNotBeEmpty() as the only check in 4 tests	:379, :394, :831-832, :799
MatchesFilesInDirectory (the hottest entry point, :2656) has no direct test; useWin32FileNameMatch: true never set; multi-globstar NFA never exceeds StackStateCount so the pool fallback is untested	MSBuildPathMatcher_Tests.cs

Every one of C1–C2/H1–H5 would have been caught by adding these cases to the differential matrix.

---

🟡 Medium

• No design doc for a 3 393-line engine rewrite. documentation/specs/ has nothing on globbing; the ChangeWaves link (#12072) is an open investigation issue ("Investigate the usage/performance of the enumerator patterns shown in the Touki library"), not an approved design. Nothing records the equivalence contract, the driver matrix, the case-sensitivity model, or the rollback plan. → Add documentation/specs/optimized-file-matching.md; link the PR in ChangeWaves.md as the neighbouring entries do. (Xplat Branch Merge #10, Changed 'canceled' back to 'cancelled" #18)
• Driver selection is a nested ternary over four predicates with zero comments (:2443-2463, :2466-2468, :2837-2851). The most surprising clause — _usesFileSystemEntryCache && excludeSpecsUnescaped?.Count is not > 0, which sends cached, exclude-free globs (bare **/*.cs) back to legacy — is entirely unexplained, and CanUseDirectEnumeration re-derives predicates SupportsOptimizedImplementation already tested. → Extract private FileMatcherDriver SelectDriver(...) returning an explicit enum, one XML-doc'd rule per branch. (Xplat Branch Merge #10, Add build status badge #12)
• Blast radius is inverted vs intuition. EvaluationContext always supplies a cache (EvaluationContext.cs:69-71), so SDK globs with DefaultItemExcludes take the new matcher while the same glob without excludes stays legacy — i.e. the wave lands squarely on SDK default items. EngineFileUtilities uses FileMatcher.Default (null cache) → the third, direct driver. Run an end-to-end differential over dotnet/sdk, dotnet/runtime and a WPF/WinForms project before flipping the wave. (Fixed some typos #15)
• Zero diagnostics and no per-feature kill switch. git diff ... | grep '^+.*///' → 0 matches; nothing indicates which of three drivers ran, and FileMatcherImplementation has no Traits/env-var binding, so the only field mitigation is MSBUILDDISABLEFEATURESFROMVERSION=18.11 — which also disables two unrelated shipped features. Precedent exists two layers up: EngineFileUtilities.cs:210-213 gates on Traits.Instance.LogExpandedWildcards. → Add MSBUILDFILEMATCHERIMPLEMENTATION + emit the selected driver under LogExpandedWildcards. (Migrate from MSTest to xunit #6)
• CreateCultureSensitiveFileRegex/CreateCultureSensitiveDirectoryRegex are hand-copied reimplementations of AppendRegularExpressionFromFilename (FileMatcher.cs:1359-1389), duplicating both quirks (the *.* collapse guard, the [^.]. expansion of ?). The escaping tables have already drifted: AppendRegexLiteral (:663) escapes \ . $ ^ { [ ( | ) + ] while IsSpecialRegexCharacter (FileMatcher.cs:1421) escapes $ ( ) + . [ ^ { | — \ and ] differ. The non-culture path already reuses FileMatcher.RegularExpressionFromFileSpec (:137), proving reuse is possible. (Xplat Branch Merge #10, Add build status badge #12)
• Three TryGetRelativeDirectory implementations (:2800 local static, :3057/:3067 overloads → :3077 core) that are not equivalent: the local copy hard-codes OrdinalIgnoreCase and omits the "base already ends in a separator" branch, so it returns false for ("/", "/foo") — a rooted exclude at / would silently stop excluding on Unix. → Collapse to one class-scope method. (Add build status badge #12, Fixed typos #22)
• IgnoreInaccessible = true (:3021) is a behavior change. Legacy catches Security/UnauthorizedAccessException per directory and returns [] for that whole directory (:395-404, :445-456); the new path skips per entry, so it returns more results. Not a security issue, but a divergence with no test. (Fixed typos #22)
• Benchmarks don't measure the changed paths. LegacyWarmCache/OptimizedWarmCache (FileMatcherBenchmark.cs:102-108) hit _cachedGlobExpansions and return the memoized array — measured cold 35.30 ms vs warm 0.0086 ms (~4 100×); they benchmark key construction + a dictionary lookup + ToArray. The tree ([Params(32, 512)], three first-level directories, depth 4-5) structurally cannot expose the parallelism loss, since legacy's fan-out is capped at 3 there. Matcher construction is hidden in [GlobalSetup]. (dir.targets: correctly enclose 'Exists' condition in single quotes #3)
• MSBuildGlob (Remove/Update matching) still uses the legacy regex engine (src/Build/Globbing/MSBuildGlob.cs:185-211). Enumeration switches under the wave while matching does not — a divergence now manifests as an item that appears in the include set but is not matched by a Remove, which is far harder to diagnose. → Add a differential test asserting MSBuildGlob.IsMatch agrees with the optimized enumeration over the shared corpus. (Xplat Branch Merge #10)
• Result ordering flips DFS → BFS (FileSystemEnumerator's pending-directory queue). Legacy order was already nondeterministic under Parallel.ForEach, so this is informational — but worth a note in the wave documentation. (Fixed typos #22)

---

⚪ Low / Nit

• Dead tracking machinery: trackedAction/trackedExcludeFileSpec (:2555-2556) are assigned once and never reassigned after commit c889a2c8d3; trackedAction is always RunSearch, trackedExcludeFileSpec always string.Empty. Delete both. (Add build status badge #12)
• ResolvedImplementation is evaluated twice per call (:1991 for the key, :2440 for dispatch) — each calls ChangeWaves.AreFeaturesEnabled. Resolve once and pass it down. (Fix casing of file references in csprojs #2, dir.targets: correctly enclose 'Exists' condition in single quotes #3)
• Duplicated redundant syscall: ShouldSkipRecursiveDirectory(state.BaseDirectory) at :2612 is repeated at :2651 for the same directory.
• Indentation: IsWin32FileNameMatch (:1708-1723), TranslateWin32Expression (:2853-2862) and ShouldSkipRecursiveDirectory (:2864-2879) are indented at 4 spaces inside an 8-space class body, with #if directives and continuation arguments misaligned. Doesn't match .editorconfig.
• Idiomatic C#: literalExcludes ??= new HashSet<string>() (:2572) vs (excludesToMatch ??= []) (:2746) in the same method; literalExcludes?.Contains(file) == true (:2686) → is true; AttributesToSkip = 0 (:3022) → FileAttributes.None; projectDirectory[projectDirectory.Length - 1] (:3131) vs baseSpan[^1] (:3084); manual for copy in the enumerator ctor (:2924-2934) → [.. excludes]; EndsWith(".", Ordinal) (MSBuildPathMatcher.cs:133) → EndsWith('.'); excludeSpecs?.Count is not > 0 → excludeSpecs is null or { Count: 0 }. (Support yaml/json #16, Add build status badge #12)
• Duplicated helpers: MatchesName's hand-rolled case-sensitive matcher (:521-557) duplicates FileMatcher.IsMatch with different edge-case semantics (IsMatch throws InternalError on empty input; the copy returns true for ("", "*")); GetFileName (:491-502) re-implements span Path.GetFileName (the repo idiom is at FileMatcher.cs:1696-1706); IsCandidateDirectorySeparator (:843) duplicates FileUtilities.IsSlash. (Add build status badge #12)
• Dead-ish API: both MatchesFile overloads (:153, :163) are referenced only by tests and benchmarks. (Add build status badge #12)
• Naming ([Nefarious Grammar] Bad sentence structure #14): _allowDirectEnumeration gates the entire optimized implementation, not just direct enumeration → _allowOptimizedImplementation; IsWin32FileNameMatch is also used for directories → IsWin32NameMatch; useCultureSensitiveMatch actually means "legacy regex mode"; MSBuildPathMatcher matches spec fragments, not paths; DirectoryMatchType.MayContainMatchingFiles also covers "definitely matches". The 9-parameter, 7-bool constructor is call-site-safe (named args used in production) but the flags are undocumented → consider a readonly struct MSBuildPathMatcherOptions.
• ArgumentNullException.ThrowIfNull in an internal ctor (MSBuildPathMatcher.cs:77-78) — the file's convention for internal invariants is Assumed.True/InternalError.Throw (FileMatcher.cs:882-890, :1746). (Issue Guide wiki entry does not exist, README references it #5)
• Lost rationale: the legacy inline symlink check carries a 10-line comment enumerating what the heuristic misses (mutual A→B/B→A links, issue FileState/Copy task support for symlinks #703); ShouldSkipRecursiveDirectory, now the shared implementation for both new drivers, carries none of it. Move the comment. (Changed 'canceled' back to 'cancelled" #18)
• Provenance header (MSBuildPathMatcher.cs:1-13) pins the touki commit (good) but omits the URL, license statement and adaptation list that the existing precedent (ValueStringBuilder.cs:1-12) includes; THIRDPARTYNOTICES.txt has no touki entry. Worth confirming with repo owners now that ~1 100 adapted lines have landed. (Changed 'canceled' back to 'cancelled" #18)
• Scope: the InnerExceptionsAreAllIoRelated → IsIoRelatedExceptionOrAggregate refactor (:3419) touches the legacy path. It is provably behavior-preserving (the old catch (AggregateException) when (...) re-tested the same predicate inside, making the throw; unreachable dead code), but it should be split out or called out in the PR description so a future glob-exception regression isn't bisected into this PR. Likewise note ShouldEnforceMatching private → internal (:319) as intentional. (Removing redundant contructors (empty is provided by default) #20)
• Tests (Truncated source file #4): no test follows MethodUnderTest_Scenario_ExpectedResult; OptimizedMatchesLegacyForPatternMatrix (:297) uses nested foreach instead of [Theory]/MemberData, so failing combinations aren't individually reported; ExternalStyleFileSystemWithoutEntryCacheUsesLegacyImplementation (:802-814) asserts the opposite of its name (ShouldBe(Optimized)) and discards the GetFiles result; manual try/finally for CurrentCulture (:611-654, MSBuildPathMatcher_Tests.cs:252-274) and for the ChangeWave env var (:274-291) contradicts tests.instructions.md — TestEnvironment.Dispose already resets both.
• using Microsoft.Build.Utilities; in FileMatcher.cs (:28) is not a layering inversion — it resolves to the Framework-internal BufferScope<T>/ValueStringBuilder in the same assembly — but it collides with the public namespace of the higher-layer Microsoft.Build.Utilities.Core and reads as one. Add a one-line comment or move those helpers to Microsoft.Build.Shared. (Xplat Branch Merge #10)
• IDirectFileSystemEnumeration.SupportsDirectEnumeration has no member XML doc and the implementor obligation (results must be set-equivalent to System.IO enumeration; no filtering/redirection/caching) is unstated. Also worth one sentence in the MSBuildFileSystemBase docs noting that custom file systems permanently take the non-direct path. (Nuked ThreadingUtilities.cs #8, Changed 'canceled' back to 'cancelled" #18)
• FileSystemEnumerator<T> ordering invariant is undocumented. I verified against the runtime sources that DirectoryFinished() calls OnDirectoryFinished before DequeueNextDirectory, and that both the Windows and Unix implementations use a Queue<> (BFS), so _fileStateValid caching is sound and parent/child entries can never interleave. But that is an implementation detail, not a documented contract — and MSBuild also runs the separately-versioned Microsoft.IO.Redist port on net472. Add Debug.Assert(directory.SequenceEqual(_stateDirectory)) in EnsureFileState plus a comment recording the relied-upon ordering. (Fixed some typos #13)

---

Suggested path forward

1. Fix C1 (exclude rebasing) and add the include-under-excluded-directory case to the differential matrix — this one alone can silently break SDK builds.
2. Fix C2 by restoring parallel fan-out, then re-run the benchmarks on a wide/deep tree and paste BDN output into the PR description.
3. Fix H1–H5 (all confirmed divergences) and extend OptimizedMatchesLegacy inline data to cover each: ./-prefixed includes, trailing-separator project directories, trailing-dot include+exclude pairs, .-in-wildcard segments, doubled **, and (Linux) case-differing directory names in regex mode.
4. Fix H6/H7 (allocation + caching), close the H10 test gaps, then add the design doc and driver-selection refactor.
5. Consider landing the matcher + its unit tests first, and the FileMatcher wiring + direct-enumeration driver as follow-ups — 3 393 lines in two commits is a lot to review as one unit, and the split would make bisecting any field regression far cheaper.

Read-only review — no files were modified. All validation harnesses created during review were deleted; git status is as found.

[ViktorHofer]

Two things that I think are additive to the review above rather than a repeat of it. I re-measured independently from the evaluation side (OrchardCore modules, same-binary ChangeWave A/B, MSBuild's own event-source markers), and I land on the same conclusion as C2 — so treat the root cause there (lost Parallel.ForEach fan-out, proven by the single-core control) as the explanation for both points below. I'm not re-litigating that.

1. The direct driver is unreachable from evaluation-time globs

CanUseDirectEnumeration requires !_usesFileSystemEntryCache:

return _allowDirectEnumeration
    && !_usesFileSystemEntryCache      // <-- 
    && literalExcludes is null
    && _fileSystem is IDirectFileSystemEnumeration { SupportsDirectEnumeration: true }
    && (!string.IsNullOrEmpty(projectDirectory) || Path.IsPathRooted(fileSpec));

and _usesFileSystemEntryCache = getFileSystemDirectoryEntriesCache is not null. Every evaluation reaches GetFiles through a FileMatcher that was handed a cache:

• EvaluationContext ctor: FileMatcher = new FileMatcher(FileSystem, FileEntryExpansionCache)
• LazyItemEvaluator.IncludeOperation: GetFileListEscaped(..., fileMatcher: FileMatcher, ...)

So for item globs during evaluation the direct FileSystemEnumerator driver can never be selected — it is always the cached-callback driver. Two consequences worth making explicit:

• The headline table in the PR description (0.42x–0.70x, "matched direct-traversal scenarios") describes a path that evaluation never takes. The rows that describe evaluation are the cache-backed ones, reported as 0.95x–1.01x; the numbers below and in C2 say those are the ones that regressed. Worth re-labelling the table so the evaluation-relevant rows aren't read as the headline.
• It also partitions the correctness findings by blast radius. The divergences reported as direct-driver-only (H1 root canonicalization, H2 trailing-separator absolute paths, H4 . segments in the wildcard part) cannot affect evaluation-time item globs; they are reachable only from callers that pass no matcher and therefore get FileMatcher.Default (no cache). C1 is the one that matters most for evaluation, since it reproduces on both drivers and **/bin/**;**/obj/** is DefaultItemExcludes.

Note this is not an argument that the guard is wrong — bypassing the entry cache would stop the 6 globs in a typical project from sharing directory entries. It just means the optimized path that evaluation actually gets is the one that was least covered by the PR's headline benchmarks.

2. What C2 costs at the evaluation level, not the component level

C2 measures FileMatcher in isolation. Sizing it as a share of a full evaluation, using the ExpandGlob event-source marker (which wraps EngineFileUtilities.GetFileListEscaped in LazyItemEvaluator.IncludeOperation), median of 3 alternating runs, same binary, wave on vs off:

Project	files / dirs	glob, optimized	glob, legacy	cold evaluation
OrchardCore.Contents	357 / 62	16.61 ms	10.91 ms	~72 ms
OrchardCore.OpenId	100 / 27	7.46 ms	6.41 ms
dotnet new console	27 / 8	1.02 ms	3.70 ms

For OrchardCore.Contents that is +5.7 ms on a ~72 ms cold evaluation, so roughly +8% of the entire evaluation of that project — glob expansion goes from 15.1% to 23.1% of cold evaluation time. Multiply by project count for a solution-wide restore/build.

Two qualifiers, both of which cut in the PR's favour and are worth recording:

• The console-app row is faster (0.28x), and the ratio degrades monotonically with tree size (0.28x → 1.16x → 1.52x). That is consistent with C2's parallelism explanation: a 27-file tree has nothing to fan out, so the cheaper per-entry engine wins outright. The crossover on this machine is somewhere between 27 and 100 files, and C2's single-core control implies it moves up with core count.
• Re-evaluating the same project against a shared EvaluationContext (design-time/VS, project graph) drops ExpandGlob to ~1% of evaluation, because the glob results are already cached. The regression is therefore specific to cold, isolated-context evaluation — which is what a command-line build does for every project.

Happy to share the harness if useful; it's event-source markers plus an injected MSBuildFileSystemBase, so it attributes to MSBuild scopes rather than to FileMatcher internals.

[JeremyKuhne]

@ViktorHofer, thank you for the detailed review and independent evaluation measurements. I rebased onto current main and pushed 743d069bfc, which addresses the findings and shifts the PR description to evaluation-first evidence.

Critical findings

• C1, exclude rebasing: optimized exclude planning now preserves legacy include/exclude root relationships. SDK-style ancestor excludes are covered for callback and direct traversal. Ancestor excludes whose non-globstar prefix would be consumed by rebasing explicitly fall back to legacy.
• C2, lost parallelism: cache-backed traversal again uses bounded top-level parallel fan-out with per-partition result buffers. Filename prefiltering now occurs before relative-path materialization, and the optimized path no longer compiles a legacy regex it never executes.

Final sampled complete-project evaluation is 0.87x-0.90x legacy time with 0.66x-0.84x allocation. Full-sampling checkpoints measured 0.80x-0.90x time. Shared-context evaluation is neutral at approximately 1.01x after the glob cache is warm.

High findings

• H1: lexical include/exclude applicability is preserved; unsafe ancestor-prefix forms fall back.
• H2: trailing project separators preserve relative output identities.
• H3: identical trailing-dot include/exclude patterns short-circuit consistently.
• H4: wildcard-directory dot segments preserve legacy semantics.
• H5: Linux regex-classified directory segments honor the established per-driver filesystem casing behavior.
• H6: cache-backed enumeration passes the filename pattern and filters before path materialization.
• H7: optimized filespec analysis carries matching mode without constructing the unused whole-path regex; matcher benchmarks now use invariant Wave semantics.
• H8: WindowsFileSystem explicitly declines direct-pass-through capability, and that capability contract is documented.
• H9: one effective driver and fallback reason are selected before cache lookup. Whole-glob keys now partition cached/uncached legacy, callback, fallback, and direct profiles; culture modes; project/include identity; and length-delimited excludes. Ordered and concurrent cache-poisoning regressions cover Linux differences.
• H10: added independent expected-output tests, actual Wave routing, cache-hit/population-order and concurrency tests, root-project and null-project cases, child-partition exception coverage, direct/callback parity, culture matrices, and exact project-evaluation item identity validation.

Additional compatibility work

Regex-backed filesystem globs now use RegexOptions.CultureInvariant under Wave 18.11. MSBUILDUSELEGACYCULTURESENSITIVEFILEGLOBS=1 restores only legacy current-culture behavior and is read dynamically so reused nodes honor environment refreshes. MSBuildGlob uses the same policy so Include/Exclude/Remove do not diverge.

Measured in-situ fallbacks retain legacy for no excludes, unrelated fixed excludes, unsupported lexical shapes, invalid wildcard exact-identity excludes, drive-wide requests, null-project relative globs, and custom/non-capable filesystems. Recursive SDK default excludes (**/bin/**, **/obj/**) select the optimized evaluation path and measure 0.78x-0.89x legacy component time.

Post-rebase validation:

• Framework net11.0 + net472: 2,133 passed, 70 expected skips
• Build culture/MSBuildGlob: 202 passed, 2 expected skips
• CreateItem: 46 passed, 4 expected skips
• Linux matcher/FileMatcher: 161 passed, 5 expected skips
• Linux culture/MSBuildGlob: 101 passed, 1 expected skip
• strict netstandard2.0: passed
• whole-repository Release build: 0 warnings, 0 errors

The PR description now leads with complete-project evaluation rather than direct traversal. Would you please take another look at the updated implementation and evidence? Fresh PR CI is pending.

[JeremyKuhne]

The macOS Core failure in build 20260818.45 is a cross-assembly test race rather than a FileMatcher failure.

BuildFailureLogInvariant in the Framework test process observed and deleted the intentional MSBuild_*failure.txt created by DebugUtils_Tests.DumpExceptionToFileShouldWriteInDebugDumpPath in the concurrently running Engine test process. That produced one Framework cleanup error (reported as 14 failed NormalizeTest theory rows) and then an Engine FileNotFoundException.

The FileMatcher tests added or changed by e4b306b875 all passed on macOS. The permanent test-isolation fix is tracked by #14740.

[OvesN]

https://dev.azure.com/devdiv/DevDiv/_git/VS/pullrequest/771156
exp insertion seems fine

[ViktorHofer]

I need to be honest that I didn't have time to review MSBuildPathMatcher.cs and FileMatcher.cs in detail but the change overall looks great.

[ViktorHofer]

complete isolated project evaluations ran 10-13% faster and allocated 16-34% less than the Wave-disabled baseline.

Be careful with such an A/B measurement. The change wave disables other features as well that we recently added.