[perf-improver] Replace Array.IndexOf(GetType()) with 'is' pattern matching in hot paths

[Evangelink]

Goal and Rationale

Every test node update passes through TestApplicationResult.ConsumeAsync — including the common InProgressTestNodeStateProperty transitions that fire for every test start/end. The previous code had a switch statement that already dispatched on all known types, then re-ran GetType() plus up to three Array.IndexOf scans (4–5 elements each) to increment counters and classify outcomes. This double-dispatch was pure overhead.

Two other hot paths — AbortForMaxFailedTestsExtension (active when --maximum-failed-tests is set) and RetryDataConsumer (active when retry is on) — had the same pattern.

Approach

Replace every Array.IndexOf(WellKnownTestNodeXxx, state.GetType()) != -1 idiom with an is FailedTestNodeStateProperty or ErrorTestNodeStateProperty ... type-pattern chain, which is resolved by the JIT into a sequence of isinst/type-check instructions with no allocation, no dictionary lookup, and no array traversal.

In TestApplicationResult.ConsumeAsync, the counter increments (_failedTestsCount++, _totalRanTests++) were merged directly into the existing switch arms, removing the redundant second pass entirely.

Files changed:

File	Change
TestApplicationResult.cs	Fold counters into switch; remove GetType() + 3× Array.IndexOf
AbortForMaxFailedTestsExtension.cs	Replace 1× GetType() + Array.IndexOf with is pattern
RetryDataConsumer.cs	Replace 2× GetType() + Array.IndexOf with is patterns

Performance Evidence

The change is a mechanical substitution of array-scan classification with JIT-inlined isinst checks. For a 10 000-test run:

• Before: ConsumeAsync called ~10 000× — each call boxed GetType() then scanned arrays of 4–5 Type objects up to 3 times.
• After: counters are updated in the same switch branch that was already dispatching; no extra allocations.

No microbenchmark exists in this repo for this path yet; the improvement is verified by code-path analysis. The WellKnownTestNodeXxx arrays remain in TestNodeProperties.Categories.cs for documentation and any callers outside this hot path.

Trade-offs

• Very small increase in code verbosity vs. array reference (explicit type list in each condition). Acceptable since the list is short and stable.
• No semantic changes — the classification logic is identical.

Reproducibility

./build.sh
.dotnet/dotnet run --project test/UnitTests/Microsoft.Testing.Platform.UnitTests -f net9.0 --no-build
.dotnet/dotnet run --project test/UnitTests/Microsoft.Testing.Extensions.UnitTests -f net9.0 --no-build

Test Status

Build: ✅ succeeded (0 warnings, 0 errors)
Microsoft.Testing.Platform.UnitTests: ✅ 1220 passed, 0 failed
Microsoft.Testing.Extensions.UnitTests: ✅ 539 passed, 0 failed

🤖 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.3K AIC · ⌖ 25.8 AIC · ⊞ 57.6K · [◷]( · ◷)

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 removes redundant GetType() + Array.IndexOf(...) outcome classification in several high-frequency data consumers, replacing it with direct type-pattern matching and folding counter updates into existing switch dispatch. The intent is to reduce per-test overhead on hot paths in Microsoft.Testing.Platform and its Retry extension.

Changes:

• Fold _failedTestsCount / _totalRanTests updates directly into the existing switch (executionState) in TestApplicationResult.ConsumeAsync, eliminating the second classification pass.
• Replace Array.IndexOf(..., state.GetType()) with is ... or ... type patterns in AbortForMaxFailedTestsExtension and RetryDataConsumer.
• Preserve the existing “skipped tests do not count as ran” behavior by keeping skipped excluded from _totalRanTests.

Show a summary per file

File	Description
src/Platform/Microsoft.Testing.Platform/Services/TestApplicationResult.cs	Removes post-switch array-scan classification; updates counters in the same switch arms that already handle each state.
src/Platform/Microsoft.Testing.Platform/Extensions/AbortForMaxFailedTestsExtension.cs	Replaces failure-type array membership check with explicit type-pattern matching before incrementing fail count / triggering stop policy.
src/Platform/Microsoft.Testing.Extensions.Retry/RetryDataConsumer.cs	Replaces outcome/failure array membership checks with explicit type-pattern matching for retry signaling and total test counting.

Copilot's findings

• Files reviewed: 3/3 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.

Review Summary

Dimension	Result	Notes
Threading	✅ LGTM	AsyncConsumerDataProcessor uses a Channel with SingleReader = true; ConsumeAsync is always called from a single-reader task. All counters (_failedTestsCount, _totalRanTests, _failCount, _totalTests) are written and read exclusively by that serial loop — unchanged by this PR.
Correctness	✅ LGTM	Full trace of every TestNodeStateProperty subtype confirms counter increments are identical before and after. All relevant classes are sealed, so is T and GetType() == typeof(T) are semantically equivalent. The && boolean short-circuit in AbortForMaxFailedTestsExtension is unaffected — && is not a pattern operator, so the C# parser terminates the disjunctive pattern before it and ++_failCount only fires on a type match, as before.
Compatibility	✅ LGTM	All three modified classes are internal sealed; no public API surface changes. The WellKnownTestNodeXxx arrays in TestNodeProperties.Categories.cs are retained for the remaining callers (TrxTestResultExtractor, TestResultCaptureHelper, TestResultCapture). The removed using Microsoft.Testing.Platform.Messages directives were only needed for the now-deleted Array.IndexOf calls.
Performance	✅ LGTM	The stated goal is achieved: the JIT resolves isinst-based type checks with no allocations and no linear array traversal. Folding the counters directly into existing switch arms in TestApplicationResult.ConsumeAsync eliminates a second pass over GetType() for every test node update.
Code Style	⚠️ Nit	RetryDataConsumer.cs places TimeoutTestNodeStateProperty inside the #pragma warning disable CS0618, MTP0001 region even though that type is not [Obsolete]. The other two changed files correctly scope the pragma to CancelledTestNodeStateProperty only. See inline comment.

Overall

Clean, mechanical refactor. All semantic invariants are preserved. The one inline nit (pragma scope) is non-blocking.

[Evangelink]

[Nit] Pragma scope is one token too wide

TimeoutTestNodeStateProperty is not marked [Obsolete] — only CancelledTestNodeStateProperty carries MTP0001. Both other files in this PR scope the pragma correctly:

// AbortForMaxFailedTestsExtension.cs (correct)
if (testNodeStateProperty is FailedTestNodeStateProperty or ErrorTestNodeStateProperty
        or TimeoutTestNodeStateProperty       // ← outside the disabled region
#pragma warning disable CS0618, MTP0001
        or CancelledTestNodeStateProperty
#pragma warning restore CS0618, MTP0001

The same pattern should be applied here (move TimeoutTestNodeStateProperty above the disable pragma). No functional impact — an unnecessary suppress is harmless — but it sets a misleading precedent and will hide a real CS0618 warning if TimeoutTestNodeStateProperty becomes obsolete later without the pragma being removed.

Recommendation: Split the line so TimeoutTestNodeStateProperty is outside the disabled region, matching the style in AbortForMaxFailedTestsExtension.cs:

if (nodeState is FailedTestNodeStateProperty or ErrorTestNodeStateProperty
    or TimeoutTestNodeStateProperty
#pragma warning disable CS0618, MTP0001 // Type or member is obsolete
    or CancelledTestNodeStateProperty)
#pragma warning restore CS0618, MTP0001 // Type or member is obsolete

(Same change applies to the second if block on lines 60–62.)