-
Notifications
You must be signed in to change notification settings - Fork 750
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
Proposal for moving async LINQ to BCL #2102
base: main
Are you sure you want to change the base?
Conversation
The `System.Linq.Async` code is in the Rx.NET repository (this repository) as an accident of history. Specifically: | ||
|
||
* This repository is also home to Rx's mirror image, `System.Interactive`, aka **Ix** | ||
* Ix originally introduced its own asynchronous version of `IEnumerable<T>` many years before .NET runtime libraries added `IAsyncEnumerable<T>` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't that be IAsyncEnumerable<T>
rather than IEnumerable<T>
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I read that as containing an implied parenthetical clause: "introduced its own asynchronous version of IEnumerable<T>
(not confusingly at the time this was called IAsyncEnumerable<T>
, later replaced with the .NET runtime version of same)" and then discovered that is what it actually said in the next sentence.
* This repository is also home to Rx's mirror image, `System.Interactive`, aka **Ix** | ||
* Ix originally introduced its own asynchronous version of `IEnumerable<T>` many years before .NET runtime libraries added `IAsyncEnumerable<T>` | ||
* Ix already had a comprehensive implementation of the standard LINQ operators for its original asynchronous enumerable | ||
* When [IAsyncEnumerable<T>](https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.iasyncenumerable-1?view=net-8.0) was added in .NET Core 3.0, it made sense for Ix to start using that instead of its original definition |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we link to the .NET Show episode where Bart demonstrates the new support? https://www.youtube.com/watch?v=Ktl8K2b1-WU
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, just seen this mentioned below!
* Ix originally introduced its own asynchronous version of `IEnumerable<T>` many years before .NET runtime libraries added `IAsyncEnumerable<T>` | ||
* Ix already had a comprehensive implementation of the standard LINQ operators for its original asynchronous enumerable | ||
* When [IAsyncEnumerable<T>](https://learn.microsoft.com/en-us/dotnet/api/system.collections.generic.iasyncenumerable-1?view=net-8.0) was added in .NET Core 3.0, it made sense for Ix to start using that instead of its original definition | ||
* At that point, Ix had a complete implementation of LINQ for `IAsyncEnumerable<T>`, and the .NET runtime libraries did not, so it make sense for Ix to publish these as the [`System.Linq.Async`](https://www.nuget.org/packages/System.Linq.Async) package we have today |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When I was trying to explain what Ix was - there was a bit of confusion about the existing packages - what is System.Interactive
vs System.Linq.Async`.
Is System.Interactive purely about the bespoke, legacy implementation of IAsync? Does the world still need this, or should it just be deprecated?
|
||
## Use of T4 templates | ||
|
||
The existing code has various `.tt` files to generate multiple forms of the same code. The `Sum`, `Min`, `Max` and `Average` operators all use this to provide implementations for each of the built-in numeric types. Of course, as of .NET 7.0 this sort of thing isn't strictly necessary because we can use generic math instead. Unfortunately, the need to maintain binary compatibility means we can't do that in practice, because existing code will be looking for the specialized methods. Also we generate 2-, 3-, and 4- argument forms of some internal combination operators use to improve the efficiency of certain `Select` and `Where` scenarios. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could modernizing the API to use the new numeric types be an option as part of the migration?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would break the existing users, and that's explicitly not something we want to do, because people behave as if this is part of the BCL even if it isn't (so we've got very strict guarantees).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But if there is a time to cause such pain, this is it.
We can certainly have the conversation again (in the dotnet/runtime repo would be a better place) about what it would look like to add IAsyncEnumerable extensions for the LINQ surface area into the core .NET libraries. But I'm fairly confident the compatibility goals outlined here would not be achieved. First, if we were actually adding this all in, I expect it would be with additions to the System.Linq.dll library and it would only be for .NET Core, no separate package, which means it wouldn't be available for .NET Framework or previous core releases. Second, there were a bunch of choices made in the APIs exposed here that we would not want to bring in, e.g. an AggregateAsync method is fine, but not AggregateAwaitAsync or AggregateAwaitWithCancellationAsync... that's simply not how we'd choose to expose such functionality. We would also be able to design the new methods taking into account features that didn't previously exist, e.g. generic math. And all that means that both a) there would be breaking changes (both source and binary) and b) the existing types could not just be type-forwarded to the new ones. We would have to be ok saying "there are these new APIs and if you're currently using System.Linq.Async.dll you'll have breaking changes you'll need to adapt to when you upgrade to the new version of .NET, and if you're multi-targeting both core and framework, your life just got more complicated". |
One reason I had thought that breaking changes would be a problem is that David Fowler's original message said:
This implies that Of course that doesn't mean that an implementation in the runtime libraries would be obliged to be compatible with the existing
It gave me flashbacks to the System.Net.Http 4.1.1-4.3.0 problems. I'm keen not to create another situation like that! If it's a given that any in-the-box async LINQ would definitely not be either source- or binary-compatible (and it sounds like that is the case), it has me wondering whether I should deprecate the existing package, and release a new one with a different name and namespaces (something not in Here's one example of the sort of problem that could occur if we remain in the public async Task<bool> UseIae(IAsyncEnumerable<int> iae)
{
return await iae.AllAsync(x => x % 2 == 0);
} If they upgrade to a new .NET with built-in async LINQ, they would find that this is now ambiguous, because there are two different libraries both defining LINQ extension methods for (If there were absolutely no overlap between the method names offered by the current So it seems like there may really be two separate questions here:
We could do 1 even if we think there's a high chance the BCL team will never provide an official async LINQ. (And arguably we should, although there are pros and cons. If an official async LINQ never happens, this renaming would just have created some fruitless churn for everyone using the library.) I'm aware 2 was never going to be as simple as moving our existing code into the .NET runtime repo. This would be a 'free as in puppy' contribution—in practice this would be a new, non-trivial eternal support commitment for the .NET team, and it may well be that after a thorough code review, they decide they don't much like our code and want to rewrite the whole thing. This code was all written in an era when much less attention was paid to GC costs and other performance refinements that people have come to expect from the .NET runtime today. We (the current maintainers of Rx.NET) don't have the budget to modernise It was only because David Fowler suggested that this might be something the .NET team should do that we have been looking at this. |
I still think it's the right path forward. |
This is a response to @davidfowl's suggestion that we migrate the
System.Linq.Async
code into the .NET runtime library: