Query: Ix-Async dependency updated to 3.1

[natemcmaster]

The Rx team has just completed a near-rewrite of a majority of the Ix Async operators. The new pattern follows the System.Linq approach of using iterator objects to minimize allocations and recursion (in favor of state machines). dotnet/reactive#237

To upgrade, to 3.1, we'll need to test the changes, possible remove internal workarounds, and investigate the effect of other changes in this update (notably: Ix 3.1 uses NETStandard.Library instead of granular BCL dependencies, which breaks from EF/ASP.NET's pattern of using granular dependencies. cref https://github.com/aspnet/Coherence-Signed/issues/362)

cc @onovotny @anpete

[clairernovotny]

An update on this -- the Ix Async optimizations/rewrite has just been checked in and is available on our CI feed in any 3.1 build starting at 00216: System.Interactive.Async.3.1.0-build-00216.nupkg.

We are cutting an RC version that we'll publish to NuGet shortly but would very much appreciate your validation before we GA 3.1.

Thanks

[anpete]

Happy to validate against the RC when it drops.

[clairernovotny]

Great, thanks - aiming for early this coming week; there's one outstanding PR aimed at 3.1 should be ready for merge first.

[clairernovotny]

I'm about to tweet this, but here's the RC:
https://www.nuget.org/packages/System.Interactive.Async/3.1.0-rc

Release notes are here: https://github.com/Reactive-Extensions/Rx.NET/releases/tag/v3.1.0-rc

[natemcmaster]

FWIW we've decided to use NETStandard.Library, so that removes one of our concerns. cref #6626

[divega]

Clearing up milestone to discuss it in triage because I think it is safer to postpone this change for now, in particular getting rid of our custom operator implementations which is riskier. We can do it in 1.2 and have more runway to find any bugs.

An alternative is to just to the minimal package upgrade in 1.1.0-preview1 but leave all our custom operator implementations in until 1.2.0. There seems to be much less value in making this change though.

[smitpatel]

I tested dev against RC build (without remove custom operator implementations), there are 2 async tests which are failing in terms of number of entries in change tracker. Everything else passed successfully.

[clairernovotny]

Is there any way we can get any validation from the EF team that Ix 3.1 fixes the underlying issues you saw before? We've had trouble reproducing those outside of EF.

[clairernovotny]

For Async, we've removed all use of Task.Run so there's no custom threading.

[divega]

@onovotny we can still do the work relatively soon and start getting some validation. I just wouldn't want to release a new version of EF Core with this dependency without proper "baking" time.

[clairernovotny]

Fair enough....just was aiming for the validation as our unit tests are all passing with very high code coverage. Just making sure there's no known issues before we declare 3.1 final.

[clairernovotny]

One argument in favor of just the package upgrades at the very least - 3.1 has fixed several bugs deep within its operators and has better performance/fewer allocations. Even without removing the custom operator impls, it'll provide a better baseline library version to your users.

[divega]

there are 2 async tests which are failing in terms of number of entries in change tracker. Everything else passed successfully

@smitpatel any idea about the cause of this? Presumably a behavior change in an operator that we haven't re-implemented?

[clairernovotny]

Just checking in here, any updates? Would love to get Ix Async locked down and shipped as 3.1 but we were awaiting your team's sign-off given the bugs you'd previously uncovered :)

[clairernovotny]

Please update your package reference for 1.1 to use Ix 3.1. @jskeet confirms bugs fixed there:

https://twitter.com/jonskeet/status/793880989663657984

Even if you don't remove your internal workarounds yet.

[jskeet]

This is the issue I ran up against today: dotnet/reactive#290

[anpete]

@onovotny We tried it and ran into some issues. @smitpatel is working on a repro.

[clairernovotny]

Thanks @anpete. If we can get a repro, we can/will commit to fixing it for Ix 3.1. What is your ETA for shipping EF Core 1.1?

There's just too many things fixed in Ix 3.1 to ignore it completely IMHO.

[smitpatel]

dotnet/reactive#291

[clairernovotny]

Thanks, will dive into this immediately.

[clairernovotny]

Thanks for the repro, just checked in a fix and am building an RC2 package to push to nuget

[clairernovotny]

I've just pushed an RC2 package to NuGet, if this passes all of your regression tests, I'd be happy to republish it as RTM.

https://www.nuget.org/packages/System.Interactive.Async/3.1.0-rc2

[divega]

We still won't upgrade the dependency in 1.1 (it is too late/risky) but we will see if we can do some additional verification over your RC2 package soon.

cc @smitpatel

[smitpatel]

RC2 Packages passes for all current tests (without removing workarounds).
I am doing additional testing with removing workarounds to see if bugs are fixed. I will report if anything still doesn't work.

[clairernovotny]

@divega my concern is that 3.0 has known bugs, as @jskeet has encountered. 3.1 is intended as a bugfix release that does address many longstanding Ix issues and we're hoping to get it out broadly. In the new world of transitive dependencies, I'm afraid that most people won't even know there's an upgrade available and hit issues that have already been fixed.

Question is if keeping 3.0 with its known bugs is more or less risky than 3.1 that has had those fixed (with testing to prove it). As to the Join issue, there was indeed a gap in the test coverage as that went unnoticed, but I suspect/and am hoping that's the only exception. Overall, the library has >98% unit test coverage, FWIW.

Happy to jump on a quick call to discuss further if you'd like.

[divega]

Clearing up milestone so that we can discuss @onovotny's concern in triage.

[smitpatel]

@divega - I have created pulled for feedback if anything missing in verification. The PR which will resolve this issue would be different or merged after RTM of Ix-Async

@onovotny - I have removed most of the workarounds we had in the place and code works like charm. I am still scanning if there more things to removed. But so far looks good.

[clairernovotny]

Ix Async 3.1 was just GA'd: https://twitter.com/onovotny/status/794922947798515713

[smitpatel]

@divega @anpete - Should we upgrade now?

[divega]

@smitpatel sounds good to me.

[clairernovotny]

One minor update - you probably want to use Ix Async 3.1.1 instead of 3.1. There's no binary changes, only a packaging update to ensure that net45 does not pull in the NETStandard.Library package to reduce friction for desktop .net users. That's the only change.

[clairernovotny]

/cc @anpete @divega @smitpatel just wanted to call your attention to Ix 3.1.1 (above comment) in case you didn't see it yet.