-
-
Notifications
You must be signed in to change notification settings - Fork 110
perf: IReadOnlyList<Artifact> call ToArray
#4527
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
SummaryChanges Artifacts property from ToList() to ToArray() to reduce allocations when the collection is empty. Critical IssuesNone found ✅ SuggestionsConsider caching to eliminate repeated allocations: The Artifacts property is accessed multiple times in hot paths (TUnit.Engine/Extensions/TestExtensions.cs:144,146 calls it twice). Each call allocates a new array/list from the ConcurrentBag. While ToArray() is better for empty collections (returns cached Array.Empty), both ToArray() and ToList() allocate for non-empty collections on every access. Consider caching if the bag is immutable after test execution starts. This would eliminate all repeated allocations. Verdict✅ APPROVE - The change improves performance for the common case (empty collections). |
Call
ToArrayinstead ofToList, most of the time the underlying collection contains zero items so it will return an empty array.Before
After