avoid some redundant enumerable alloc#4972
Conversation
There was a problem hiding this comment.
Code Review
This is a clean, well-targeted optimization. Eliminating the async/yield return wrappers that were purely forwarding elements is exactly the right call.
Why this is correct
Each async IAsyncEnumerable method backed by a yield return causes the C# compiler to generate a state machine class. When the wrapper does nothing but forward items, you're allocating that state machine per call for zero benefit. Returning the inner IAsyncEnumerable<T> directly is semantically equivalent here and avoids the allocation entirely — consistent with the project's "Performance First" mandate.
There's one subtle behavioral difference worth being aware of: in the old wrapper form, any synchronous exception thrown inside GenerateDataSourcesAsync before its first MoveNextAsync would be deferred to the enumeration phase. With the direct return, such an exception propagates immediately to the caller. In practice this is a non-issue here — GenerateDataSourcesAsync is itself an async iterator in derived classes and won't throw synchronously before returning the IAsyncEnumerable — but it's worth noting for future changes.
AOT benefit
Fewer compiler-generated state machine types also reduces the AOT codegen surface, which aligns well with TUnit's Native AOT compatibility requirement.
Minor observation (out of scope for this PR)
After this change in AsyncUntypedDataSourceSourceGeneratorAttribute, the call chain is:
GetDataRowsAsync → GenerateAsync → GenerateDataSourcesAsync
GenerateAsync is now a pure pass-through. GetDataRowsAsync could call GenerateDataSourcesAsync directly, but that's a separate refactor and the current structure is fine as-is.
The removed comment in AsyncUntypedDataSourceSourceGeneratorAttribute.cs was only documenting intent in the now-deleted loop body, so removing it is appropriate.
Overall: LGTM. Straightforward, correct, and beneficial to performance and AOT compatibility.
Remove async state machine type references from public API files since methods are no longer async (they just return the IAsyncEnumerable directly).
Updated [TUnit](https://github.com/thomhurst/TUnit) from 1.18.37 to 1.19.16. <details> <summary>Release notes</summary> _Sourced from [TUnit's releases](https://github.com/thomhurst/TUnit/releases)._ ## 1.19.16 <!-- Release notes generated using configuration in .github/release.yml at v1.19.16 --> ## What's Changed ### Other Changes * Truncate exceptions in GitHub summary tables by @thomhurst in thomhurst/TUnit#5108 ### Dependencies * chore(deps): update tunit to 1.19.11 by @thomhurst in thomhurst/TUnit#5106 * chore(deps): bump dompurify from 3.3.0 to 3.3.2 in /docs by @dependabot[bot] in thomhurst/TUnit#5096 * chore(deps): bump svgo from 3.2.0 to 3.3.3 in /docs by @dependabot[bot] in thomhurst/TUnit#5086 **Full Changelog**: thomhurst/TUnit@v1.19.11...v1.19.16 ## 1.19.11 <!-- Release notes generated using configuration in .github/release.yml at v1.19.11 --> ## What's Changed ### Other Changes * Fix HTML report sort to also reorder groups by @thomhurst in thomhurst/TUnit#5103 * fix: correct expand-all icon SVG in HTML report by @slang25 in thomhurst/TUnit#5105 * Avoid some redundant list allocations by @SimonCropp in thomhurst/TUnit#4963 * avoid some redundant enumerable alloc by @SimonCropp in thomhurst/TUnit#4972 * use char instead of string for joins by @SimonCropp in thomhurst/TUnit#4989 ### Dependencies * chore(deps): update dependency nunit to 4.5.1 by @thomhurst in thomhurst/TUnit#5097 * chore(deps): update tunit to 1.19.0 by @thomhurst in thomhurst/TUnit#5099 * chore(deps): update dependency humanizer to 3.0.10 by @thomhurst in thomhurst/TUnit#5101 * chore(deps): update dependency nunit.analyzers to 4.12.0 by @thomhurst in thomhurst/TUnit#5102 **Full Changelog**: thomhurst/TUnit@v1.19.0...v1.19.11 ## 1.19.0 <!-- Release notes generated using configuration in .github/release.yml at v1.19.0 --> ## What's Changed ### Other Changes * fix: improve CreateTestVariant API and fix void/ValueTask return types by @thomhurst in thomhurst/TUnit#5095 ### Dependencies * chore(deps): update tunit to 1.18.37 by @thomhurst in thomhurst/TUnit#5094 **Full Changelog**: thomhurst/TUnit@v1.18.37...v1.19.0 Commits viewable in [compare view](thomhurst/TUnit@v1.18.37...v1.19.16). </details> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
No description provided.