Possible Task.WhenAll performance improvements

[Alex-Sob]

Let me share a couple of considerations related to Task.WhenAll performance, specifically for WhenAll<TResult>(IEnumerable<Task<TResult>> tasks) overload.

1. Currently if tasks is not ICollection<T> and therefore the count is not known, it will create a list of tasks and then return new WhenAllPromise<TResult>(list.ToArray()). It seems like WhenAllPromise could take a Span<T> instead of an array, but currently it cannot because it stores that array into the field. Looks like the field is there only to be used in ShouldNotifyDebuggerOfWaitCompletion property and is probably needed only for debugging, can that allocation made by list.ToArray() be avoided in release builds?

2. I believe that it's pretty common when a set of tasks is produced for some collection of items, e.g when calling source.Select(x => ProcessAsync(x)). We know collection size, but then we're losing it when calling Select. It probably would make sense to handle Iterator<T> internal type to get task count if it's available to avoid unnecessary allocations.

[stephentoub]

Looks like the field is there only to be used in ShouldNotifyDebuggerOfWaitCompletion property and is probably needed only for debugging, can that allocation made by list.ToArray() be avoided in release builds?

That's not the only reason it's there. It's used in Invoke, when the last constituent task completes, to extract the results/exceptions/etc. from all of the tasks, and importantly in the same order those tasks were provided.

It probably would make sense to handle Iterator internal type to get task count if it's available to avoid unnecessary allocations.

Task is in corelib. The Iterator<T> you mention is an internal implementation detail of System.Linq. There's no good way for Task.WhenAll to query the underlying type here to extract length details. It could use something like the internal ValueListBuilder to at least use some stack memory / pooled memory as part of building up the temporary list.

[julealgon]

@stephentoub does Task.WhenEach suffer from similar issues or is this 100% limited to Task.WhenAll?

[stephentoub]

@stephentoub does Task.WhenEach suffer from similar issues or is this 100% limited to Task.WhenAll?

There's no "suffering" here :) My PR is a minor tweak to possibly avoid an allocation in the case where the incoming enumerable isn't a list/array.

Task.WhenEach is completely different. What are your concerns there?

[julealgon]

@stephentoub

@stephentoub does Task.WhenEach suffer from similar issues or is this 100% limited to Task.WhenAll?

There's no "suffering" here :) My PR is a minor tweak to possibly avoid an allocation in the case where the incoming enumerable isn't a list/array.

To clarify, I wasn't necessarily referring to your PR, but to the overall topic here. And I guess the wording was not great on my part: consider it "caracteristics" instead of "issues" 😝

Task.WhenEach is completely different. What are your concerns there?

It was just a question for my own understanding here. For new code, I suspect we'll end up using WhenEach substantially more over WhenAll and was wondering if there is a memory consideration there as well. I assume that since WhenEach is lazily evaluated, it wouldn't have to preallocate an array like WhenAll is doing as per the OP?

[Alex-Sob]

That's not the only reason it's there. It's used in Invoke, when the last constituent task completes, to extract the results/exceptions/etc. from all of the tasks, and importantly in the same order those tasks were provided.

It seems that Invoke is called only from the constructor and just iterates through the array, I was just wondering if the constructor could take a Span and hand it over to Invoke.

Task is in corelib. The Iterator<T> you mention is an internal implementation detail of System.Linq.

Ah, it's in a different assembly, missed that.

[stephentoub]

It seems that Invoke is called only from the constructor

That is not the case. Invoke provides the implementation of ITaskCompletionAction.Invoke, which is called when the task completes.

[Alex-Sob]

@stephentoub Got it, thanks.
Anyway, there will be an improvement in the core libraries, turns out it was worth starting a conversation :)

[Alex-Sob]

@stephentoub btw, what about Memory<T>? Though it has to be stored on the heap as well, would it be cheaper to use instead of allocating a new array with ToArray? (not sure if there's a way to get Memory<T> similar to CollectionsMarshal.AsSpan).

[stephentoub]

@stephentoub btw, what about Memory<T>? Though it has to be stored on the heap as well, would it be cheaper to use instead of allocating a new array with ToArray? (not sure if there's a way to get Memory<T> similar to CollectionsMarshal.AsSpan).

Memory<T> still needs to reference storage.

[Alex-Sob]

Memory<T> still needs to reference storage.

I mean it first creates a List<T> and then calls ToArray, would it be cheaper to use Memory<T> for its backing array instead?