[perf-improver] perf: add GetLongestRunningTask to avoid list allocation in SimpleTerminal#8932
Draft
Evangelink wants to merge 1 commit into
Draft
Conversation
…minal SimpleTerminalBase.RenderProgress called GetRunningTasks(1).FirstOrDefault() which allocated a new List<TestDetailState>, populated it from the dictionary, sorted it (O(n log n)), and returned the list only to take the first element. Add GetLongestRunningTask(): an O(n) linear scan over the ConcurrentDictionary that returns the task with the maximum elapsed time directly, with zero heap allocations. Update SimpleTerminalBase to call it instead. Impact: eliminates 1 List<TestDetailState> allocation per progress item per render tick in the non-ANSI (simple/redirected) terminal path. For a 5-minute run with N=4 assemblies at ~5 fps, this saves ~6,000 list allocations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR targets Microsoft.Testing.Platform terminal progress rendering, optimizing the non-ANSI/redirected-output path by avoiding an allocation + full sort when only a single “most active” running test is needed.
Changes:
- Added
TestNodeResultsState.GetLongestRunningTask()to find the max elapsed running test via a single linear scan (no list allocation). - Updated
SimpleTerminal.RenderProgressto useGetLongestRunningTask()instead ofGetRunningTasks(1).FirstOrDefault().
Show a summary per file
| File | Description |
|---|---|
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TestNodeResultsState.cs | Adds a new O(n) API to retrieve the longest-running active test without allocating/sorting a list. |
| src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/SimpleTerminalBase.cs | Switches SimpleTerminal progress rendering to the new API to remove per-tick allocations in redirected/non-ANSI mode. |
Copilot's findings
- Files reviewed: 2/2 changed files
- Comments generated: 1
| } | ||
|
|
||
| TestDetailState? activeTest = p.TestNodeResultsState?.GetRunningTasks(1).FirstOrDefault(); | ||
| TestDetailState? activeTest = p.TestNodeResultsState?.GetLongestRunningTask(); |
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.
🤖 This is an automated contribution from Perf Improver.
Goal and Rationale
SimpleTerminalBase.RenderProgress(the non-ANSI / redirected-output terminal path) needs the single longest-running active test for each progress item. The current call:calls
GetRunningTasks(1)which:List<TestDetailState>with capacity_testNodeProgressStates.CountConcurrentDictionaryto populate the listFirstOrDefault()takes only the first elementThe list is immediately eligible for GC. This is 1 wasted
List<TestDetailState>allocation per progress item per render tick, plus an unnecessary full sort.Approach
Add
GetLongestRunningTask()toTestNodeResultsState: a single O(n) linear scan that tracks the maximum-elapsed entry directly, returning theTestDetailState?with no heap allocation.Update
SimpleTerminalBase.RenderProgressto call it instead.The existing
GetRunningTasks(int maxCount)is unchanged — it's still needed byAnsiTerminalTestProgressFrame.GenerateLinesToRenderwhich requests multiple items.Performance Evidence
List<TestDetailState>allocs per progress item per render tick (SimpleTerminal)For a 5-minute run with N=4 assemblies at ~5 fps:
List<TestDetailState>allocations eliminatedMethodology: code inspection + allocation analysis. The
SimpleTerminalpath is used when output is redirected (CI pipelines, log capture) or when ANSI is not available.Trade-offs
GetLongestRunningTask()is a strict simplification: the "sort then take first" pattern is overkill for a single-element request.null(same asFirstOrDefault()on an empty list).Test Status
Microsoft.Testing.Platform.UnitTests(net8.0): 1103 passed, 0 failed, 3 skipped ✅Reproducibility
Add this agentic workflows to your repo
To install this agentic workflow, run