nats-io / nats.net.v2

Full Async C# / .NET client for NATS
https://nats-io.github.io/nats.net.v2/
Apache License 2.0
202 stars 40 forks source link

Keep sub alive when reading channel #506

Closed caleblloyd closed 1 month ago

caleblloyd commented 1 month ago

Resolves #499 Resolves #505

Adds GC keepalive calls to ActivityEndingMsgReader in order to prevent sub from being GC'd while the channel reader is being interacted with.

Learned about GCHandle.Alloc from this blog post - Using GC.KeepAlive in async methods

I left RegisterSubAnchor in place for now as I saw it was in-use still in the JetStream project. But if every sub there uses an ActivityEndingMsgReader it should be able to be eliminated.

mtmk commented 1 month ago

Very nice! 😎

mtmk commented 1 month ago

...just checking the tests. they are still passing for me even after removing the GC magic.

caleblloyd commented 1 month ago

Of course... I had it all working then simplified the tests to DRY it up, and I covered up the original failures. Just refactored them back to use Task.Run which now fail again for me when using the original ActivityEndingMsgReader

caleblloyd commented 1 month ago

Added a new benchmark for Subscribe, a side effect of the GC Handle is that the WaitAsync... TryRead pattern allocates more than ReadAllAsync on the channel reader now. So we will probably want to change our patterns to use more of ReadAllAsync when accessing the sub reader

Method Mean Error StdDev Gen0 Allocated
SubscribeAsync 1.002 s 0.0076 s 0.0004 s 3000.0000 30.54 MB
CoreWait 1.002 s 0.0016 s 0.0001 s 5000.0000 46.65 MB
CoreRead 1.003 s 0.0112 s 0.0006 s 5000.0000 40.18 MB
CoreReadAll 1.002 s 0.0005 s 0.0000 s 3000.0000 30.62 MB