Optimize how packets are queued and transferred

src/openrct2/network/NetworkBase.cpp

@@ -501,13 +501,13 @@ void NetworkBase::Flush()
 {
     if (GetMode() == NETWORK_MODE_CLIENT)
     {
-        _serverConnection->SendQueuedPackets();
+        _serverConnection->SendQueuedData();
     }
     else
     {
         for (auto& it : client_connection_list)
         {
-            it->SendQueuedPackets();
+            it->SendQueuedData();
         }
     }
 }
@@ -2014,7 +2014,7 @@ void NetworkBase::ProcessDisconnectedClients()
         }
 
         // Make sure to send all remaining packets out before disconnecting.
-        connection->SendQueuedPackets();
+        connection->SendQueuedData();
         connection->Socket->Disconnect();
 
         ServerClientDisconnected(connection);

src/openrct2/network/NetworkConnection.cpp

@@ -20,11 +20,13 @@
 using namespace OpenRCT2;
 
 static constexpr size_t kNetworkDisconnectReasonBufSize = 256;
-static constexpr size_t kNetworkBufferSize = 1024 * 64; // 64 KiB, maximum packet size.
+static constexpr size_t kNetworkBufferSize = (1024 * 64) - 1; // 64 KiB, maximum packet size.
     #ifndef DEBUG
 static constexpr size_t kNetworkNoDataTimeout = 20; // Seconds.
     #endif
 
+static_assert(kNetworkBufferSize <= std::numeric_limits<uint16_t>::max(), "kNetworkBufferSize too big, uint16_t is max.");
+
 NetworkConnection::NetworkConnection() noexcept
 {
     ResetLastPacketTime();
@@ -99,60 +101,43 @@ NetworkReadPacket NetworkConnection::ReadPacket()
     return NetworkReadPacket::MoreData;
 }
 
-bool NetworkConnection::SendPacket(NetworkPacket& packet)
+static std::vector<uint8_t> serializePacket(const NetworkPacket& packet)
 {
-    auto header = packet.Header;
-
-    std::vector<uint8_t> buffer;
-    buffer.reserve(sizeof(header) + header.Size);
-
     // NOTE: For compatibility reasons for the master server we need to add sizeof(Header.Id) to the size.
     // Previously the Id field was not part of the header rather part of the body.
-    header.Size += sizeof(header.Id);
+    const auto bodyLength = packet.Data.size() + sizeof(packet.Header.Id);
+
+    Guard::Assert(bodyLength <= std::numeric_limits<uint16_t>::max(), "Packet size too large");
+
+    auto header = packet.Header;
+    header.Size = static_cast<uint16_t>(bodyLength);
     header.Size = Convert::HostToNetwork(header.Size);
     header.Id = ByteSwapBE(header.Id);
 
+    std::vector<uint8_t> buffer;
+    buffer.reserve(sizeof(header) + packet.Data.size());
+
     buffer.insert(buffer.end(), reinterpret_cast<uint8_t*>(&header), reinterpret_cast<uint8_t*>(&header) + sizeof(header));
     buffer.insert(buffer.end(), packet.Data.begin(), packet.Data.end());
 
-    size_t bufferSize = buffer.size() - packet.BytesTransferred;
-    size_t sent = Socket->SendData(buffer.data() + packet.BytesTransferred, bufferSize);
-    if (sent > 0)
-    {
-        packet.BytesTransferred += sent;
-    }
-
-    bool sendComplete = packet.BytesTransferred == buffer.size();
-    if (sendComplete)
-    {
-        RecordPacketStats(packet, true);
-    }
-    return sendComplete;
+    return buffer;
 }
 
-void NetworkConnection::QueuePacket(NetworkPacket&& packet, bool front)
+void NetworkConnection::QueuePacket(const NetworkPacket& packet, bool front)
 {
     if (AuthStatus == NetworkAuth::Ok || !packet.CommandRequiresAuth())
     {
-        packet.Header.Size = static_cast<uint16_t>(packet.Data.size());
+        const auto payload = serializePacket(packet);
         if (front)
         {
-            // If the first packet was already partially sent add new packet to second position
-            if (!_outboundPackets.empty() && _outboundPackets.front().BytesTransferred > 0)
-            {
-                auto it = _outboundPackets.begin();
-                it++; // Second position
-                _outboundPackets.insert(it, std::move(packet));
-            }
-            else
-            {
-                _outboundPackets.push_front(std::move(packet));
-            }
+            _outboundBuffer.insert(_outboundBuffer.begin(), payload.begin(), payload.end());
         }
         else
         {
-            _outboundPackets.push_back(std::move(packet));
+            _outboundBuffer.insert(_outboundBuffer.end(), payload.begin(), payload.end());
         }
+
+        RecordPacketStats(packet, true);
     }
 }
 
@@ -166,11 +151,18 @@ bool NetworkConnection::IsValid() const
     return !ShouldDisconnect && Socket->GetStatus() == SocketStatus::Connected;
 }
 
-void NetworkConnection::SendQueuedPackets()
+void NetworkConnection::SendQueuedData()
 {
-    while (!_outboundPackets.empty() && SendPacket(_outboundPackets.front()))
+    // Send queued packets.
+    if (_outboundBuffer.empty())
     {
-        _outboundPackets.pop_front();
+        return;
+    }
+
+    size_t sent = Socket->SendData(_outboundBuffer.data(), _outboundBuffer.size());
+    if (sent > 0)
+    {
+        _outboundBuffer.erase(_outboundBuffer.begin(), _outboundBuffer.begin() + sent);
     }
 }
 

src/openrct2/network/NetworkConnection.h

@@ -41,19 +41,14 @@ public:
     NetworkConnection() noexcept;
 
     NetworkReadPacket ReadPacket();
-    void QueuePacket(NetworkPacket&& packet, bool front = false);
-    void QueuePacket(const NetworkPacket& packet, bool front = false)
-    {
-        auto copy = packet;
-        return QueuePacket(std::move(copy), front);
-    }
+    void QueuePacket(const NetworkPacket& packet, bool front = false);
 
     // This will not immediately disconnect the client. The disconnect
     // will happen post-tick.
     void Disconnect() noexcept;
 
     bool IsValid() const;
-    void SendQueuedPackets();
+    void SendQueuedData();
     void ResetLastPacketTime() noexcept;
     bool ReceivedPacketRecently() const noexcept;
 
@@ -62,12 +57,11 @@ public:
     void SetLastDisconnectReason(const StringId string_id, void* args = nullptr);
 
 private:
-    std::deque<NetworkPacket> _outboundPackets;
+    std::vector<uint8_t> _outboundBuffer;
     uint32_t _lastPacketTime = 0;
     std::string _lastDisconnectReason;
 
     void RecordPacketStats(const NetworkPacket& packet, bool sending);
-    bool SendPacket(NetworkPacket& packet);
 };
 
 #endif // DISABLE_NETWORK