dotnet / reactive

The Reactive Extensions for .NET
http://reactivex.io
MIT License
6.73k stars 751 forks source link

Proposal for moving async LINQ to BCL #2102

Open idg10 opened 7 months ago

idg10 commented 7 months ago

This is a response to @davidfowl's suggestion that we migrate the System.Linq.Async code into the .NET runtime library:

Throwing out an idea: it would be good to cede the IX operators back into the BCL given the first class support for IAsyncEnumerable. There’s been many discussions about why we’re not adding async LINQ to the BCL and it’s because this package already does that. While I have no problems in general with an IX dependency, it feels like something that should be built in now given the in the box (including compiler support) for IAE.

stephentoub commented 7 months ago

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".

idg10 commented 7 months ago

One reason I had thought that breaking changes would be a problem is that David Fowler's original message said:

we’re not adding async LINQ to the BCL and it’s because this package already does that

This implies that System.Linq.Async has for some years been the BCL team's answer to the question: how do I do async LINQ to Objects? So although it's not an officially Microsoft-recommended, supported package, it is in practice a thing Microsoft has been telling people to use.

Of course that doesn't mean that an implementation in the runtime libraries would be obliged to be compatible with the existing System.Linq.Async. It would just mean that there might be a non-trivial number of aggrieved developers unhappy about the situation. Also, this worried me slightly:

if you're multi-targeting both core and framework, your life just got more complicated

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 System) to a) clarify that this isn't the official implementation it might superficially seem to be and b) clear the path for an official implementation at some point in the future.

Here's one example of the sort of problem that could occur if we remain in the System namespace hierarchy and a future .NET provides async LINQ out of the box. Anyone who writes using System.Linq; (or global using global::System.Linq;; so any .NET >= 8.0 project that accepts the default ImplicitUsings setting) who has a reference to our System.Linq.Async might write this sort of code:

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 IAsyncEnumerable<T>.

(If there were absolutely no overlap between the method names offered by the current System.Linq.Async and a new built-in async LINQ—e.g., if the built-in implementation stuck with the standard names like All instead of AllAsync—then this wouldn't occur. But even so, the idea of two competing async LINQ implementations offering extension methods for the same type, imported via the same namespace, sounds like a bad idea.)

So it seems like there may really be two separate questions here:

  1. Should the Ix.NET project change the package and namespace names, and deprecated the existing System.Linq.Async package? (Probably.)
  2. Can we offer anything to the BCL team to help make built-in async LINQ happen? (Debatable.)

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 System.Linq.Async, so I'm aware this isn't so much a gift from the Rx maintainers to the .NET team as it is a request for you to take this support overhead off our hands!

It was only because David Fowler suggested that this might be something the .NET team should do that we have been looking at this.

davidfowl commented 7 months ago

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.