[Efficiency Improver] perf: eliminate LINQ closure allocations in TreeNodeFilter#8035
Open
abdelghani-moussaid wants to merge 5 commits intomicrosoft:mainfrom
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes the TreeNodeFilter matching hot path to reduce runtime allocations and GC pressure during large test suite executions.
Changes:
- Replaced several LINQ-based filter/property matching paths with procedural
switch+ index-based loops. - Changed
OperatorExpression.SubExpressionsto an array to enable allocation-free indexed iteration. - Avoided substring allocations in path matching by slicing the input path into fragments.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/Platform/Microsoft.Testing.Platform/Requests/TreeNodeFilter/TreeNodeFilter.cs | Reworks filter matching to reduce allocations (procedural loops, span-based fragment slicing). |
| src/Platform/Microsoft.Testing.Platform/Requests/TreeNodeFilter/OperatorExpression.cs | Stores sub-expressions as an array to enable efficient indexed iteration. |
Author
|
@microsoft-github-policy-service agree |
Comment on lines
551
to
565
| private static bool MatchFilterPattern( | ||
| FilterExpression filterExpression, | ||
| string testNodeFullPath, | ||
| int startFragmentIndex, | ||
| int endFragmentIndex, | ||
| PropertyBag properties) | ||
| { | ||
| string str = testNodeFullPath[startFragmentIndex..endFragmentIndex]; | ||
| return MatchFilterPattern(filterExpression, str, properties); | ||
| ReadOnlySpan<char> fragment = testNodeFullPath.AsSpan(startFragmentIndex, endFragmentIndex - startFragmentIndex); | ||
| return MatchFilterPattern(filterExpression, fragment, properties); | ||
| } | ||
|
|
||
| private static bool MatchFilterPattern( | ||
| FilterExpression filterExpression, | ||
| string testNodeFragment, | ||
| ReadOnlySpan<char> testNodeFragment, | ||
| PropertyBag properties) |
Comment on lines
+620
to
+627
| case PropertyExpression { PropertyName: var propExpr, Value: var valueExpr }: | ||
| foreach (IProperty prop in properties) | ||
| { | ||
| if (IsMatchingProperty(prop, propExpr, valueExpr)) | ||
| { | ||
| return true; | ||
| } | ||
| } |
Comment on lines
7
to
12
| internal sealed class OperatorExpression(FilterOperator op, IReadOnlyCollection<FilterExpression> subExpressions) : FilterExpression | ||
| { | ||
| public FilterOperator Op { get; } = op; | ||
|
|
||
| public IReadOnlyCollection<FilterExpression> SubExpressions { get; } = subExpressions; | ||
| public IReadOnlyList<FilterExpression> SubExpressions { get; } = [.. subExpressions]; | ||
| } |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
| { | ||
| if (_filters.Count == 0) | ||
| { | ||
| throw new InvalidOperationException("Filter parsed to zero segments."); |
Comment on lines
+620
to
+624
| case PropertyExpression { PropertyName: var propExpr, Value: var valueExpr }: | ||
| foreach (IProperty prop in properties) | ||
| { | ||
| if (IsMatchingProperty(prop, propExpr, valueExpr)) | ||
| { |
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Goal
Eradicate per-call heap allocations in the filter-matching hot path to improve energy proportionality and reduce GC pressure during large test suite executions.
Changes
OperatorExpression.SubExpressions: Changed type fromIReadOnlyCollection<T>toFilterExpression[]. This enables zero-allocationforloops instead of boxedforeachenumerators.MatchFilterPattern&MatchProperties: Replaced LINQ switch expressions and.Any()calls with proceduralswitchstatements and index-basedforloops.MatchPropertiesto walk thePropertyBagdirectly, removing theAsEnumerable()wrapper and its associated closures._filters.Last()with_filters[_filters.Count - 1]and updated the constructor/initializers to use explicit loops.Performance Evidence
For a suite of 10,000 tests, this eliminates ~60,000–100,000 allocations per run.
Validation
test\UnitTests\Microsoft.Testing.Platform.UnitTeststargetingTreeNodeFilterTests.