perf: avoid Cast().FirstOrDefault() iterator alloc in CastHelper (#6029) by thomhurst · Pull Request #6074 · thomhurst/TUnit (original) (raw)

Summary

• Replace enumerable.Cast<object>().FirstOrDefault() in CastHelper.TryAotSafeConversion with a manual foreach that breaks after the first element, eliminating the OfTypeIterator + closure allocations per argument conversion.
• Hot path: runs on every [Arguments] value flowing through the AOT-safe conversion when the source is IEnumerable and the target isn't.

Closes #6029

Test plan

• dotnet build TUnit.Core/TUnit.Core.csproj -c Release succeeds across all TFMs.
• CI green.

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.

[](/apps/claude)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Summary: This is a focused, correct micro-optimization. The change is solid — here's the full breakdown.

What the change does

Replaces:

var firstElement = enumerable.Cast().FirstOrDefault();

with:

object? firstElement = null; foreach (var e in enumerable) { firstElement = e; break; }

Correctness ✅

Behaviorally identical in all cases:

• Empty enumerable → both yield null; the if (firstElement != null) guard below prevents recursion. ✅
• Value-type elements → both box to object (since enumerable is non-generic IEnumerable). ✅
• Cast<object>() cannot throw InvalidCastException since every value is already assignable to object, so dropping it loses no safety net. ✅
• Enumerator disposal → foreach compiles to a try/finally that calls Dispose() when IDisposable is implemented, matching the behavior of FirstOrDefault(). ✅

Performance gain ✅

Enumerable.Cast<TResult>() allocates a CastIterator<object> (a heap-allocated wrapper) before FirstOrDefault() can even start iterating. The manual foreach calls GetEnumerator() directly on the underlying IEnumerable, eliminating that extra allocation. Since this path is hit for every [Arguments] value that flows through as an IEnumerable with a non-IEnumerable target, the gain compounds quickly under test-suite load.

Minor observation (non-blocking)

The variable name e is a bit terse for a code path that may be revisited — item or element would be slightly more self-documenting. That said, the break immediately after assignment makes the intent obvious anyway, so this is trivial.

Verdict

The optimization is real, the implementation is correct, and the scope is appropriately tight. No issues found.

Code Review

Summary: This is a focused, correct micro-optimization. The change is solid — here's the full breakdown.

What the change does

Replaces:

var firstElement = enumerable.Cast().FirstOrDefault();

with a manual foreach + break that avoids the LINQ iterator wrapper.

Correctness ✅

Behaviorally identical in all cases:

• Empty enumerable → both yield null; the if (firstElement != null) guard below prevents recursion.
• Value-type elements → both box to object (since enumerable is non-generic IEnumerable).
• Cast<object>() cannot throw InvalidCastException since every value is assignable to object, so dropping it loses no safety net.
• Enumerator disposal → foreach compiles to a try/finally that calls Dispose() when IDisposable is implemented, matching FirstOrDefault() behavior.

Performance gain ✅

Enumerable.Cast<TResult>() allocates a CastIterator<object> (heap-allocated wrapper) before FirstOrDefault() can start iterating. The manual foreach calls GetEnumerator() directly on the underlying IEnumerable, eliminating that extra allocation. Since this path runs for every [Arguments] value flowing through as an IEnumerable with a non-IEnumerable target, the gain compounds across large test suites.

Minor observation (non-blocking)

The variable name e is a bit terse — item or element would be slightly more self-documenting. That said, the break immediately after makes the intent obvious, so this is genuinely trivial.

Verdict

Correct, focused, and the optimization is real. No issues found. 👍

Addresses review nit on #6074 — clearer loop variable name.

[](/apps/claude)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Verdict: LGTM — no bugs found.

What the diff does

Replaces enumerable.Cast<object>().FirstOrDefault() with a manual foreach + break in the AOT-safe conversion hot path. This eliminates the CastIterator<object> heap allocation that LINQ's Cast<T>() creates internally, at the cost of six extra lines.

Correctness analysis

The change is semantically equivalent for every reachable input:

Enumerator disposal is also preserved: foreach compiles to a try/finally that calls Dispose() on the IEnumerator if it implements IDisposable, matching the FirstOrDefault() path.

Minor observation

The PR description says "eliminating the OfTypeIterator + closure allocations", but the original code uses Cast<object>(), which internally creates a CastIterator<object> (not OfTypeIterator — that's OfType<T>()). No impact on correctness; just a small inaccuracy in the description.

thomhurst deleted the perf/6029-castelper-firstordefault branch

May 28, 2026 18:22

This was referenced

May 29, 2026

This was referenced

Jun 14, 2026

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

[ Show hidden characters]({{ revealButtonHref }})