[efficiency-improver] perf: eliminate closure allocations and redundant dict lookups in FastFilter.Evaluate - #16160
Conversation
…tFilter.Evaluate Replace 'foreach (var name in FilterProperties.Keys)' with 'foreach (var kvp in FilterProperties)' to cache the value set per-property, eliminating 1-3 redundant ImmutableDictionary hash-and-compare lookups per property per Evaluate() call. Replace the Select().Any(lambda) pattern in the multi-value branch with explicit foreach loops, eliminating: - The Func<string,bool> delegate allocation - The captured-variable closure object allocation (capturing FilterProperties and 'name' / 'filterValues') - The intermediate Select iterator allocation (in the regex path) ValidForProperties is intentionally left unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
This PR improves the hot-path performance of FastFilter.Evaluate (used per test case when filters are enabled) by removing redundant dictionary lookups and avoiding LINQ/closure allocations in the multi-value evaluation path.
Changes:
- Iterate
FilterPropertiesas key/value pairs to reuse the per-property value set (filterValues) and avoid repeated dictionary indexing. - Replace the
Select(...).Any(...)pattern with allocation-freeforeachloops (with early-exit) for multi-value comparisons, including the regex-transformation path.
Jakub Jareš (nohwnd)
left a comment
There was a problem hiding this comment.
Code Review — FastFilter.Evaluate optimization
Dimensions activated: Algorithmic Correctness · Performance & Allocations · Null Safety & Boundary Validation · Backward Compatibility
Algorithmic Correctness ✅
All execution paths are semantically equivalent to the original:
singleValuepath:filterValues.Contains(value)is identical toFilterProperties[name].Contains(value)sincefilterValues = kvp.Valueandkvp.Key == name.multiValuesno-regex path: The innerbreakexits the innerforeachonly; the outer loop'sif (matched) { break; }guard fires immediately after, preserving early-exit behavior.multiValuesregex path:ApplyRegex(result)is called for each element, andtransformed != nullmirrors the oldresult != nullcheck on the post-Selectenumerable.hasNoneFilter+ empty-array path (else if (hasNoneFilter)at the bottom): unchanged and still reachable whenmultiValuesis non-null butLength == 0.
Enumerating an ImmutableDictionary<K,V> as KeyValuePairs vs. via .Keys yields the same iteration order, so outer-loop behavior is preserved.
Performance ✅
The savings are genuine:
- Eliminates 1–3
FilterProperties[name]hash-and-compare lookups per outer iteration. - Eliminates the
Funcdelegate + captured-variable closure in theAny(lambda)call. - In the regex path, eliminates the intermediate
Selectiterator object.
using System.Linq is still needed for ValidForProperties (.All, .Where, .Contains, .ToArray), so no spurious IDE0005 warning.
Null Safety ✅
filterValues = kvp.Valueis always non-null:AddPropertyalways creates the set before inserting.multiValues is { Length: > 0 }guarantees non-null, non-empty array before either inner loop.result != null(no-regex) andtransformed != null(regex) null guards are preserved.
No issues found.
🧠 Reviewed by expert-reviewer workflow · PR #16160
🧠 Reviewed by Expert Code Reviewer 🧠
…tFilter.Evaluate (microsoft#16160) Replace 'foreach (var name in FilterProperties.Keys)' with 'foreach (var kvp in FilterProperties)' to cache the value set per-property, eliminating 1-3 redundant ImmutableDictionary hash-and-compare lookups per property per Evaluate() call. Replace the Select().Any(lambda) pattern in the multi-value branch with explicit foreach loops, eliminating: - The Func<string,bool> delegate allocation - The captured-variable closure object allocation (capturing FilterProperties and 'name' / 'filterValues') - The intermediate Select iterator allocation (in the regex path) ValidForProperties is intentionally left unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Goal and Rationale
FastFilter.Evaluateis called once per test case whenever a test filter is active. The previous implementation had two independent inefficiencies in the hot loop:Redundant dictionary lookups: iterating over
FilterProperties.Keysrequired 1–3 additionalFilterProperties[name]hash-and-compare lookups per property per call (forhasNoneFilter,singleValue, and the multi-value branch).Closure allocation in multi-value path: the
Select().Any(lambda)pattern allocated aFunc<string,bool>delegate, a captured-variable closure object (capturingFilterProperties+name), and (in the regex path) an intermediateSelectiterator — all per invocation.This PR addresses both issues with no behaviour change.
ValidForPropertiesis intentionally left unchanged.Focus Area
Code-Level Efficiency — eliminating per-test-case allocations and redundant computation in the filter evaluation hot path.
Approach
Change 1 – cache the filter value set:
Change 2 – replace
Select().Any(lambda)withforeach:The
foreach-based replacement was suggested by the maintainer in review comments on PR #16139.Energy Efficiency Evidence
Proxy metric: heap allocation count → GC pressure → CPU energy; instruction count → CPU energy.
Per
Evaluate()call, the savings are:singleValue(common)multiValues, no regexmultiValues, with regexhasNoneFilterfilterValues)For a 10,000-test run with a single-property filter: up to 20,000 fewer dictionary lookups and — for runs with multi-category filters — up to 20,000 fewer heap allocations, reducing GC pause time and CPU energy.
Green Software Foundation Context
Hardware Efficiency: Eliminating allocation-per-call noise makes better use of CPU cache lines for actual test evaluation work.
SCI (Software Carbon Intensity):
dotnet testis invoked millions of times daily across .NET CI/CD pipelines. Reducing per-invocation work lowers the energy term in the SCI equation at ecosystem scale.Trade-offs
The multi-value branch is slightly more verbose (explicit
foreachinstead of LINQ one-liner). This is offset by:foreachpattern is idiomatic C# and consistent with the rest of the codebaseTest Status
Microsoft.TestPlatform.Common.UnitTests(Filtering namespace): ✅ 108/108 passedMicrosoft.TestPlatform.Filter.Source.UnitTests: ✅ 45/45 passedReproducibility
.dotnet/dotnet run --project test/Microsoft.TestPlatform.Common.UnitTests/Microsoft.TestPlatform.Common.UnitTests.csproj -f net11.0 -- --filter "Filtering" .dotnet/dotnet run --project test/Microsoft.TestPlatform.Filter.Source.UnitTests/Microsoft.TestPlatform.Filter.Source.UnitTests.csproj -f net11.0