From 007ce24a111372ddd0d4d5a9fbe862cea430fa4b Mon Sep 17 00:00:00 2001 From: Jozef Behran Date: Sun, 14 Apr 2019 15:56:38 -0500 Subject: [PATCH] Various network performance improvements (#8125) * Optimize packet construction functions Some of the functions that construct packets in connection.cpp are using a const reference to get the raw packet data to package and others use a value passed parameter to do that. The ones that use the value passed parameter suffer from performance hit as the rather bulky packet data gets a temporary copy when the parameter is passed before it lands at its final destination inside the newly constructed packet. The unnecessary temporary copy hurts quite badly as the underlying class (SharedBuffer) actually allocates the space for the data in the heap. Fix the performance hit by converting all of these value passed parameters to const references. I believe that this is what the author of the relevant code actually intended to do as there is a couple of packet construction helper functions that already use a const reference to get the raw data. * Optimize packet sender thread class Most of the data sending methods of the packet sender thread class use a value passed parameter for the packet data to be sent. This causes the rather bulky data to be allocated on the heap and copied, slowing the packet sending down. Convert these parameters to const references to avoid the performance hit. * Optimize packet receiver thread class The packet receiver and processor thread class has many methods (mostly packet handlers) that receive the packed data by value. This causes a performance hit that is actually worse than the one caused by the packet sender methods because the packet is first handed to the processPacket method which looks at the packet type stored in the header and then delegates the actual handling to one of the handlers. Both, processPacket and all the handlers get the packet data by value, leading to at least two unnecessary copies of the data (with malloc and all the slow bells and whistles of bulky classes). As there already is a few methods that use a const reference parameter for the packet data, convert all this value passed packets to const references. --- src/network/connection.cpp | 6 +++--- src/network/connection.h | 6 +++--- src/network/connectionthreads.cpp | 18 +++++++++--------- src/network/connectionthreads.h | 25 +++++++++++++------------ 4 files changed, 28 insertions(+), 27 deletions(-) diff --git a/src/network/connection.cpp b/src/network/connection.cpp index 28084c921..9fb43179d 100644 --- a/src/network/connection.cpp +++ b/src/network/connection.cpp @@ -56,7 +56,7 @@ std::mutex log_message_mutex; #define PING_TIMEOUT 5.0 -BufferedPacket makePacket(Address &address, SharedBuffer data, +BufferedPacket makePacket(Address &address, const SharedBuffer &data, u32 protocol_id, session_t sender_peer_id, u8 channel) { u32 packet_size = data.getSize() + BASE_HEADER_SIZE; @@ -126,7 +126,7 @@ void makeSplitPacket(const SharedBuffer &data, u32 chunksize_max, u16 seqnum } } -void makeAutoSplitPacket(SharedBuffer data, u32 chunksize_max, +void makeAutoSplitPacket(const SharedBuffer &data, u32 chunksize_max, u16 &split_seqnum, std::list> *list) { u32 original_header_size = 1; @@ -140,7 +140,7 @@ void makeAutoSplitPacket(SharedBuffer data, u32 chunksize_max, list->push_back(makeOriginalPacket(data)); } -SharedBuffer makeReliablePacket(SharedBuffer data, u16 seqnum) +SharedBuffer makeReliablePacket(const SharedBuffer &data, u16 seqnum) { u32 header_size = 3; u32 packet_size = data.getSize() + header_size; diff --git a/src/network/connection.h b/src/network/connection.h index 346c7f886..6b3826151 100644 --- a/src/network/connection.h +++ b/src/network/connection.h @@ -103,16 +103,16 @@ struct BufferedPacket }; // This adds the base headers to the data and makes a packet out of it -BufferedPacket makePacket(Address &address, SharedBuffer data, +BufferedPacket makePacket(Address &address, const SharedBuffer &data, u32 protocol_id, session_t sender_peer_id, u8 channel); // Depending on size, make a TYPE_ORIGINAL or TYPE_SPLIT packet // Increments split_seqnum if a split packet is made -void makeAutoSplitPacket(SharedBuffer data, u32 chunksize_max, +void makeAutoSplitPacket(const SharedBuffer &data, u32 chunksize_max, u16 &split_seqnum, std::list> *list); // Add the TYPE_RELIABLE header to the data -SharedBuffer makeReliablePacket(SharedBuffer data, u16 seqnum); +SharedBuffer makeReliablePacket(const SharedBuffer &data, u16 seqnum); struct IncomingSplitPacket { diff --git a/src/network/connectionthreads.cpp b/src/network/connectionthreads.cpp index 63a9064e7..f8b58c025 100644 --- a/src/network/connectionthreads.cpp +++ b/src/network/connectionthreads.cpp @@ -327,7 +327,7 @@ void ConnectionSendThread::sendAsPacketReliable(BufferedPacket &p, Channel *chan } bool ConnectionSendThread::rawSendAsPacket(session_t peer_id, u8 channelnum, - SharedBuffer data, bool reliable) + const SharedBuffer &data, bool reliable) { PeerHelper peer = m_connection->getPeerNoEx(peer_id); if (!peer) { @@ -575,7 +575,7 @@ void ConnectionSendThread::disconnect_peer(session_t peer_id) } void ConnectionSendThread::send(session_t peer_id, u8 channelnum, - SharedBuffer data) + const SharedBuffer &data) { assert(channelnum < CHANNEL_COUNT); // Pre-condition @@ -615,7 +615,7 @@ void ConnectionSendThread::sendReliable(ConnectionCommand &c) peer->PutReliableSendCommand(c, m_max_packet_size); } -void ConnectionSendThread::sendToAll(u8 channelnum, SharedBuffer data) +void ConnectionSendThread::sendToAll(u8 channelnum, const SharedBuffer &data) { std::list peerids = m_connection->getPeerIDs(); @@ -776,7 +776,7 @@ void ConnectionSendThread::sendPackets(float dtime) } void ConnectionSendThread::sendAsPacket(session_t peer_id, u8 channelnum, - SharedBuffer data, bool ack) + const SharedBuffer &data, bool ack) { OutgoingPacket packet(peer_id, channelnum, data, false, ack); m_outgoing_queue.push(packet); @@ -1086,7 +1086,7 @@ bool ConnectionReceiveThread::checkIncomingBuffers(Channel *channel, } SharedBuffer ConnectionReceiveThread::processPacket(Channel *channel, - SharedBuffer packetdata, session_t peer_id, u8 channelnum, bool reliable) + const SharedBuffer &packetdata, session_t peer_id, u8 channelnum, bool reliable) { PeerHelper peer = m_connection->getPeerNoEx(peer_id); @@ -1125,7 +1125,7 @@ const ConnectionReceiveThread::PacketTypeHandler }; SharedBuffer ConnectionReceiveThread::handlePacketType_Control(Channel *channel, - SharedBuffer packetdata, Peer *peer, u8 channelnum, bool reliable) + const SharedBuffer &packetdata, Peer *peer, u8 channelnum, bool reliable) { if (packetdata.getSize() < 2) throw InvalidIncomingDataException("packetdata.getSize() < 2"); @@ -1224,7 +1224,7 @@ SharedBuffer ConnectionReceiveThread::handlePacketType_Control(Channel *chan } SharedBuffer ConnectionReceiveThread::handlePacketType_Original(Channel *channel, - SharedBuffer packetdata, Peer *peer, u8 channelnum, bool reliable) + const SharedBuffer &packetdata, Peer *peer, u8 channelnum, bool reliable) { if (packetdata.getSize() <= ORIGINAL_HEADER_SIZE) throw InvalidIncomingDataException @@ -1238,7 +1238,7 @@ SharedBuffer ConnectionReceiveThread::handlePacketType_Original(Channel *cha } SharedBuffer ConnectionReceiveThread::handlePacketType_Split(Channel *channel, - SharedBuffer packetdata, Peer *peer, u8 channelnum, bool reliable) + const SharedBuffer &packetdata, Peer *peer, u8 channelnum, bool reliable) { Address peer_address; @@ -1269,7 +1269,7 @@ SharedBuffer ConnectionReceiveThread::handlePacketType_Split(Channel *channe } SharedBuffer ConnectionReceiveThread::handlePacketType_Reliable(Channel *channel, - SharedBuffer packetdata, Peer *peer, u8 channelnum, bool reliable) + const SharedBuffer &packetdata, Peer *peer, u8 channelnum, bool reliable) { assert(channel != NULL); diff --git a/src/network/connectionthreads.h b/src/network/connectionthreads.h index ddac4ae20..da4ea92f5 100644 --- a/src/network/connectionthreads.h +++ b/src/network/connectionthreads.h @@ -52,8 +52,8 @@ public: private: void runTimeouts(float dtime); void rawSend(const BufferedPacket &packet); - bool rawSendAsPacket(session_t peer_id, u8 channelnum, SharedBuffer data, - bool reliable); + bool rawSendAsPacket(session_t peer_id, u8 channelnum, + const SharedBuffer &data, bool reliable); void processReliableCommand(ConnectionCommand &c); void processNonReliableCommand(ConnectionCommand &c); @@ -61,14 +61,14 @@ private: void connect(Address address); void disconnect(); void disconnect_peer(session_t peer_id); - void send(session_t peer_id, u8 channelnum, SharedBuffer data); + void send(session_t peer_id, u8 channelnum, const SharedBuffer &data); void sendReliable(ConnectionCommand &c); - void sendToAll(u8 channelnum, SharedBuffer data); + void sendToAll(u8 channelnum, const SharedBuffer &data); void sendToAllReliable(ConnectionCommand &c); void sendPackets(float dtime); - void sendAsPacket(session_t peer_id, u8 channelnum, SharedBuffer data, + void sendAsPacket(session_t peer_id, u8 channelnum, const SharedBuffer &data, bool ack = false); void sendAsPacketReliable(BufferedPacket &p, Channel *channel); @@ -119,26 +119,27 @@ private: channelnum: channel on which the packet was sent reliable: true if recursing into a reliable packet */ - SharedBuffer processPacket(Channel *channel, SharedBuffer packetdata, - session_t peer_id, u8 channelnum, bool reliable); + SharedBuffer processPacket(Channel *channel, + const SharedBuffer &packetdata, session_t peer_id, + u8 channelnum, bool reliable); SharedBuffer handlePacketType_Control(Channel *channel, - SharedBuffer packetdata, Peer *peer, u8 channelnum, + const SharedBuffer &packetdata, Peer *peer, u8 channelnum, bool reliable); SharedBuffer handlePacketType_Original(Channel *channel, - SharedBuffer packetdata, Peer *peer, u8 channelnum, + const SharedBuffer &packetdata, Peer *peer, u8 channelnum, bool reliable); SharedBuffer handlePacketType_Split(Channel *channel, - SharedBuffer packetdata, Peer *peer, u8 channelnum, + const SharedBuffer &packetdata, Peer *peer, u8 channelnum, bool reliable); SharedBuffer handlePacketType_Reliable(Channel *channel, - SharedBuffer packetdata, Peer *peer, u8 channelnum, + const SharedBuffer &packetdata, Peer *peer, u8 channelnum, bool reliable); struct PacketTypeHandler { SharedBuffer (ConnectionReceiveThread::*handler)(Channel *channel, - SharedBuffer packet, Peer *peer, u8 channelnum, + const SharedBuffer &packet, Peer *peer, u8 channelnum, bool reliable); };