mgravell / Pipelines.Sockets.Unofficial

.NET managed sockets wrapper using the new "Pipelines" API
Other
400 stars 51 forks source link

UDP Sockets #36

Open softworkz opened 4 years ago

softworkz commented 4 years ago

One question:

Is this supposed to work with UDP sockets as well?

Did somebody try this?

Thanks,

softworkz

mgravell commented 4 years ago

UDP sockets aren't streams in the same way and would presumably need extra consideration. Feel free to test it!

On Fri, 12 Jul 2019, 00:50 softworkz, notifications@github.com wrote:

One question:

Is this supposed to work with UDP sockets as well?

Did somebody test this?

Thanks,

softworkz

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/mgravell/Pipelines.Sockets.Unofficial/issues/36?email_source=notifications&email_token=AAAEHMBZU7QFU752LGGQEL3P67BKPA5CNFSM4IBZ7BHKYY3PNVWWK3TUL52HS4DFUVEXG43VMWVGG33NNVSW45C7NFSM4G6YHPNA, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAEHMGJ3O6MJQXQE3Z2P6TP67BKPANCNFSM4IBZ7BHA .

softworkz commented 4 years ago

I did.

To make it work in general it requires just a single line change:

https://github.com/mgravell/Pipelines.Sockets.Unofficial/blob/ea683bfc007ee4adba351d42a8438af5239225c1/src/Pipelines.Sockets.Unofficial/SocketConnection.Receive.cs#L54

Needs to be changed to this:

var buffer = _receiveFromSocket.Writer.GetMemory(this._receiveOptions.MinimumSegmentSize);

And ZeroLength reads must not be used.

But overall it didn't perform very well due to the frequent flushing (= after each receive).

With UDP, the data doesn't get accumulated in the buffer. You get a single datagram with each receive call. And that causes too many async calls which are becoming a performance issue with high data rates.

Drawaes commented 4 years ago

I'll be honest as a big user of UDP and a user of pipelines I had considered it. However as Marc said it doesn't make a lot of sense in this case, you completely lose the framing of the datagram so then you need to implement framing over the top, at which point I question if you are rewriting tcp over UDP.

softworkz commented 4 years ago

No, the framing doesn't get lost as mentioned above because in case of UDP you always get a single datagram with each receive call. In my case, a datagram contains n packets of size x with x being a global constant. Packets are not sliced across datagrams.

Maybe I should say that this is about media streaming. Several protocols in this area are UDP based and the challenge is to find the most effective implementation.

Arguing about protocol design and choice is a dead end in this case.. :-)

mgravell commented 4 years ago

To me, UDP seems more like a Channel<ReadOnlySequence<byte>>. Perhaps that's something that we could implement with minimal work, giving you all the benefits without the pain, and respecting the framing differences between UDP and TCP.

Happy to explore that area. I've mostly been TCP focused, but I love expanding my horizons...

Drawaes commented 4 years ago

Which was exactly the discussion I had with Fowler around UDP. I suspect it's not on the pipelines asp.net radar yet either (maybe quic will change that)

softworkz commented 4 years ago

What do you mean by Channel<ReadOnlySequence> ? Wasn't 'Channel' the initial term before it became 'Pipeline'?

Drawaes commented 4 years ago

System.Threading.Channels made around the same time as pipelines (which maybe why pipelines was renamed... But that's just a rumour 😀)

mgravell commented 4 years ago

Channel<T> is the high perf reader-writer async queue thing. Yes, the name was also the early name of pipelines, but pipelines lost and became pipelines. Related and overlapping topic, and I do think it would make a good API here, for packet-based rather than stream-based protocols.

softworkz commented 4 years ago

That sounds really interesting, every time I read about channels somewhere I thought it was about the early pipeline stuff.

Too bad that it doesn't seem to support netstandard2.0 (need to support BSD as well)

mgravell commented 4 years ago

Channels and pipelines are both netstandard 1.3+; you're fine

softworkz commented 4 years ago

So, it's not yet documented as such?

Or is that the wrong place: https://docs.microsoft.com/en-us/dotnet/api/system.threading.channels.channel-1?view=dotnet-plat-ext-2.1&viewFallbackFrom=netstandard-2.0

mgravell commented 4 years ago

Nuget: https://www.nuget.org/packages/System.Threading.Channels/ - click dependencies

softworkz commented 4 years ago

Thanks a lot, I'll look into it!

mgravell commented 4 years ago

In fact, maybe a case could be made for IAsyncEnumerable<ReadOnlySequence<byte>> - I will have to play!

softworkz commented 4 years ago

Think about small packet sizes and a huge amount of them :-)

softworkz commented 4 years ago

Don't forget to add a running number when sending udp datagrams for testing. You need to make sure that you're getting them all.

Mine are about 25k size, I tried to get close to half of the maximum allowed (64k). The smaller I made the datagrams the worse it got (CPU usage). (it's a real world application - that means: half datagram size => double amount of datagrams per time)

PS: Watch your e-mail, I can provide some more interesting details if you wish

mgravell commented 4 years ago

Started a spike with this. Looking viable. A running number in the lib would be pointless - it can't tell if it didn't get a packet. Hence IMO any running number must be part of your payload, not the code that makes things work.

If you mean a running number in test code: sure, absolutely.

softworkz commented 4 years ago

Yes, of course I meant a number inside the payload for testing.

sebandraos commented 4 years ago

Did anything come of this discussion? I can see a UDP branch but it doesn't seem like it ever got merged into master. Was an alternative to that branch implemented instead? Thanks

mgravell commented 4 years ago

At the current time: no. It is of interest, but I have 72 other things that I'm working on, and it isn't at the top of my list. I'm so aware that Bedrock is (hopefully) gaining some client-side love for .NET 5, so I was hoping to see if anything changes there.

On Fri, 22 May 2020, 15:50 Sebastian Andraos, notifications@github.com wrote:

Did anything come of this discussion? I can see a UDP branch but it doesn't seem like it ever got merged into master. Was an alternative to that branch implemented instead? Thanks

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mgravell/Pipelines.Sockets.Unofficial/issues/36#issuecomment-632732026, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAEHMEFB4TITPFT3HZ34JDRS2GLXANCNFSM4IBZ7BHA .

sebandraos commented 4 years ago

Ok, cool thanks.

sgf commented 3 years ago

The TCP protocol performs very badly in the case of a wide geographical distance around the world. UDP is controlled by appropriate high-level algorithms to achieve an ideal low latency.