How do you write efficient sockets?

[Xio.]

Currently my Socket's use the standard APM Pattern BeginXXX, EndXXX

Additionally I'm utilizing a BufferPool that completely eliminates allocations that would be present on each receive/send operation.

Part's of the class for illustration

PHP Code:


public static class BufferPool
    {
        public static int BigPoolCount => BigPool.Count;

        public const int MinBigPoolSize = 2500;
        public const int MaxBufferSizeBytes = 850;

        public static ulong BigRecycleCount;
        public static ulong BigRequestCount;
        public static ulong CopyCount;

        private static readonly ConcurrentStack<byte[]> BigPool = new ConcurrentStack<byte[]>();

        public static byte[] GetBigBuffer()
        {
            BigRequestCount++;
            byte[] buffer;

            while (!BigPool.TryPop(out buffer))
                CreateBufferPool();
            
            return buffer;
        }

        public static byte[] Copy(byte[] packet)
        {
            CopyCount++;
            var copy = GetBigBuffer();
            Buffer.BlockCopy(packet, 0, copy, 0, packet.Length);
            return copy;
        }

        public static void RecycleBuffer(byte[] buffer)
        {
            Array.Clear(buffer, 0, buffer.Length);
            BigPool.Push(buffer);
            BigRecycleCount++;
        }

        private static void CreateBufferPool()
        {
            if (BigPool.Count <= MinBigPoolSize)
            {
                for (var i = 0; i < MinBigPoolSize; i++)
                    BigPool.Push(new byte[MaxBufferSizeBytes]);
                Output.WriteLine($"Socket's Big BufferPool Expanded to {BigPoolCount}", ConsoleColor.Green);
            }
        }
    } 


Packets are structs which I copy to a pooled buffer object using unsafe code so no additional allocations are being created.

(de)Serialization:

PHP Code:


public static unsafe implicit operator byte[](MsgAction msg)
        {
            var buffer = BufferPool.GetBigBuffer();
            fixed (byte* p = buffer)
                *(MsgAction*)p = *&msg;
            return buffer;
        }

        public static unsafe implicit operator MsgAction(byte[] msg)
        {
            MsgAction packet;
            fixed (byte* p = msg)
                packet = *(MsgAction*)p;
            BufferPool.RecycleBuffer(msg);
            return packet;
        } 


Outgoing packets are being merged into 850byte buffers, adding a little latency overhead (max 30ms + looptime) but improving the overal performance of the socket system by a huge margin. Usually 100 packets end up being 1-2 packets - especially useful with attack packets. (I'll override the equality/hashcode generation of the Account class to improve performance)

PHP Code:

public static class OutgoingPacketQueue
    {
        public static readonly ConcurrentDictionary<Account, ConcurrentQueue<byte[]>> PacketQueue = new ConcurrentDictionary<Account, ConcurrentQueue<byte[]>>();
        public static readonly AutoResetEvent Block = new AutoResetEvent(false);
        public static readonly Thread QueueThread = new Thread(WorkLoop);
        public static byte[] MergedPacket;
        public static byte[] Chunk;
        public static int Counter;
        public static int Offset;
        public static int TempLen;

        public static void Add(Account client, byte[] packet)
        {
            if (!QueueThread.IsAlive)
                QueueThread.Start();

            if (!PacketQueue.ContainsKey(client))
                PacketQueue.TryAdd(client, new ConcurrentQueue<byte[]>());

            PacketQueue[client].Enqueue(packet);
        }

        public static void WorkLoop()
        {
            while (true)
            {
                Block.WaitOne(30);
                foreach (var q in PacketQueue.Where(q => q.Value.Count > 0))
                {
                    Counter = 0;
                    Offset = 0;
                    MergedPacket = BufferPool.GetBigBuffer();
                    while (q.Value.Count > 0 && q.Value.TryPeek(out Chunk) && BitConverter.ToUInt16(Chunk, 0) + Offset <= BufferPool.MaxBufferSizeBytes)
                    {
                        if (q.Value.TryDequeue(out Chunk))
                        {
                            TempLen = BitConverter.ToUInt16(Chunk, 0);
                            if (TempLen == 0)
                            {
                                BufferPool.RecycleBuffer(Chunk);
                                continue;
                            }
                            Buffer.BlockCopy(Chunk, 0, MergedPacket, Offset, TempLen);
                            Offset += TempLen;
                            Counter++;
                            BufferPool.RecycleBuffer(Chunk);
                        }
                    }
                    if (Counter > 0)
                    {
                        q.Key.ActualPacketsPerSecond += Counter;
                        q.Key.ForceSend(MergedPacket, Offset);
                    }
                }
            }
        }
    } 


Sending 850bytes would be a waste if the actual content length is less than that, but copying the packet to a smaller buffer would create an additional allocation so I'm using the BeginSend with a Size parameter on my 850byte buffer;

PHP Code:

public void Send(byte[] packet, int size)
        {
            try
            {
                if (packet == null || BitConverter.ToUInt16(packet, 0) < 4)
                    return;

                if (Crypto!=null)
                    packet = Crypto.Encrypt(packet, size);

                Socket.BeginSend(packet, 0, size, SocketFlags.None, EndSend, packet);
                Ref.PacketPerSecond++;
            }
            catch (Exception e)
            {
                BufferPool.RecycleBuffer(packet);
                Output.WriteLine(e);
                Disconnect();
            }
        } 


Now, all those IAsyncResults create enough allocations on their own, so I need a way of getting rid of them. Is there something I can do other than implementing SAEA?

Any improvements/flaws I should take care of? Socket's never really been my strength - this is my first attempt on good ones Of course you could argue if I should call anything coding related a strength of mine haha

Fang will destroy me for sure

[pro4never]

There generally should not be any need to merge packets yourself as the network layer should already do that dynamically.

Eg if you try to send data to a socket multiple times before one send completes it merges the data together. I forget the name of the algorithm but it's built right into .net sockets and is something you have to manually disable if you didn't want to use it.

[KraHen]

Originally Posted by pro4never

There generally should not be any need to merge packets yourself as the network layer should already do that dynamically.

Eg if you try to send data to a socket multiple times before one send completes it merges the data together. I forget the name of the algorithm but it's built right into .net sockets and is something you have to manually disable if you didn't want to use it.

Nagle. For CO it doesn't make that much of a difference. (Since you're handling the ping packet the nagle wont cause a big delay)

[Best Coder 2014]

Originally Posted by KraHen

Nagle. For CO it doesn't make that much of a difference. (Since you're handling the ping packet the nagle wont cause a big delay)

Disabling Nagle actually reduces latency in CO by a noticeable amount.

[KraHen]

Originally Posted by Best Coder 2014

Disabling Nagle actually reduces latency in CO by a noticeable amount.

To be hones I never tried with Nagle enabled in .NET, but a boost::asio implementation didn't yield much of a difference (though they may be implemented differently, so yeah, not a solid argument).

[pro4never]

Originally Posted by KraHen

Nagle. For CO it doesn't make that much of a difference. (Since you're handling the ping packet the nagle wont cause a big delay)

Yah, I more meant to imply instead of doing it himself, he could just let the socket system handle it.

[U2_Caparzo]

Other than saea, i cant think a decent way of avoiding IAsyncResult allocations (discarding threadpool and creating new threads obv.)

Saea looks a bit weird at first, but aren' really hard to understand, and lets you do 2 little optimizations that the APM cant:
-Create one big buffer for app. instead of one little buffer for socket, this reduces the memory fragmentation for obvious reasons. Creating a buffer pool using BeginX would be the other choice, doing this at the start plus a gc call, wondering if that would be a better approach since never researched more about the gc, my theory is that long living objects created at the start + a gc call my reduce the fragmentation, this remains as a supposition
-Saea instances can be reused, IAsyncResults will always be created with the APM pattern.

Wondering how your packet queue affects ping and server performance against send call per packet

[Xio.]

There generally should not be any need to merge packets yourself as the network layer should already do that dynamically.

Eg if you try to send data to a socket multiple times before one send completes it merges the data together. I forget the name of the algorithm but it's built right into .net sockets and is something you have to manually disable if you didn't want to use it.

That's not it's purpose. It's so I can combine many small packets into a single bigger one to reduce the amount of packets being sent. Less packets = better ping. Especially for players on poor quality net.

Disabling Nagle actually reduces latency in CO by a noticeable amount.

Nagle is already disabled

Originally Posted by U2_Caparzo

Other than saea, i cant think a decent way of avoiding IAsyncResult allocations (discarding threadpool and creating new threads obv.)

Saea looks a bit weird at first, but aren' really hard to understand, and lets you do 2 little optimizations that the APM cant:
-Create one big buffer for app. instead of one little buffer for socket, this reduces the memory fragmentation for obvious reasons. Creating a buffer pool using BeginX would be the other choice, doing this at the start plus a gc call, wondering if that would be a better approach since never researched more about the gc, my theory is that long living objects created at the start + a gc call my reduce the fragmentation, this remains as a supposition
-Saea instances can be reused, IAsyncResults will always be created with the APM pattern.

Wondering how your packet queue affects ping and server performance against send call per packet

Thanks,

There is almost no change in Ping as I'm blocking for 30ms max - conquer has a local ping of 24-31 I might add at this point. Also packets like Ping or MsgTick bypass the queue entirely and get sent right away.

[Spirited]

Stop it. This is ridiculous. Allocating memory for a buffer isn't expensive, and the socket system already allocates a buffer for you. You know what is expensive? Locking. Concurrency doesn't mean something can run in parallel. Having a "ConcurrentStack" popping off buffers means it's going to lock up there each time you want to get a buffer, which is far less efficient than just allocating stack memory when a function/method is called like how C# sockets work. Get rid of your Buffer cesspool factory and just use what's already available in C# and provided by the language. I mean, C# is crappy, but you can trust that it'll allocate memory properly for you. ****.

In general, socket systems aren't the worst offenders. Yes, it's the entry point and it's important that you handle packets correctly, but almost all servers do this just fine. The problem comes down to threading and how you handle the packets after the socket system. Like with Redux - it was basically unusable because of the Monster AI. And then I see this "OutgoingPacketQueue" you wrote. It's a single thread and a single concurrent packet queue that's going to lock up like crazy throughout your entire server. Come on man... you're looking for an event based socket system. C# can provide that to you using ThreadPool. I haven't seen a single person here use it correctly. Give that a try instead of whatever the **** this is.

Edit: Also, Nagle's Algorithm is a feature of the operating system, not of .NET.

[iBotx]

Originally Posted by Spirited

Stop it. This is ridiculous. Allocating memory for a buffer isn't expensive, and the socket system already allocates a buffer for you. You know what is expensive? Locking. Concurrency doesn't mean something can run in parallel. Having a "ConcurrentStack" popping off buffers means it's going to lock up there each time you want to get a buffer, which is far less efficient than just allocating stack memory when a function/method is called like how C# sockets work. Get rid of your Buffer cesspool factory and just use what's already available in C# and provided by the language. I mean, C# is crappy, but you can trust that it'll allocate memory properly for you. ****.

In general, socket systems aren't the worst offenders. Yes, it's the entry point and it's important that you handle packets correctly, but almost all servers do this just fine. The problem comes down to threading and how you handle the packets after the socket system. Like with Redux - it was basically unusable because of the Monster AI. And then I see this "OutgoingPacketQueue" you wrote. It's a single thread and a single concurrent packet queue that's going to lock up like crazy throughout your entire server. Come on man... you're looking for an event based socket system. C# can provide that to you using ThreadPool. I haven't seen a single person here use it correctly. Give that a try instead of whatever the **** this is.

Edit: Also, Nagle's Algorithm is a feature of the operating system, not of .NET.

That escalated quickly.

[Spirited]

Originally Posted by iBotx

That escalated quickly.

I get that you want to stand up for your buddy's code while simultaneously being a salt towards me for whatever harbored reason that I don't care about, but the purpose of this thread is to expose performance issues. I got straight to the point and that's what Yuki wanted. Got a problem with how I write my responses? Report me. Otherwise, do your buddy a favor and point out some weaknesses in his code.

[iBotx]

Originally Posted by Spirited

I get that you want to stand up for your buddy's code while simultaneously being a salt towards me for whatever harbored reason that I don't care about, but the purpose of this thread is to expose performance issues. I got straight to the point and that's what Yuki wanted. Got a problem with how I write my responses? Report me. Otherwise, do your buddy a favor and point out some weaknesses in his code.

First of all, he's neither my buddy nor i'm criticizing you. Actually all i look in the code is it's effeciency and keeping it simple as possible.

[Spirited]

Originally Posted by iBotx

First of all, he's neither my buddy nor i'm criticizing you. Actually all i look in the code is it's effeciency and keeping it simple as possible.

I see. Well, I don't see it as such a huge escalation to question the purpose and side-effects of the code rather than just the current form. In general, people should be reviewing their code better. Maybe you see efficiency, but all I see from this thread is bloat and unnecessary complexity for a desultory non-problem. It's code like this that just creates overcomplicated systems for the sake of a large code base.

[iBotx]

Originally Posted by Spirited

I see. Well, I don't see it as such a huge escalation to question the purpose and side-effects of the code rather than just the current form. In general, people should be reviewing their code better. Maybe you see efficiency, but all I see from this thread is bloat and unnecessary complexity for a desultory non-problem. It's code like this that just creates overcomplicated systems for the sake of a large code base.

No i was talking about my own code, not Yuki's. I didn't even read it.

[Spirited]

Originally Posted by iBotx

No i was talking about my own code, not Yuki's. I didn't even read it.

Ah, yeah. Simplicity is often under appreciated.