Fix O(n²) per-node UID lookup in TestNodeUidListFilter re-run path#7806
Fix O(n²) per-node UID lookup in TestNodeUidListFilter re-run path#7806Evangelink merged 2 commits intomainfrom
Conversation
…1) membership test Agent-Logs-Url: https://github.com/microsoft/testfx/sessions/fa2343e0-6020-4160-8937-17fa45259f87 Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Optimizes test-node UID filtering during traversal/execution to avoid repeated linear scans when re-running/retrying tests via TestNodeUidListFilter, reducing total work from O(n×m) to ~O(n+m).
Changes:
- Precomputes a
HashSetof filtered UIDs once and usesContainsfor O(1) per-node membership checks inBFSTestNodeVisitor.VisitAsync. - Applies the same pattern in the
TestingPlatformExplorersample execution loop.
Show a summary per file
| File | Description |
|---|---|
| src/Adapter/MSTest.Engine/Engine/BFSTestNodeVisitor.cs | Builds a UID HashSet once and replaces per-node Any() scans with Contains(). |
| samples/public/TestingPlatformExamples/TestingPlatformExplorer/TestingFramework/TestingFramework.cs | Uses a precomputed UID HashSet for faster filtering in the sample runner loop. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 2
| if (uidFilterSet is null | ||
| || uidFilterSet.Contains(currentNode.StableUid.ToPlatformTestNodeUid())) |
There was a problem hiding this comment.
currentNode.StableUid is already Microsoft.Testing.Platform.Extensions.Messages.TestNodeUid, so calling ToPlatformTestNodeUid() here allocates a new TestNodeUid per visited node. If you want this to be allocation-free (and likely drop the PlatformTestNodeUid alias), you can check uidFilterSet.Contains(currentNode.StableUid) directly.
| if (uidFilterSet is not null | ||
| && !uidFilterSet.Contains($"{test.DeclaringType!.FullName}.{test.Name}")) | ||
| { |
There was a problem hiding this comment.
In this sample, uidFilterSet is a HashSet<TestNodeUid> but Contains is called with an interpolated string, which relies on the implicit string -> TestNodeUid conversion and allocates a new TestNodeUid per check. If you want this to be allocation-free, consider storing uid.Value in a HashSet<string> (e.g., with StringComparer.Ordinal) and comparing against the computed UID string.
BFSTestNodeVisitorwas performing an O(n)Any()linear scan overTestNodeUidListFilter.TestNodeUidsfor every test node visited, resulting in O(n×m) total work (n = test nodes, m = filter UIDs). This is hit on every retry/re-run viaRetryExecutionFilterFactory.Changes
BFSTestNodeVisitor.VisitAsync: Precompute aHashSet<PlatformTestNodeUid>fromTestNodeUidListFilter.TestNodeUidsonce before the BFS loop; replaceAny()withHashSet.Contains()for O(1) per-node lookup. Adds ausingalias to disambiguate from the framework-internalTestNodeUidtype.TestingPlatformExplorer/TestingFramework.cs(sample): Same fix — extract theHashSetoutside theforeachand useContainsinstead ofAny.Warning
Firewall rules blocked me from connecting to one or more addresses (expand for details)
I tried to connect to the following addresses, but was blocked by firewall rules:
4m6vsblobprodcus384.vsblob.vsassets.io.dotnet/dotnet .dotnet/dotnet build samples/public/TestingPlatformExamples/TestingPlatformExplorer/TestingPlatformExplorer.csproj(dns block)7k6vsblobprodcus337.vsblob.vsassets.io.dotnet/dotnet .dotnet/dotnet build samples/public/TestingPlatformExamples/TestingPlatformExplorer/TestingPlatformExplorer.csproj(dns block)jrqvsblobprodcus343.vsblob.vsassets.io.dotnet/dotnet .dotnet/dotnet build src/Adapter/MSTest.Engine/MSTest.Engine.csproj --no-restore(dns block)pc2vsblobprodcus360.vsblob.vsassets.io.dotnet/dotnet .dotnet/dotnet build samples/public/TestingPlatformExamples/TestingPlatformExplorer/TestingPlatformExplorer.csproj(dns block)If you need me to access, download, or install something from one of these locations, you can either: