[efficiency-improver] perf: pre-allocate List(T) capacity in DiscoveryResultCache and TestRunCache

[Jakub Jareš (nohwnd)]

Goal and Rationale

Eliminate repeated transient array allocations that occur every time DiscoveryResultCache and TestRunCache flush a batch. Both caches previously re-created their backing collections with zero capacity, causing the List<T> runtime to resize 0→4→8→16 on every new batch — that's 3 intermediate array allocations and 2 copy operations before the list reaches capacity for the default batch size of 10.

Focus area: Code-Level Efficiency

---

Approach

1. DiscoveryResultCache — replaced new List<TestCase>() (no capacity) with new List<TestCase>(InitialCapacity(_cacheSize)) in the constructor and after each flush.

2. TestRunCache — switched _inProgressTests and _testResults from Collection<T> (a virtual-method wrapper over List<T>) to plain List<T> with pre-allocated capacity. Updated all three allocation sites: constructor, SendResults(), and GetLastChunk().

3. Removed the now-unused using System.Collections.ObjectModel from TestRunCache.cs.

4. private static int InitialCapacity(long cacheSize) — helper capped at 512 to avoid over-allocation when users configure extreme batch sizes.

---

Energy Efficiency Evidence

Proxy metric used: GC allocation count / memory pressure → directly maps to CPU energy via fewer GC mark/sweep cycles and fewer cache-line evictions from transient objects.

Measurement methodology:

With DefaultBatchSize = 10 (the default configured in Constants.cs):

Collection op	Before	After
Internal array allocations per batch	3 (at 4, 8, 16 elements)	0 (pre-sized to 10)
Copy operations per batch	2	0
Collection<T> virtual method overhead (Add)	✓ per call	✗ eliminated

For a 10 000-test run (≈1 000 batches × 3 caches = 3 000 flushes):

• Saves ≈ 9 000 transient array allocations (3 per flush × 3 000 flushes)
• Eliminates 20 000+ virtual dispatch calls (Collection<T>.Add wraps List<T>.Add via override)

These are dotnet test hot-path calls — OnNewTestResult and AddTest are invoked once per test case.

---

Green Software Foundation Context

• Hardware Efficiency: fewer GC allocations → less DRAM refresh pressure and fewer L1/L2 cache evictions from short-lived objects. The GC itself consumes non-trivial CPU cycles during each collection.
• SCI (Software Carbon Intensity): every dotnet test invocation at ecosystem scale (millions of CI runs per day across the .NET ecosystem) benefits from reduced per-test CPU work.
• Energy Proportionality: removing overhead allocations makes CPU work more proportional to actual test execution rather than incidental bookkeeping.

---

Trade-offs

• Pre-allocating 10 slots (default) adds a trivial ~80 bytes per batch object. This is a net win vs. the 3 transient arrays it replaces.
• The 512-element cap prevents excessive pre-allocation for users with very large batch sizes.
• Collection<T> → List<T> is a private field change; all public API still returns ICollection<T>.

---

Reproducibility

# Build and run CrossPlatEngine + related unit tests
./build.sh
./test.sh -p CrossPlatEngine

Test Status

✅ Build succeeded (0 errors, 0 new warnings)
✅ All TestRunCache and DiscoveryResultCache unit tests passed

Generated by Efficiency Improver · 3.5K AIC · ⌖ 26.9 AIC · ⊞ 45.5K · ◷

[Copilot]

Pull request overview

This PR improves execution/discovery hot-path efficiency in CrossPlatEngine by pre-allocating list capacities in DiscoveryResultCache and TestRunCache, reducing repeated internal List<T> growth allocations during batching/flush operations.

Changes:

• Pre-allocate List<TestCase> capacity in DiscoveryResultCache (constructor + post-flush reset).
• Replace Collection<T> buffers with List<T> buffers in TestRunCache and pre-allocate capacity across constructor/flush/reset paths.
• Add InitialCapacity(long cacheSize) helpers (capped at 512) and remove the now-unused System.Collections.ObjectModel using.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File	Description
src/Microsoft.TestPlatform.CrossPlatEngine/Execution/TestRunCache.cs	Switch internal buffers to pre-sized List<T> and add a capped InitialCapacity helper.
src/Microsoft.TestPlatform.CrossPlatEngine/Discovery/DiscoveryResultCache.cs	Pre-size discovery buffer list and reset with a capped initial capacity after flush.

[Jakub Jareš (nohwnd)]

Expert Review — DiscoveryResultCache and TestRunCache pre-allocation

Activated dimensions (files in src/Microsoft.TestPlatform.CrossPlatEngine/): Parallel Execution & Scheduling Safety, Performance & Allocations, Public API Surface Protection.

---

✅ Parallel Execution & Scheduling Safety

The collection-swap pattern is safe. In both files, the old list is passed to the callback before the field is reassigned to the new pre-allocated list. The lock is held across the swap, so no concurrent reader can observe a partially-replaced state. Monitor.Enter in C# is re-entrant (same-thread), so the nested lock (_syncObject) inside TestRunStatistics called from SendResults() does not deadlock.

✅ Public API Surface

Only private fields were changed (ICollection<T> → List<T> for _inProgressTests and _testResults). All public properties still return ICollection<T>. No public surface area was affected.

✅ Algorithmic Correctness

InitialCapacity(long cacheSize) => (int)Math.Min(cacheSize, 512) is correct for all valid inputs. Since cacheSize > 0 is enforced by TPDebug.Assert, Math.Min always returns a positive value ≤ 512, making the int cast safe. The constructor-side and post-flush-side allocations are symmetric and correct.

⚠️ Performance — one minor issue (see inline comment)

The GetLastChunk() replacement list is pre-allocated with InitialCapacity(_cacheSize) capacity even though GetLastChunk() is the end-of-run drain and that capacity will likely never be consumed. Using new List<TestResult>() there avoids an unnecessary array allocation in the cold path. See the inline comment at line 274.

🔍 Observation: _inProgressTests capacity semantics

_inProgressTests is pre-allocated to InitialCapacity(_cacheSize) on each flush in SendResults(). Its actual utilization is bounded by the runner's parallelism level (typically 1 for sequential, up to MaxConcurrency for parallel), not by _cacheSize. For sequential test runs the list reaches at most 1–2 elements per batch, so pre-allocating min(cacheSize, 512) slots allocates more capacity than needed.

That said, the CheckForCacheHit() flush condition is (_testResults.Count + _inProgressTests.Count) >= _cacheSize, so in highly parallel runs _inProgressTests can legitimately approach _cacheSize. The 512 cap prevents extreme over-allocation. The PR description acknowledges this as an acceptable ~80-byte trade-off per batch, and that reasoning holds for parallel runners. For sequential runners the overhead is trivial and bounded.

PR Description Alignment ✅

The description accurately describes all changes: the two allocation sites in DiscoveryResultCache, the three allocation sites in TestRunCache, the using removal, and the InitialCapacity helper with its 512 cap. The energy-efficiency framing is consistent with the actual diff scope.

---

🧠 Reviewed by expert-reviewing workflow · Dimensions: Parallel Execution & Scheduling Safety · Performance & Allocations · Public API Surface Protection

🧠 Reviewed by Expert Code Reviewer 🧠

[Copilot]

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.