RevenantX / LiteNetLib

Lite reliable UDP library for Mono and .NET
https://revenantx.github.io/LiteNetLib/index.html
MIT License
3.07k stars 496 forks source link

GC Allocations #205

Closed Alloc86 closed 6 years ago

Alloc86 commented 6 years ago

Hi again,

I did some profiling on the library and noticed some things that could possibly be improved on in regards to allocations (and thanks to Unity still not switching to a proper GC this is always an issue :( ).

  1. First of all I noticed that the NetUtils.DebugWrite* methods all take an params-argument, meaning that every call (even if no args are passed) results in an allocation of an array on the caller side. I wrote a simple patch that allows each of those methods to be called without any args (and also switched some of the calls that only supplied one arg anyway to just concat the string itself as a string.Format call for those was not really helpful :) ). As I haven't worked with GitHub and pull requests so far I attached it here:DebugWrite.txt

Now for those that I only looked at but no change so far :)

  1. NetManager.ReceiveFromPeer creates a NetEvent and sets the DataReader's Source to a copy (GetPacketData()) of the NetPacket's internal array. Could the NetPacket be kept with the NetEvent and only be recycled after the event was handled so that it does not allocate new arrays all the time?

  2. NetEvent instances aren't pooled at all currently. Still they both pile up in size as well as number of allocations (afaik the number is just as important as the size as the GC has to traverse all objects). This seems to be done in Master already?

As for the last two I would not be surprised if there's no way to change this on your end, but maybe there is anyway?

  1. Socket.SendTo calls IPEndPoint.Serialize internally each time, causing 80 Bytes of garbage for a new SocketAddress instance. No idea if there eg. is a way to provide this serialized form to Socket.SendTo so you could cache it or something like that?

  2. Socket.ReceiveFrom does the same as SendTo, but also actually creates the IPEndPoint instances itself. Doubt there's anything that can be done about this one at all?

Library version: release version 0.7.7.12

Framework: Unity 2018.1

RevenantX commented 6 years ago

First of all I noticed that the NetUtils.DebugWrite* methods all take an params-argument, meaning that

This calls disabled in almost all places unless you enable DEBUG_MESSAGESS definition, so there is no allocations in release and debug.

NetEvent instances aren't pooled at all currently.

Pooled. Only with unsynced evenets there is no pooling (my bad) https://github.com/RevenantX/LiteNetLib/blob/master/LiteNetLib/NetManager.cs#L371

Socket.SendTo calls IPEndPoint.Serialize Socket.ReceiveFrom does the same

This can be fixed with unsafe calls to winsock api on windows and bsd sockets on linux,mac but impact of this garbage very low.

NetManager.ReceiveFromPeer creates a NetEvent and sets the DataReader's Source to a copy (GetPacketData()) of the NetPacket's internal array. Could the NetPacket be kept with the NetEvent and only be recycled after the event was handled so that it does not allocate new arrays all the time?

I will look to this. I think i can improve that place.

Alloc86 commented 6 years ago

This calls disabled in almost all places unless you enable DEBUG_MESSAGESS definition, so there is no allocations in release and debug.

Argh, my bad. Forgot that Unity always sets the DEBUG define in the editor and also on dev builds (which I had to do so I could even profile at all :) ).

Socket.SendTo calls IPEndPoint.Serialize

This can be fixed with unsafe calls to winsock api on windows and bsd sockets on linux,mac but impact of this garbage very low.

Well, the size of the garbage is definitely very low. But still the number of objects is very high and that's what costs time in the GC, no matter if the individual objects are small (at least with Unity's non-generational GC). So hope is up they finally get around updating that :)

I will look to this. I think i can improve that place. Cool, that was the thing troubling me the most atm.

Combined with the changes for the NetEvent pooling and the feedback on Rejects in master this will make 0.8 a really great new release for us :)

RevenantX commented 6 years ago

In LiteNetLib 0.7 NetEvent polling also exists

RevenantX commented 6 years ago

@Alloc86 added NetPacketReader instead of NetDataReader at OnReceive calls. And you can recycle this reader. So for now there is no copies and GC calls in receive.

Alloc86 commented 6 years ago

That's really great, thank you a lot 👍

Alloc86 commented 6 years ago

Hm, as I'm updating for testing #237 I just noticed that in return you removed the Data property from NetDataReader so I can no longer access the whole byte[] without copying it. Can that be readded?

RevenantX commented 6 years ago

@Alloc86 you can access that data like array dataReader[0] == first byte. Or you can use dataReader.GetRemainingBytes() you will get copy of all remaining bytes. And you can use dataReader.GetBytes(myBuffer, dataReader.AvailableBytes)

Alloc86 commented 6 years ago

Yeah, I'm aware of those accessors. But the network abstraction layer we have in requires that I get a byte[] out of it. So in this case my only option would be to use GetBytes but that copies the array which is just overhead as the data is already copied (synchronously, so no danger here) on the other into a MemoryStream.

RevenantX commented 6 years ago

@Alloc86 this was result of internal pooling and performance optimisation. NetDataReader internal _data array contains all packet data (including header and other things). You can make one copy and it will not greatly affect performance (because this copy removed from library code)

Alloc86 commented 6 years ago

Ah, gotcha. Thanks. Btw, are you ever sleeping? Feels like there's never a moment you won't immediately reply :)

Alloc86 commented 6 years ago

PS: Am I right that for the added NetPacketReader recycling I should manually call _reader.Recycle after I'm done with it?

RevenantX commented 6 years ago

@Alloc86

Btw, are you ever sleeping? Feels like there's never a moment you won't immediately reply :)

Yes :)

I should manually call _reader.Recycle after I'm done with it?

Yes)