0

I wrote a C# game server that usually has several hundred people connected to it, and based it on this example from Microsoft. The players are always sending data back and forth, and the connections are rarely idle. I have started wondering though: is every player connection launching its own Thread? Would it be better performance-wise if I recoded it to use 'Tasks/Await/etc' instead?

[EDIT] Is the way I'm calling Thread.Sleep() an appropriate way to throttle someone's connection? Or, instead of using Thread.Sleep(), should I have it launch a task with Task.Delay() that contains SendBytes()

public class Server
{
    void Listen()
    {
        IPAddress IpAddr = IPAddress.Parse(ipv4_local);
        IPEndPoint ipEnd = new IPEndPoint(IpAddr, port);
        listener = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);
        listener.Bind(ipEnd);
        listener.Listen(5000);

        while (true)
        {
            listener_allDone.Reset();
            listener.BeginAccept(new AsyncCallback(AcceptConnection), listener);
            listener_allDone.WaitOne();
        }
    }

    void AcceptConnection(IAsyncResult ar)
    {
        listener_allDone.Set();

        Socket listener = (Socket)ar.AsyncState;
        Socket handler = listener.EndAccept(ar);

        Player player = new Player(handler);
        player.InitReceive();
    }
}

And the player class, where Thread.Sleep() is used to throttle...

public class Player
{
    public Socket socket;
    public byte[] send_buffer = new byte[2048];
    public byte[] receive_buffer = new byte[2048];

    public Player(Socket socket)
    {
        this.socket = socket;
    }

    void SendBytes()
    {
        socket.BeginSend(send_buffer, 0, send_buffer.Length, SocketFlags.None, OnSendComplete, null);
    }

    void OnSendComplete(System.IAsyncResult iar)
    {
        int bytes_sent = socket.EndSend(iar);
        if (MORE BYTES TO SEND)
        {
            Thread.Sleep(100); //THROTTLE
            SendBytes();
        }
    }

    public void InitReceive()
    {
        socket.BeginReceive(receive_buffer, 0, receive_buffer.Length, 0, new AsyncCallback(ReadCallback), this);
    }

    public void ReadCallback(IAsyncResult ar)
    {
        Player player = (Player)ar.AsyncState;
        Socket socket = player.socket;

        int bytesRead = socket.EndReceive(ar);

        if (bytesRead != 0)
        {
            // READ BYTES...

            // PROCESS DATA...

            if (NEED TO SEND RESPONSE)
            {
                SendBytes();
            }

            socket.BeginReceive(player.receive_buffer, 0, player.receive_buffer.Length, 0, new AsyncCallback(ReadCallback), player);
        }
    }
}
Harrison
  • 114
  • 6
  • It is definitely better to use tasks and `async/await` where you can. There is a lower overhead for switching between tasks compared to switching between threads, and they typically use less memory. I've done a lot of work with an asynchronous socket server in my current job, and I noticed a huge increase in performance when switching from thread-based to task-based – Andrew Williamson Dec 12 '21 at 00:58
  • 1
    To add to that, if you have the time, it's well worth looking into the classes in [`System.IO.Pipelines`](https://learn.microsoft.com/en-us/dotnet/standard/io/pipelines). It's fairly new, so it's a bit hard to find examples using these classes, but they are very useful for dealing with sockets and custom protocols – Andrew Williamson Dec 12 '21 at 01:03
  • 3
    That code example from the link is **already** using asynchronous I/O (albeit in a legacy way) via I/O Completion ports (IOCP) and thus uses no thread. Re-writing it to use the contemporary `async/await` might not have any net effect. [Windows has had IOCP way before .NET came along](https://learn.microsoft.com/en-us/windows/win32/fileio/i-o-completion-ports) and so just because C# code isn't using `async/await` does not necessarily mean it isn't using efficient IOCP –  Dec 12 '21 at 02:17
  • 2
    @MickyD The OP specifically mentioned `Thread.Sleep()` and launching a new thread for each player. Yes, using `IAsyncResult` is asynchronous and this is better than using threads, but I would still recommend using tasks over the `IAsyncResult` interface. It's a nicer interface to work with and it gives the same performance benefits. Using the `System.IO.Pipelines` classes should also see a performance boost, as they use `ValueTask` extensively and avoid unnecessary buffer allocations – Andrew Williamson Dec 12 '21 at 22:19
  • @AndrewWilliamson Great info! I attached some pseudo-code to my original question, wondering if it is okay to mix Thread.Sleep() and IAsyncResult without performance penalties? – Harrison Dec 13 '21 at 08:00
  • 1
    @HarrisonWalters have a look at [Task vs Thread](https://stackoverflow.com/a/4130347/2363967). `IAsyncResult` is basically the original implementation of `Task`, before `Task` and the `async/await` keywords were added to C# (You can convert it pretty easily using `Task.Factory.FromAsync(...)`). Your example doesn't actually create a thread per player. When a task completes, and you want to use the result, that callback is being run on a thread in the thread pool. This is good, it's pretty much the same as what would happen if you used `async/await` – Andrew Williamson Dec 13 '21 at 18:17
  • 1
    However, when you call `Thread.Sleep()`, you are reducing the number of available threads in the pool. If you have too many connections calling `Sleep` at the same time, then all the threads in the pool will be put to sleep. I'm pretty sure C# will make sure there is always at least one active thread, so when it gets to this point it will spawn a new one. So your code is not directly creating a thread per connection, but the `Thread.Sleep()` statements will eventually cause new threads to be created. I would recommend using `Task.Delay` instead – Andrew Williamson Dec 13 '21 at 18:21
  • @AndrewWilliamson You are a life saver! Thank you for the insightful comment, it answers all my questions and gives me a good idea how to proceed. The whole reason I asked the question in the first place is because I published the server a few days ago with Thread.Sleep() and the server ended up crashing almost immediately. I wonder if it was causing the limited number of threads on my Azure server to be quickly consumed. – Harrison Dec 13 '21 at 18:54
  • _"The OP specifically mentioned Thread.Sleep() and launching a new thread for each player"_ - that is not what he asked at all. Rewriting it to use `async/await` is not going to solve OPs immediate problem of throttling –  Dec 14 '21 at 02:56
  • Never use `Thread.Sleep` in code responsible for I/O for you run the risk of data loss. If you want to do throttling for a particular connection then simply measure the amount of I/O performed and cease transmission after a certain limit is reached –  Dec 14 '21 at 03:01

0 Answers0