Skip to content
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

Dedup Task.WhenAll non-generic and generic implementations #88154

Merged
merged 2 commits into from
Jul 5, 2023

Conversation

stephentoub
Copy link
Member

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.
[Benchmark]
public Task<Task<int>> WhenAnyGeneric_ListNotCompleted()
{
    AsyncTaskMethodBuilder<int> atmb1 = default;
    AsyncTaskMethodBuilder<int> atmb2 = default;
    AsyncTaskMethodBuilder<int> atmb3 = default;

    Task<Task<int>> wa = Task.WhenAny(new List<Task<int>>() { atmb1.Task, atmb2.Task, atmb3.Task });

    atmb3.SetResult(42);

    return wa;
}
Method Toolchain Mean Error StdDev Ratio Alloc Ratio
WhenAnyGeneric_ListNotCompleted \main\corerun.exe 356.0 ns 7.19 ns 17.23 ns 1.00 1.00
WhenAnyGeneric_ListNotCompleted \pr\corerun.exe 225.7 ns 0.81 ns 0.68 ns 0.61 0.72

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.
@ghost
Copy link

ghost commented Jun 28, 2023

Tagging subscribers to this area: @dotnet/area-system-threading-tasks
See info in area-owners.md if you want to be subscribed.

Issue Details

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.
[Benchmark]
public Task<Task<int>> WhenAnyGeneric_ListNotCompleted()
{
    AsyncTaskMethodBuilder<int> atmb1 = default;
    AsyncTaskMethodBuilder<int> atmb2 = default;
    AsyncTaskMethodBuilder<int> atmb3 = default;

    Task<Task<int>> wa = Task.WhenAny(new List<Task<int>>() { atmb1.Task, atmb2.Task, atmb3.Task });

    atmb3.SetResult(42);

    return wa;
}
Method Toolchain Mean Error StdDev Ratio Alloc Ratio
WhenAnyGeneric_ListNotCompleted \main\corerun.exe 356.0 ns 7.19 ns 17.23 ns 1.00 1.00
WhenAnyGeneric_ListNotCompleted \pr\corerun.exe 225.7 ns 0.81 ns 0.68 ns 0.61 0.72
Author: stephentoub
Assignees: -
Labels:

area-System.Threading.Tasks

Milestone: -

Copy link
Member

@jeffhandley jeffhandley left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the summary of the changes in the PR description. Looks great!

@stephentoub stephentoub merged commit 9bda919 into dotnet:main Jul 5, 2023
@stephentoub stephentoub deleted the taskwhenany branch July 5, 2023 23:41
@ghost ghost locked as resolved and limited conversation to collaborators Aug 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants