[efficiency-improver] perf: single-pass PropertyBag walk in JUnitReport TestResultCapture#9018
Draft
Evangelink wants to merge 1 commit into
Draft
Conversation
Replace 5 × SingleOrDefault<T>() + 1 × foreach/GetEnumerator() in TestResultCapture.TryCapture() with a single GetStructEnumerator() pass, saving 5 linked-list traversals and 1 IEnumerator<IProperty> heap allocation per terminal test result when --report-junit is enabled. Also update GetClassAndMethodName() to accept the already-collected TestMethodIdentifierProperty directly, removing the final redundant walk. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR optimizes Microsoft.Testing.Extensions.JUnitReport’s TestResultCapture.TryCapture() by consolidating multiple PropertyBag lookups into a single struct-enumerator traversal, reducing linked-list walks and avoiding IEnumerator<IProperty> boxing allocations on terminal test results.
Changes:
- Replaced multiple
SingleOrDefault<T>()calls andforeachenumeration with a singleGetStructEnumerator()pass collecting all needed properties. - Refactored
GetClassAndMethodName()to accept the already-collectedTestMethodIdentifierProperty, avoiding an additional property bag lookup. - Simplified stdout/stderr and trait collection to reuse the same traversal.
Show a summary per file
| File | Description |
|---|---|
| src/Platform/Microsoft.Testing.Extensions.JUnitReport/TestResultCapture.cs | Consolidates property extraction into a single struct-enumerator pass and updates method-name extraction to reuse collected data. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 1
Comment on lines
+42
to
+46
| // Single-pass collection of all required properties — replaces 5 × SingleOrDefault<T>() | ||
| // + 1 × foreach/GetEnumerator() with one zero-allocation GetStructEnumerator() pass, | ||
| // saving 5 linked-list traversals and 1 IEnumerator<IProperty> heap allocation per | ||
| // terminal test result. Singleton-typed properties use the local GetSingleOrDefaultValue | ||
| // helper to preserve the throw-on-duplicate invariant that SingleOrDefault<T>() provided; |
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 a draft PR created by Efficiency Improver, an automated AI assistant focused on reducing the energy consumption and computational footprint of this repository.
Goal and Rationale
Reduce redundant CPU work and heap allocations in
TestResultCapture.TryCapture()inMicrosoft.Testing.Extensions.JUnitReport, which runs on every completed test result when JUnit report generation (--report-junit) is enabled.Focus area: Code-Level Efficiency — eliminating unnecessary linked-list traversals and heap allocations per test result.
Approach
TestResultCapture.TryCapture()previously made 6 separate passes over thePropertyBaglinked list:SingleOrDefault<T>()(forTestNodeStateProperty,TimingProperty,TestMethodIdentifierProperty,StandardOutputProperty,StandardErrorProperty)foreach (IProperty p in node.Properties)— calls the publicGetEnumerator()which boxes the internal struct enumerator asIEnumerator<IProperty>(1 heap allocation)The fix replaces all of these with a single
GetStructEnumerator()pass using aswitchon the current node, collecting all required properties in one traversal.GetClassAndMethodName()is updated to accept the already-collectedTestMethodIdentifierPropertydirectly, removing the need for an internal walk.Energy Efficiency Evidence
Proxy metric used: CPU cycles / heap allocations per test result (maps directly to energy — fewer pointer dereferences and GC pressure = less power draw per test run).
IEnumerator<IProperty>heap allocations per test resultforeach)For a test run with 1 000 tests: ~50 000 pointer dereferences eliminated and ~1 000 heap allocations removed.
Reproducibility: Build
src/Platform/Microsoft.Testing.Extensions.JUnitReport— succeeded with 0 warnings, 0 errors.Green Software Foundation Context
Hardware Efficiency: Fewer repeated traversals of the same linked-list nodes reduces CPU cache pressure. Fewer cache misses → lower power draw per test result processed.
Software Carbon Intensity (SCI): Fewer instructions per functional unit (one test result processed) directly reduces the energy term in the SCI equation.
Trade-offs
The single-pass
switchblock is slightly more verbose than the previous sequence of namedSingleOrDefault<T>()calls. However, the pattern is now established in the codebase (seeDotnetTestDataConsumer,OpenTelemetryResultHandler,TrxTestResultExtractor) and is recognisable to contributors familiar with those files.Test Status
✅ Build succeeded (0 warnings, 0 errors) for
Microsoft.Testing.Extensions.JUnitReportAdd this agentic workflows to your repo
To install this agentic workflow, run