perf: collapse OfType chains in JUnitXmlWriter (#6052) by thomhurst · Pull Request #6070 · thomhurst/TUnit (original) (raw)

Code Review

No issues found. This is a clean, correct micro-optimization.

What the PR does

Collapses three independent LINQ chains into a single loop in and , eliminating up to 3 iterator allocations and up to 3 full traversals of per test node.

Correctness analysis

The only structural risk in this pattern is the chain: on a single loop iteration, at most one variable can be assigned. In theory, if a property object were simultaneously a subtype of two target types (e.g., both a and a ), the old three-pass code would assign it to both variables while the new code would only assign it to the first matching branch.

That risk is fully mitigated here:

• Disjoint hierarchies. All three target types (, , ) are independent of each other — no concrete type in implements more than one of them. and have no subclasses at all; all subtypes are .
• Null safety. never returns and throws, so the loop cannot encounter null elements.
• ** internal structure.** is stored in a dedicated private field and always emitted last by the enumerator. This means timing and method identifier are always encountered before state, and the branches for those two types always fire correctly.
• No TUnit-defined cross-type properties. A grep across the entire codebase confirms no TUnit class inherits from any of these three platform types.

The early-break optimization is also correct: it only fires once all three (or two, in ) variables are assigned, which requires they were all found — no premature exit possible.

Summary

Behavior is semantically identical to the original, the change is safe, and the performance improvement is real. LGTM.