[perf-improver] perf: pool RenderedProgressItem instances in AnsiTerminalTestProgressFrame

[Evangelink]

Goal and Rationale

Each render tick (~2 fps) of the ANSI terminal progress display previously created a new RenderedProgressItem object for every rendered line — one per assembly progress row and one per each running-test detail row.

With 5 assemblies running concurrently and 2 detail lines each, that is 15 allocations per tick × 2 fps × 300 s ≈ 9,000 short-lived object allocations per 5-minute run. At higher assembly/detail counts the numbers scale linearly.

Approach

Replace the per-tick new RenderedProgressItem(id, version) + List.Add(...) pattern with a pre-allocated pool array per AnsiTerminalTestProgressFrame instance:

Before	After
new RenderedProgressItem(id, version) on every rendered line	GetOrAllocateNextSlot() — returns an existing slot, grows array only when the high-water mark is exceeded
List<RenderedProgressItem>? RenderedLines (null-checked, .Count, .Add, .Clear())	RenderedProgressItem[] _renderedLines (pool) + int RenderedLinesCount
RenderedLines?.Clear() in Reset()	RenderedLinesCount = 0 — zeroes the count; pooled objects remain alive and are reused
RenderedProgressItem(long id, long version) constructor	Reset(long id, long version) method on a reusable instance

The pool array is never shrunk, so it reaches steady state at the high-water mark of simultaneous rendered lines (bounded by terminal height × 0.7) and allocates nothing thereafter.

AnsiTerminal.EraseProgress() is updated to use RenderedLinesCount instead of the removed RenderedLines property.

Performance Evidence

Scenario	Before	After
5 assemblies × 2 detail lines × 2 fps × 5 min	~9,000 RenderedProgressItem allocs	0 (after first tick)
10 assemblies × 3 detail lines × 2 fps × 5 min	~18,000 RenderedProgressItem allocs	0 (after first tick)

Methodology: static code analysis — every code path through Render() that produces a rendered line now calls GetOrAllocateNextSlot() instead of new RenderedProgressItem(...).

Trade-offs

• The RenderedProgressItem.ProgressId and RenderedProgressItem.ProgressVersion properties change from { get; } (readonly) to { get; private set; } (mutable via internal Reset). The class remains internal sealed, so this has no API-surface impact.
• Pooled instances live as long as the AnsiTerminalTestProgressFrame that owns them. Frames are long-lived (double-buffer swap), so this is at most 2 × Height * 0.7 ≈ 24 live RenderedProgressItem objects — negligible.

Reproducibility

dotnet-trace collect -- artifacts/bin/Microsoft.Testing.Platform.UnitTests/Debug/net8.0/Microsoft.Testing.Platform.UnitTests
# Compare Microsoft.Testing.Platform.OutputDevice.Terminal.AnsiTerminalTestProgressFrame+RenderedProgressItem allocation counts

Test Status

✅ Build: 0 warnings, 0 errors (all TFMs).
✅ Unit tests: 1125 passed, 0 failed, 3 skipped (net8.0).

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Perf Improver workflow. · 2.1K AIC · ⌖ 38.7 AIC · [◷]( · ◷)

Add this agentic workflows to your repo

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/perf-improver.md@main

[Copilot]

Pull request overview

This PR reduces per-tick heap allocations in the ANSI terminal live progress renderer in Microsoft.Testing.Platform by pooling RenderedProgressItem instances within AnsiTerminalTestProgressFrame, reusing them across render ticks instead of allocating new objects for each rendered line.

Changes:

• Replaced per-render new RenderedProgressItem(...) + List.Add(...) with a reusable _renderedLines pool and RenderedLinesCount high-watermark tracking.
• Converted RenderedProgressItem from constructor-initialized (immutable ID/version) to reusable via Reset(id, version) and mutable ProgressId/ProgressVersion.
• Updated AnsiTerminal.EraseProgress() to use RenderedLinesCount rather than the removed RenderedLines list.

Show a summary per file

File	Description
src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/AnsiTerminalTestProgressFrame.cs	Introduces pooled rendered-line tracking and reuses RenderedProgressItem instances across render ticks.
src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/AnsiTerminal.cs	Adjusts progress erasure logic to rely on RenderedLinesCount after the rendered-lines storage refactor.

Copilot's findings

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

[Evangelink]

Note

🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.

✅ 22/22 dimensions clean — no findings.

Full dimension-by-dimension analysis

Changed files (src/Platform/ hotspot — priority: Threading, Performance, IPC Wire, Security, API Surface):

• AnsiTerminal.cs — 4 lines changed (EraseProgress)
• AnsiTerminalTestProgressFrame.cs — 71 lines changed (pool introduction, RenderedProgressItem refactor)

---

1. Algorithmic Correctness — LGTM

Traced through GetOrAllocateNextSlot() exhaustively:

• Pre-increment idx = RenderedLinesCount++ then checks (uint)idx >= (uint)_renderedLines.Length. Since idx is always non-negative the unsigned cast is safe and correctly detects both idx == Length and (unreachable) negative cases.
• Growth: Length == 0 → 8, else Length * 2; the if (newSize <= idx) fallback handles the theoretical integer-overflow case (Length * 2 overflows to negative); in practice impossible for terminal line counts.
• _renderedLines[idx] ??= new RenderedProgressItem() lazily allocates; subsequent calls reuse the same object.
• Post-Reset(), RenderedLinesCount = 0 means no slot beyond index −1 is accessible; stale pool items above the watermark are never reached through the count guard.
• previousFrame._renderedLines[i] is accessed only when previousFrame.RenderedLinesCount > i; since RenderedLinesCount ≤ _renderedLines.Length is invariant (maintained by GetOrAllocateNextSlot), the index is always in-bounds.
• Private-field cross-instance access (previousFrame._renderedLines) is valid C# (private is per-class, not per-instance) and is correct here.
• The two separate if (item is TestProgressState) / if (item is TestDetailState) blocks are mutually exclusive in practice (both types are sealed-equivalent internals); exactly one GetOrAllocateNextSlot() call per loop iteration, keeping RenderedLinesCount and i in sync.
• RenderedProgressItem.Reset() zeroes all three fields; AppendTestWorkerProgress / AppendTestWorkerDetail immediately overwrite RenderedDurationLength, so no stale-duration comparison can occur.

2. Threading & Concurrency — LGTM

Progress rendering is driven by a single render timer callback on one thread; AnsiTerminal._currentFrame and _spareFrame are only read/written within RenderProgress() and EraseProgress(), both called from that same thread. No shared mutable state, no synchronisation concerns.

3. Security & IPC Contract Safety — LGTM

N/A — change is confined to ANSI terminal rendering, no file I/O, serialization, or IPC boundary touched.

4. Public API & Binary Compatibility — LGTM

AnsiTerminalTestProgressFrame is internal sealed and RenderedProgressItem is internal sealed. The removed RenderedLines property and changed RenderedProgressItem constructor are all internal; no public API surface is affected. No PublicAPI.Unshipped.txt update required.

5. Performance & Allocations — LGTM

This is the purpose of the PR. The object-pool pattern correctly eliminates ~N per-tick heap allocations (one RenderedProgressItem per rendered line × render frequency). After the first few ticks the pool stabilises at the high-watermark size and GetOrAllocateNextSlot() never allocates again. Array.Resize is amortized O(1). RenderedLinesCount = 0 in Reset() is O(1) vs the old List.Clear(). The pool array itself is never shrunk, which is the explicit design intent documented in the comment block. Minor: initial capacity 8 vs the old progress.Length * 2 pre-allocation is a wash — the pool outlives individual render ticks so it pays for itself after one growth cycle.

6. Cross-TFM Compatibility — LGTM

Uses only Array.Resize, ??=, uint cast comparison, and standard auto-properties — all available on net462, netstandard2.0, net8.0, and net9.0.

7. Resource & IDisposable Management — LGTM

No IDisposable types introduced or removed.

8. Defensive Coding at Boundaries — LGTM

Change is wholly internal rendering infrastructure. No user-provided callbacks, no external trust boundary crossed.

9. Localization & Resources — LGTM

No user-facing strings modified.

10. Test Isolation — N/A

No test files changed.

11. Assertion Quality — N/A

No test files changed.

12. Flakiness Patterns — N/A

No test files changed.

13. Test Completeness & Coverage — LGTM

GetOrAllocateNextSlot() is private; the semantic behaviour it replaces (per-line tracking of ProgressId, ProgressVersion, RenderedDurationLength) is exercised by existing tests in TerminalTestReporterTests.cs via the full rendering pipeline. The change is semantically equivalent to the old new RenderedProgressItem(id, version) + List.Add pattern; existing tests continue to validate observable rendering output.

14. Data-Driven Test Coverage — N/A

No test files changed.

15. Code Structure & Simplification — LGTM

GetOrAllocateNextSlot() is a clean, self-contained helper with a single responsibility. Early-return / guard-clause patterns are used appropriately throughout Render(). The RenderedLinesCount = 0 idiom in Reset() and Clear() is clearer than the old null-guarded ?.Clear().

16. Naming & Conventions — LGTM

GetOrAllocateNextSlot is descriptive and matches the intent precisely. RenderedLinesCount is consistent with the existing Width/Height property naming on the same class. Reset(long id, long version) follows the existing Reset(int width, int height) pattern on the outer class.

17. Documentation Accuracy — LGTM

The new block comment above _renderedLines correctly describes the pool design and the high-watermark semantics. The XML doc on GetOrAllocateNextSlot() accurately describes both what it returns and the side-effect on RenderedLinesCount. The XML doc on Reset() accurately describes the reuse intent.

18. Analyzer & Code Fix Quality — N/A

No src/Analyzers/ files changed.

19. IPC Wire Compatibility — N/A

No serialization or IPC protocol code changed.

20. Build Infrastructure & Dependencies — N/A

No eng/ or project file changes.

21. Scope & PR Discipline — LGTM

Single, coherent concern: pool RenderedProgressItem instances in AnsiTerminalTestProgressFrame. No unrelated changes mixed in.

22. PowerShell Scripting Hygiene — N/A

No .ps1 files changed.

🤖 Automated content by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review (on PR ready) workflow. · 520.6 AIC · ⌖ 12.5 AIC · ◷