Skip to content

Commit

Permalink
Dedup Task.WhenAll non-generic and generic implementations (#88154)
Browse files Browse the repository at this point in the history
The generic implementation was calling the non-generic one and then using an additional continuation to extract the resulting `Task<TResult>` due to lack of covariance on classes.  We can instead just factor the shared implementation out into a generic with the type parameter constrained to be a Task.  This results in simpler code as well as avoiding an extra continuation in the generic case.

As part of cleaning this up:
- I changed code where we need to make a defensive copy of an input collection to use CopyTo; we were already doing this in some places but not others.  This saves on an enumerator allocation when enumerating the source collection, as well as multiple interface calls.
- I augmented WhenAny to also special-case `List<Task>`, as that's a common input and we can handle it a bit more efficiently, especially if the collection ends up containing just two tasks.
- I removed the `GenericDelegateCache<TAntecedentResult, TResult>`.  That was from a time before the C# compiler supported caching of generic lambdas. It would have needed to have been updated to handle the stronger type coming out of CommonCWAnyLogic, so I instead just got rid of it.  We're better off lazily-creating these rarely used delegates, anyway.
  • Loading branch information
stephentoub committed Jul 5, 2023
1 parent d29c1c5 commit 9bda919
Show file tree
Hide file tree
Showing 3 changed files with 128 additions and 139 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -1626,15 +1626,19 @@ internal static Task<TResult> ContinueWhenAllImpl<TAntecedentResult>(Task<TAntec
if (continuationFunction != null)
{
return starter.ContinueWith(
GenericDelegateCache<TAntecedentResult, TResult>.CWAllFuncDelegate,
static (starter, continuationFunction) => ((Func<Task<TAntecedentResult>[], TResult>)continuationFunction!)(starter.Result),
continuationFunction, scheduler, cancellationToken, continuationOptions);
}
else
{
Debug.Assert(continuationAction != null);

return starter.ContinueWith(
GenericDelegateCache<TAntecedentResult, TResult>.CWAllActionDelegate,
static (starter, continuationAction) =>
{
((Action<Task<TAntecedentResult>[]>)continuationAction!)(starter.Result);
return default(TResult)!;
},
continuationAction, scheduler, cancellationToken, continuationOptions);
}
}
Expand Down Expand Up @@ -2026,7 +2030,7 @@ internal static Task<TResult> ContinueWhenAnyImpl<TAntecedentResult>(Task<TAntec
if (scheduler == null) ThrowHelper.ThrowArgumentNullException(ExceptionArgument.scheduler);

// Call common ContinueWhenAny setup logic, extract starter
Task<Task> starter = TaskFactory.CommonCWAnyLogic(tasks);
Task<Task<TAntecedentResult>> starter = TaskFactory.CommonCWAnyLogic(tasks);

// Bail early if cancellation has been requested.
if (cancellationToken.IsCancellationRequested
Expand All @@ -2040,64 +2044,20 @@ internal static Task<TResult> ContinueWhenAnyImpl<TAntecedentResult>(Task<TAntec
if (continuationFunction != null)
{
return starter.ContinueWith(
GenericDelegateCache<TAntecedentResult, TResult>.CWAnyFuncDelegate,
static (starter, continuationFunction) => ((Func<Task<TAntecedentResult>, TResult>)continuationFunction!)(starter.Result),
continuationFunction, scheduler, cancellationToken, continuationOptions);
}
else
{
Debug.Assert(continuationAction != null);
return starter.ContinueWith(
GenericDelegateCache<TAntecedentResult, TResult>.CWAnyActionDelegate,
static (starter, continuationAction) =>
{
((Action<Task<TAntecedentResult>>)continuationAction!)(starter.Result);
return default(TResult)!;
},
continuationAction, scheduler, cancellationToken, continuationOptions);
}
}
}

// For the ContinueWhenAnyImpl/ContinueWhenAllImpl methods that are generic on TAntecedentResult,
// the compiler won't cache the internal ContinueWith delegate because it is generic on both
// TAntecedentResult and TResult. The GenericDelegateCache serves as a cache for those delegates.
internal static class GenericDelegateCache<TAntecedentResult, TResult>
{
// ContinueWith delegate for TaskFactory<TResult>.ContinueWhenAnyImpl<TAntecedentResult>(non-null continuationFunction)
internal static Func<Task<Task>, object?, TResult> CWAnyFuncDelegate =
static (Task<Task> wrappedWinner, object? state) =>
{
Debug.Assert(state is Func<Task<TAntecedentResult>, TResult>);
var func = (Func<Task<TAntecedentResult>, TResult>)state;
var arg = (Task<TAntecedentResult>)wrappedWinner.Result;
return func(arg);
};

// ContinueWith delegate for TaskFactory<TResult>.ContinueWhenAnyImpl<TAntecedentResult>(non-null continuationAction)
internal static Func<Task<Task>, object?, TResult> CWAnyActionDelegate =
static (Task<Task> wrappedWinner, object? state) =>
{
Debug.Assert(state is Action<Task<TAntecedentResult>>);
var action = (Action<Task<TAntecedentResult>>)state;
var arg = (Task<TAntecedentResult>)wrappedWinner.Result;
action(arg);
return default!;
};

// ContinueWith delegate for TaskFactory<TResult>.ContinueWhenAllImpl<TAntecedentResult>(non-null continuationFunction)
internal static Func<Task<Task<TAntecedentResult>[]>, object?, TResult> CWAllFuncDelegate =
static (Task<Task<TAntecedentResult>[]> wrappedAntecedents, object? state) =>
{
wrappedAntecedents.NotifyDebuggerOfWaitCompletionIfNecessary();
Debug.Assert(state is Func<Task<TAntecedentResult>[], TResult>);
var func = (Func<Task<TAntecedentResult>[], TResult>)state;
return func(wrappedAntecedents.Result);
};

// ContinueWith delegate for TaskFactory<TResult>.ContinueWhenAllImpl<TAntecedentResult>(non-null continuationAction)
internal static Func<Task<Task<TAntecedentResult>[]>, object?, TResult> CWAllActionDelegate =
static (Task<Task<TAntecedentResult>[]> wrappedAntecedents, object? state) =>
{
wrappedAntecedents.NotifyDebuggerOfWaitCompletionIfNecessary();
Debug.Assert(state is Action<Task<TAntecedentResult>[]>);
var action = (Action<Task<TAntecedentResult>[]>)state;
action(wrappedAntecedents.Result);
return default!;
};
}
}
Loading

0 comments on commit 9bda919

Please sign in to comment.