[efficiency-improver] perf: defer GetTestName() to failure branches and avoid OfType<> alloc in AzureDevOpsReporter

[Evangelink]

Goal and Rationale

Reduce CPU work and heap allocations in AzureDevOpsReporter.ConsumeAsync(), which runs on every TestNodeUpdateMessage during Azure DevOps–enabled test runs.

Focus area: Code-Level Efficiency — eliminating unnecessary PropertyBag traversals and heap allocations for the common case (passing tests).

Approach

Before

EnsureEnabledConfigurationLoaded();
TestNodeStateProperty? nodeState = nodeUpdateMessage.TestNode.Properties.SingleOrDefault<TestNodeStateProperty>();
string testDisplayName = nodeUpdateMessage.TestNode.DisplayName;
string testName = GetTestName(nodeUpdateMessage.TestNode);  // ← always called

switch (nodeState)
{
    case FailedTestNodeStateProperty:   // testName used
    case ErrorTestNodeStateProperty:    // testName used
    case CancelledTestNodeStateProperty: // testName used
    case TimeoutTestNodeStateProperty:  // testName used
    // No case for Passed/Skipped/Discovered/InProgress — testName computed but discarded
}

GetTestName() used OfType<SerializableKeyValuePairStringProperty>() which:

1. Walks the full PropertyBag linked list
2. Allocates a SerializableKeyValuePairStringProperty[] for every call

After

Two targeted changes:

1. Defer GetTestName() to failure branches only:

switch (nodeState)
{
    case FailedTestNodeStateProperty failed:
        await WriteExceptionAsync(testDisplayName, GetTestName(nodeUpdateMessage.TestNode), ...);
        break;
    // same for Error, Cancelled, Timeout
}

Passing/skipped/in-progress tests never call GetTestName().

2. Replace OfType<T>().FirstOrDefault(predicate) with struct-enumerator short-circuit in GetTestName():

PropertyBag.PropertyBagEnumerator enumerator = testNode.Properties.GetStructEnumerator();
while (enumerator.MoveNext())
{
    if (enumerator.Current is SerializableKeyValuePairStringProperty kvp
        && kvp.Key == FullyQualifiedNamePropertyKey)
        return kvp.Value;
}
return testNode.DisplayName;

No SerializableKeyValuePairStringProperty[] allocated; exits as soon as the key is found.

Energy Efficiency Evidence

Proxy metric: Heap allocations + linked-list node visits per test result (fewer pointer dereferences = less DRAM traffic = less CPU energy).

Metric	Before (passing test)	After (passing test)
PropertyBag walks	2 (state + GetTestName)	1 (state only)
SerializableKeyValuePairStringProperty[] alloc	1	0

Metric	Before (failing test)	After (failing test)
PropertyBag walks	2 (state + GetTestName via OfType)	2 (state + GetTestName via struct enumerator, early exit)
Array allocs	1	0

For 1 000 passing tests in a CI run with --report-azdo enabled:

• ~1 000 PropertyBag linked-list traversals eliminated
• ~1 000 SerializableKeyValuePairStringProperty[] heap allocations eliminated

Green Software Foundation Context

Energy Proportionality: The reporter now does work proportional to the number of failures, not the total number of tests. Most CI runs have zero or few failures; the energy cost of the reporter now scales with the rare case rather than the common case.

Hardware Efficiency: Fewer heap allocations = less GC pressure = less memory bandwidth consumed per test result processed.

Trade-offs

• GetTestName() is now called up to 4 times per failing test node (once per switch case) instead of once. In practice only one branch fires, so GetTestName() is still called at most once per update. No behavioural change.
• The switch branches are slightly more verbose, but the pattern (not computing values before checking their precondition) is standard C# idiom.

Reproducibility

DOTNET_INSTALL_DIR=/usr/share/dotnet ./build.sh
artifacts/bin/Microsoft.Testing.Extensions.UnitTests/Debug/net8.0/Microsoft.Testing.Extensions.UnitTests
# Expected: 129 tests pass

Test Status

✅ Build: 0 errors, 0 warnings
✅ Microsoft.Testing.Extensions.UnitTests: 129/129 passed

🤖 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 Efficiency Improver workflow. · 1.1K AIC · ⌖ 99.5 AIC · [◷]( · ◷)

Add this agentic workflows to your repo

To install this agentic workflow, run

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

[Copilot]

✅ Ready to approve

The change is a targeted, low-risk perf improvement that preserves existing behavior while reducing allocations and unnecessary work in the hot path.

Note: this review does not count toward required approvals for merging.

Pull request overview

This PR improves runtime efficiency in the Azure DevOps reporter path (AzureDevOpsReporter.ConsumeAsync) by avoiding unnecessary property-bag work and allocations for the common case (non-failing test updates) within the Microsoft.Testing.Platform Azure DevOps reporting extension.

Changes:

• Deferred GetTestName() computation so it only runs for failure/error/cancel/timeout test states.
• Reworked GetTestName() to use PropertyBag.GetStructEnumerator() with an early-exit loop instead of PropertyBag.OfType<T>() (which materializes an array).

File summaries

File	Description
src/Platform/Microsoft.Testing.Extensions.AzureDevOpsReport/AzureDevOpsReporter.cs	Defers test-name lookup to failure states and replaces OfType<T>() usage with a single-pass struct enumerator to reduce allocations.

Copilot's findings

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

Note

Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.

[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.

Review Summary

Verdict: COMMENT — no blocking or major issues.

Findings

Dimension	Result
Resource & IDisposable Management	NIT (see inline)
All other 15 applicable dimensions	✅ LGTM

---

Detailed Notes

Performance claims are accurate.
The PR description correctly identifies that PropertyBag.OfType<T>() is a custom method that eagerly materializes a TProperty[] array — not LINQ's lazy OfType<T>(). The old code therefore allocated a SerializableKeyValuePairStringProperty[] on every call, including for passing tests. The new struct-enumerator path allocates nothing and short-circuits on the first match. Both the deferral optimisation and the enumerator swap are genuine wins.

Semantics are preserved.
PropertyBagEnumerator walks the linked list in the same order as the old OfType<T>() + FirstOrDefault chain. The behaviour of GetTestName() is identical to the previous implementation.

Threading — clean.
GetTestName() is a pure static function over immutable TestNode data; PropertyBagEnumerator is a local struct. All await calls retain ConfigureAwait(false). No concurrency issues.

Cross-TFM — clean.
GetStructEnumerator() is already used by a dozen callers across src/Platform/ on all supported TFMs. No new API surface is required.

Missing using on PropertyBagEnumerator (NIT — inline comment on line 442).
PropertyBagEnumerator implements IDisposable via IEnumerator<IProperty>, and the early-return path does not call Dispose(). The struct's Dispose() only nulls two stack-local reference fields, so there is zero real resource impact. Flagged as NIT for consistency with DiscoveredTestsJsonSerializer.cs, the sole existing caller that correctly uses using.

No new tests added.
GetTestName() is a private static method with unchanged semantics; existing coverage is inherited through ConsumeAsync() call paths. Acceptable for a refactor of this kind.

🤖 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. · 359.8 AIC · ⌖ 12.4 AIC · ◷

[Copilot]

✅ Ready to approve

The changes are low-risk, preserve behavior, and improve the hot-path efficiency; only a minor comment wording nit was found.

Note: this review does not count toward required approvals for merging.

Copilot's findings

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

Note

Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.

[0101]

Automated safety check passed: no dangerous changes detected (no secrets, exfiltration, destructive ops, or suspicious dependencies). Approving as requested. Note: this is a quick safety sanity check, not a full code review.

[Copilot]

✅ Ready to approve

The changes are small, localized, and behavior-preserving, with only a minor comment-accuracy nit identified.

Note: this review does not count toward required approvals for merging.

Copilot's findings

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

Note

Your feedback helps us improve the quality of this feature.
Please use 👍 or 👎 to tell us whether this assessment is correct.