Skip to content

[Efficiency Improver] perf: reduce allocations in BFSTestNodeVisitor path tracking and WrappedName building#7836

Merged
Evangelink merged 3 commits into
mainfrom
efficiency/bfstestnode-string-path-tracking-f474c342a1a32973
Apr 27, 2026
Merged

[Efficiency Improver] perf: reduce allocations in BFSTestNodeVisitor path tracking and WrappedName building#7836
Evangelink merged 3 commits into
mainfrom
efficiency/bfstestnode-string-path-tracking-f474c342a1a32973

Conversation

@Evangelink
Copy link
Copy Markdown
Member

@Evangelink Evangelink commented Apr 25, 2026

  • Replace StringBuilder in the BFS queue with string, eliminating one StringBuilder allocation per test node during tree traversal. Each node's full path is now an immutable string shared directly with child nodes as their base path.

  • Replace HashSet with HashSet for UID filter lookups. This avoids allocating a temporary PlatformTestNodeUid wrapper object on every node lookup in the HashSet.

  • Simplify CreateWrappedName in TestArgumentsManager to a conditional string.Concat, removing a StringBuilder allocation per argument entry.

Proxy metric: heap allocations per test node during BFS traversal. Before: 1 StringBuilder + 1 string per node (path) + 1 PlatformTestNodeUid
per node (filter lookup).
After: 1 string per node (path), no PlatformTestNodeUid per lookup.

Fixes #7828

…teWrappedName

- Replace StringBuilder in the BFS queue with string, eliminating one
  StringBuilder allocation per test node during tree traversal. Each
  node's full path is now an immutable string shared directly with
  child nodes as their base path.

- Replace HashSet<PlatformTestNodeUid> with HashSet<string> for UID
  filter lookups. This avoids allocating a temporary PlatformTestNodeUid
  wrapper object on every node lookup in the HashSet.

- Simplify CreateWrappedName in TestArgumentsManager to a conditional
  string.Concat, removing a StringBuilder allocation per argument entry.

Proxy metric: heap allocations per test node during BFS traversal.
Before: 1 StringBuilder + 1 string per node (path) + 1 PlatformTestNodeUid
  per node (filter lookup).
After: 1 string per node (path), no PlatformTestNodeUid per lookup.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 25, 2026 20:17
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reduces heap allocations in MSTest.Engine’s test-node discovery/execution pipeline by simplifying per-node path tracking in BFSTestNodeVisitor and by removing StringBuilder usage in argument display-name wrapping.

Changes:

  • Replace per-node StringBuilder path propagation in BFSTestNodeVisitor with immutable string path tracking.
  • Avoid per-lookup PlatformTestNodeUid wrapper allocations by switching UID filter membership checks to HashSet<string>.
  • Simplify TestArgumentsManager.CreateWrappedName to use string.Concat instead of StringBuilder.
Show a summary per file
File Description
src/Adapter/MSTest.Engine/Engine/BFSTestNodeVisitor.cs Removes per-node StringBuilder allocations in BFS traversal and reduces UID-filter lookup allocations by using string values directly.
src/Adapter/MSTest.Engine/Engine/TestArgumentsManager.cs Removes StringBuilder allocation from wrapped-name formatting for argument fragments.

Copilot's findings

  • Files reviewed: 2/2 changed files
  • Comments generated: 2

Comment thread src/Adapter/MSTest.Engine/Engine/BFSTestNodeVisitor.cs
Comment thread src/Adapter/MSTest.Engine/Engine/BFSTestNodeVisitor.cs Outdated
…deVisitor

Agent-Logs-Url: https://github.com/microsoft/testfx/sessions/c99d957f-6c20-4942-988b-5c1977ecef1d

Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
…nd cache PathSeparator as string field

Agent-Logs-Url: https://github.com/microsoft/testfx/sessions/eb488e40-99da-4224-81ee-95f9c0e4dc4b

Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 26, 2026 15:57
@Evangelink Evangelink review requested due to automatic review settings April 26, 2026 15:57
@JanKrivanek
Copy link
Copy Markdown
Member

Btw. PR description got likely half eaten out

@Evangelink Evangelink merged commit d7064a8 into main Apr 27, 2026
24 checks passed
@Evangelink Evangelink deleted the efficiency/bfstestnode-string-path-tracking-f474c342a1a32973 branch April 27, 2026 07:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Efficiency Improver] perf: reduce allocations in BFSTestNodeVisitor path tracking and WrappedName building

4 participants