refactor server code to avoid memcpy and extra stack allocation

AnTCP.Client.Sample/AnTCP.Client.Sample.csproj

@@ -2,7 +2,7 @@
 
   <PropertyGroup>
     <OutputType>Exe</OutputType>
-    <TargetFramework>net5.0</TargetFramework>
+    <TargetFramework>net7.0-windows</TargetFramework>
   </PropertyGroup>
 
   <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|AnyCPU'">

AnTCP.Client.Sample/Program.cs

@@ -1,5 +1,6 @@
 using AnTCP.Client.Objects;
 using System;
+using System.Diagnostics;
 using System.Text.Json;
 
 namespace AnTCP.Client.Sample
@@ -42,9 +43,12 @@ private static void Main(string[] args)
                     (int, int) data = (new Random().Next(1, 11), new Random().Next(1, 11));
 
                     Console.WriteLine($"\n>> Sending: {data.Item1}, {data.Item2}");
+
+                    Stopwatch sw = Stopwatch.StartNew();
                     AnTcpResponse response = client.Send((byte)new Random().Next(0, 4), data);
+                    sw.Stop();
 
-                    Console.WriteLine($">> Response: {response.Length} bytes | Type: {(MessageType)response.Type}");
+                    Console.WriteLine($">> Response ({sw.Elapsed}): {response.Length} bytes | Type: {(MessageType)response.Type}");
 
                     switch ((MessageType)response.Type)
                     {

AnTCP.Client/AnTCP.Client.csproj

@@ -1,7 +1,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
 
   <PropertyGroup>
-    <TargetFramework>net5.0</TargetFramework>
+    <TargetFramework>net7.0</TargetFramework>
   </PropertyGroup>
 
   <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|AnyCPU'">

AnTCP.Server.Sample/AnTCP.Server.Sample.vcxproj

@@ -29,26 +29,26 @@
   <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'" Label="Configuration">
     <ConfigurationType>Application</ConfigurationType>
     <UseDebugLibraries>true</UseDebugLibraries>
-    <PlatformToolset>v142</PlatformToolset>
+    <PlatformToolset>v143</PlatformToolset>
     <CharacterSet>Unicode</CharacterSet>
   </PropertyGroup>
   <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|Win32'" Label="Configuration">
     <ConfigurationType>Application</ConfigurationType>
     <UseDebugLibraries>false</UseDebugLibraries>
-    <PlatformToolset>v142</PlatformToolset>
+    <PlatformToolset>v143</PlatformToolset>
     <WholeProgramOptimization>true</WholeProgramOptimization>
     <CharacterSet>Unicode</CharacterSet>
   </PropertyGroup>
   <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|x64'" Label="Configuration">
     <ConfigurationType>Application</ConfigurationType>
     <UseDebugLibraries>true</UseDebugLibraries>
-    <PlatformToolset>v142</PlatformToolset>
+    <PlatformToolset>v143</PlatformToolset>
     <CharacterSet>Unicode</CharacterSet>
   </PropertyGroup>
   <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|x64'" Label="Configuration">
     <ConfigurationType>Application</ConfigurationType>
     <UseDebugLibraries>false</UseDebugLibraries>
-    <PlatformToolset>v142</PlatformToolset>
+    <PlatformToolset>v143</PlatformToolset>
     <WholeProgramOptimization>true</WholeProgramOptimization>
     <CharacterSet>Unicode</CharacterSet>
   </PropertyGroup>
@@ -119,6 +119,9 @@
       <LanguageStandard>stdcpplatest</LanguageStandard>
       <StringPooling>true</StringPooling>
       <EnableEnhancedInstructionSet>StreamingSIMDExtensions2</EnableEnhancedInstructionSet>
+      <BufferSecurityCheck>false</BufferSecurityCheck>
+      <FavorSizeOrSpeed>Speed</FavorSizeOrSpeed>
+      <OmitFramePointers>true</OmitFramePointers>
     </ClCompile>
     <Link>
       <SubSystem>Console</SubSystem>
@@ -153,6 +156,9 @@
       <LanguageStandard>stdcpplatest</LanguageStandard>
       <StringPooling>true</StringPooling>
       <EnableEnhancedInstructionSet>StreamingSIMDExtensions2</EnableEnhancedInstructionSet>
+      <BufferSecurityCheck>false</BufferSecurityCheck>
+      <FavorSizeOrSpeed>Speed</FavorSizeOrSpeed>
+      <OmitFramePointers>true</OmitFramePointers>
     </ClCompile>
     <Link>
       <SubSystem>Console</SubSystem>

AnTCP.Server/AnTCP.Server.vcxproj

@@ -29,26 +29,26 @@
   <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|Win32'" Label="Configuration">
     <ConfigurationType>StaticLibrary</ConfigurationType>
     <UseDebugLibraries>true</UseDebugLibraries>
-    <PlatformToolset>v142</PlatformToolset>
+    <PlatformToolset>v143</PlatformToolset>
     <CharacterSet>Unicode</CharacterSet>
   </PropertyGroup>
   <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|Win32'" Label="Configuration">
     <ConfigurationType>StaticLibrary</ConfigurationType>
     <UseDebugLibraries>false</UseDebugLibraries>
-    <PlatformToolset>v142</PlatformToolset>
+    <PlatformToolset>v143</PlatformToolset>
     <WholeProgramOptimization>true</WholeProgramOptimization>
     <CharacterSet>Unicode</CharacterSet>
   </PropertyGroup>
   <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Debug|x64'" Label="Configuration">
     <ConfigurationType>StaticLibrary</ConfigurationType>
     <UseDebugLibraries>true</UseDebugLibraries>
-    <PlatformToolset>v142</PlatformToolset>
+    <PlatformToolset>v143</PlatformToolset>
     <CharacterSet>Unicode</CharacterSet>
   </PropertyGroup>
   <PropertyGroup Condition="'$(Configuration)|$(Platform)'=='Release|x64'" Label="Configuration">
     <ConfigurationType>StaticLibrary</ConfigurationType>
     <UseDebugLibraries>false</UseDebugLibraries>
-    <PlatformToolset>v142</PlatformToolset>
+    <PlatformToolset>v143</PlatformToolset>
     <WholeProgramOptimization>true</WholeProgramOptimization>
     <CharacterSet>Unicode</CharacterSet>
   </PropertyGroup>
@@ -114,6 +114,9 @@
       <LanguageStandard>stdcpplatest</LanguageStandard>
       <EnableEnhancedInstructionSet>StreamingSIMDExtensions2</EnableEnhancedInstructionSet>
       <StringPooling>true</StringPooling>
+      <OmitFramePointers>true</OmitFramePointers>
+      <FavorSizeOrSpeed>Speed</FavorSizeOrSpeed>
+      <BufferSecurityCheck>false</BufferSecurityCheck>
     </ClCompile>
     <Link>
       <SubSystem>Console</SubSystem>
@@ -146,6 +149,9 @@
       <LanguageStandard>stdcpplatest</LanguageStandard>
       <EnableEnhancedInstructionSet>StreamingSIMDExtensions2</EnableEnhancedInstructionSet>
       <StringPooling>true</StringPooling>
+      <OmitFramePointers>true</OmitFramePointers>
+      <FavorSizeOrSpeed>Speed</FavorSizeOrSpeed>
+      <BufferSecurityCheck>false</BufferSecurityCheck>
     </ClCompile>
     <Link>
       <SubSystem>Console</SubSystem>

AnTCP.Server/src/AnTcpServer.cpp

@@ -94,20 +94,12 @@ AnTcpError AnTcpServer::Run() noexcept
 
 void ClientHandler::Listen() noexcept
 {
-    // buffer for the incoming bytes
-    char recvbuf[ANTCP_BUFFER_LENGTH];
-
-    // the index of our current header
-    int headerPosition = 0;
-    // buffer for the header
-    char header[sizeof(AnTcpSizeType)]{ 0 };
-
-    // the total packet size
+    // the total packet size and data ptr offset
     AnTcpSizeType packetSize = 0;
-    // the amount of bytes we are missing to complete the packet
-    int packetBytesMissing = 0;
+    AnTcpSizeType packetOffset = 0;
+
     // buffer for the packet
-    char packet[ANTCP_MAX_PACKET_SIZE]{ 0 };
+    char packet[sizeof(AnTcpSizeType) + ANTCP_MAX_PACKET_SIZE]{ 0 };
 
     if (OnClientConnected)
     {
@@ -116,92 +108,61 @@ void ClientHandler::Listen() noexcept
 
     while (!ShouldExit)
     {
-        int incomingBytes = recv(Socket, recvbuf, ANTCP_BUFFER_LENGTH, 0);
-        int bytesProcessed = 0;
+        auto packetBytesMissing = packetSize - packetOffset;
+        // how many bytes to receive at most, if packet is beeing built it's the remaining packets size 
+        // otherwise the size of the AnTcpSizeType which tells us how big the packet will be
+        auto maxReceiveSize = packetBytesMissing > 0 ? packetBytesMissing : sizeof(AnTcpSizeType);
+
+        auto receivedPacketBytes = recv(Socket, packet + packetOffset, maxReceiveSize, 0);
+        packetOffset += receivedPacketBytes;
 
         // if we received 0 or -1 bytes, we're going to disconnect the client
-        if (incomingBytes <= 0)
+        if (receivedPacketBytes <= 0)
         {
             break;
         }
 
-        DEBUG_ONLY(std::cout << "[" << Id << "] " << "Received " << std::to_string(incomingBytes) + " bytes" << std::endl);
+        DEBUG_ONLY(std::cout << "[" << Id << "] " << "Received " << std::to_string(receivedPacketBytes) + " bytes" << std::endl);
 
-        do
+        if (packetSize == 0)
         {
-            if (packetBytesMissing > 0)
+            // wait until the transmission of the AnTcpSizeType is completed to determine packet size
+            if (packetOffset >= sizeof(AnTcpSizeType))
             {
-                // gather bytes complete the packet
-                const int bytesToCopy = std::min(incomingBytes - bytesProcessed, packetBytesMissing);
-
-                DEBUG_ONLY(std::cout << "[" << Id << "] " << "Packet Chunk: " << std::to_string(bytesToCopy) << " bytes ("
-                    << std::to_string(packetSize - (packetBytesMissing - bytesToCopy))
-                    << "/" << std::to_string(packetSize) << ")" << std::endl);
+                packetSize = *reinterpret_cast<AnTcpSizeType*>(packet);
 
-                // copy data from incoming buffer to packet buffer
-                memcpy(packet + (packetSize - packetBytesMissing), recvbuf + bytesProcessed, bytesToCopy);
-                bytesProcessed += bytesToCopy;
-
-                // check whether we still miss some bytes
-                packetBytesMissing -= bytesToCopy;
-
-                if (packetBytesMissing == 0 && !ProcessPacket(packet, packetSize))
+                if (packetSize > ANTCP_MAX_PACKET_SIZE)
                 {
-                    Disconnect();
-                    return;
+                    // packet is too big, this may be a wrong/malicious payload
+                    DEBUG_ONLY(std::cout << "[" << Id << "] " << "Packet too big (" << std::to_string(packetSize)
+                        << "/" << ANTCP_MAX_PACKET_SIZE << "), disconnecting client..." << std::endl);
+                    break;
                 }
+
+                DEBUG_ONLY(std::cout << "[" << Id << "] " << "New Packet: " << std::to_string(packetSize) + " bytes" << std::endl);
             }
-            else
+        }
+        else
+        {
+            // we know the packet's size, wait for it to be complete
+            if (packetBytesMissing == 0)
             {
-                if (headerPosition < static_cast<int>(sizeof(AnTcpSizeType)))
+                if (!ProcessPacket(packet + sizeof(AnTcpSizeType), packetSize))
                 {
-                    // we still need to finish building the header
-                    const int bytesToCopy = std::min(incomingBytes - bytesProcessed, static_cast<int>(sizeof(AnTcpSizeType)) - headerPosition);
-
-                    // copy data from buffer to header buffer
-                    memcpy(header + headerPosition, recvbuf + bytesProcessed, bytesToCopy);
-                    bytesProcessed += bytesToCopy;
-                    headerPosition += bytesToCopy;
-                }
-                else
-                {
-                    // header is completed, begin to build packet
-                    headerPosition = 0;
-                    packetSize = *reinterpret_cast<AnTcpSizeType*>(header);
-
-                    if (packetSize > ANTCP_MAX_PACKET_SIZE)
-                    {
-                        // packet is too big, this may be a wrong/malicious payload
-                        DEBUG_ONLY(std::cout << "[" << Id << "] " << "Packet too big (" << std::to_string(packetSize)
-                            << "/" << ANTCP_MAX_PACKET_SIZE << "), disconnecting client..." << std::endl);
-
-                        Disconnect();
-                        return;
-                    }
-                    else
-                    {
-                        DEBUG_ONLY(std::cout << "[" << Id << "] " << "New Packet: " << std::to_string(packetSize) + " bytes" << std::endl);
-
-                        if (incomingBytes >= packetSize)
-                        {
-                            // we already received the full packet, so process it directly
-                            if (!ProcessPacket(recvbuf + bytesProcessed, packetSize))
-                            {
-                                Disconnect();
-                                return;
-                            }
-
-                            bytesProcessed += packetSize;
-                        }
-                        else
-                        {
-                            // some bytes are missing
-                            packetBytesMissing = packetSize;
-                        }
-                    }
+                    // processing the packet failed, disconnect client
+                    break;
                 }
+
+                // reset packet buffer
+                packetSize = 0;
+                packetOffset = 0;
             }
-        } while (bytesProcessed < incomingBytes);
+            else
+            {
+                DEBUG_ONLY(std::cout << "[" << Id << "] " << "Packet Chunk: " << std::to_string(receivedPacketBytes) << " bytes ("
+                    << std::to_string(packetOffset) << "/" << std::to_string(packetSize) << ")" << std::endl);
+            }
+        }
     }
 
     Disconnect();

AnTCP.Server/src/AnTcpServer.hpp

@@ -25,8 +25,7 @@
 #include <ws2tcpip.h>
 #include <iphlpapi.h>
 
-constexpr auto ANTCP_SERVER_VERSION = "1.1.0.0";
-constexpr auto ANTCP_BUFFER_LENGTH = 512;
+constexpr auto ANTCP_SERVER_VERSION = "1.2.0.0";
 constexpr auto ANTCP_MAX_PACKET_SIZE = 256;
 
 // type used in the payload to specify the size of a packet
@@ -54,7 +53,6 @@ class ClientHandler
     std::atomic<bool>& ShouldExit;
     std::unordered_map <AnTcpMessageType, std::function<void(ClientHandler*, AnTcpMessageType, const void*, int)>>* Callbacks;
 
-    unsigned int UniqueId;
     bool IsActive;
     std::thread* Thread;
 
@@ -79,12 +77,12 @@ class ClientHandler
         std::function<void(ClientHandler*)>* onClientConnected = nullptr,
         std::function<void(ClientHandler*)>* onClientDisconnected = nullptr
     )
-        : IsActive(true),
-        Id(static_cast<unsigned int>(socketInfo.sin_addr.S_un.S_addr + socketInfo.sin_port)),
+        : Id(static_cast<unsigned int>(socketInfo.sin_addr.S_un.S_addr + socketInfo.sin_port)),
         Socket(socket),
         SocketInfo(socketInfo),
         ShouldExit(shouldExit),
         Callbacks(callbacks),
+        IsActive(true),
         Thread(new std::thread(&ClientHandler::Listen, this)),
         OnClientConnected(onClientConnected),
         OnClientDisconnected(onClientDisconnected)
@@ -212,27 +210,27 @@ class ClientHandler
     /// <param name="type">Message type.</param>
     /// <param name="data">Data received.</param>
     /// <returns>True if callback was fired, false if not.</returns>
-    inline bool ProcessPacket(const char* data, int size) noexcept
+    inline bool ProcessPacket(const char* data, AnTcpMessageType size) noexcept
     {
-        const AnTcpMessageType msgType = *reinterpret_cast<const AnTcpMessageType*>(data);
+        auto msgType = *reinterpret_cast<const AnTcpMessageType*>(data);
 
         if ((*Callbacks).contains(msgType))
         {
             // measure packet processing time in debug mode
             BENCHMARK(const auto packetStart = std::chrono::high_resolution_clock::now());
 
             // fire the callback with the raw data
-            (*Callbacks)[msgType](this, msgType, data + static_cast<int>(sizeof(AnTcpMessageType)), size - static_cast<int>(sizeof(AnTcpMessageType)));
+            (*Callbacks)[msgType](this, msgType, data + sizeof(AnTcpMessageType), size - sizeof(AnTcpMessageType));
 
             BENCHMARK(std::cout << "[" << Id << "] " << "Processing packet of type \""
-                << std::to_string(*reinterpret_cast<const AnTcpMessageType*>(data)) << "\" took: "
+                << std::to_string(msgType) << "\" took: "
                 << std::chrono::duration_cast<std::chrono::microseconds>(std::chrono::high_resolution_clock::now() - packetStart) << std::endl);
 
             return true;
         }
 
-        DEBUG_ONLY(std::cout << "[" << Id << "] " << "\"" << std::to_string(*reinterpret_cast<const AnTcpMessageType*>(data))
-            << "\" is an unknown message type, disconnecting client..." << std::endl);
+        DEBUG_ONLY(std::cout << "[" << Id << "] " << "\"" << std::to_string(msgType)
+            << "\" is an unknown message type..." << std::endl);
 
         return false;
     }