Skip to content

Commit 007ce24

Browse files
osjcparamat
osjc
authored and
paramat
committedApr 14, 2019
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.
·
5.11.05.1.0
1 parent 4d2ad7c commit 007ce24

File tree

4 files changed

+28
-27
lines changed

4 files changed

+28
-27
lines changed
 

Diff for: ‎src/network/connection.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ std::mutex log_message_mutex;
5656

5757
#define PING_TIMEOUT 5.0
5858

59-
BufferedPacket makePacket(Address &address, SharedBuffer<u8> data,
59+
BufferedPacket makePacket(Address &address, const SharedBuffer<u8> &data,
6060
u32 protocol_id, session_t sender_peer_id, u8 channel)
6161
{
6262
u32 packet_size = data.getSize() + BASE_HEADER_SIZE;
@@ -126,7 +126,7 @@ void makeSplitPacket(const SharedBuffer<u8> &data, u32 chunksize_max, u16 seqnum
126126
}
127127
}
128128

129-
void makeAutoSplitPacket(SharedBuffer<u8> data, u32 chunksize_max,
129+
void makeAutoSplitPacket(const SharedBuffer<u8> &data, u32 chunksize_max,
130130
u16 &split_seqnum, std::list<SharedBuffer<u8>> *list)
131131
{
132132
u32 original_header_size = 1;
@@ -140,7 +140,7 @@ void makeAutoSplitPacket(SharedBuffer<u8> data, u32 chunksize_max,
140140
list->push_back(makeOriginalPacket(data));
141141
}
142142

143-
SharedBuffer<u8> makeReliablePacket(SharedBuffer<u8> data, u16 seqnum)
143+
SharedBuffer<u8> makeReliablePacket(const SharedBuffer<u8> &data, u16 seqnum)
144144
{
145145
u32 header_size = 3;
146146
u32 packet_size = data.getSize() + header_size;

Diff for: ‎src/network/connection.h

+3-3
Original file line numberDiff line numberDiff line change
@@ -103,16 +103,16 @@ struct BufferedPacket
103103
};
104104

105105
// This adds the base headers to the data and makes a packet out of it
106-
BufferedPacket makePacket(Address &address, SharedBuffer<u8> data,
106+
BufferedPacket makePacket(Address &address, const SharedBuffer<u8> &data,
107107
u32 protocol_id, session_t sender_peer_id, u8 channel);
108108

109109
// Depending on size, make a TYPE_ORIGINAL or TYPE_SPLIT packet
110110
// Increments split_seqnum if a split packet is made
111-
void makeAutoSplitPacket(SharedBuffer<u8> data, u32 chunksize_max,
111+
void makeAutoSplitPacket(const SharedBuffer<u8> &data, u32 chunksize_max,
112112
u16 &split_seqnum, std::list<SharedBuffer<u8>> *list);
113113

114114
// Add the TYPE_RELIABLE header to the data
115-
SharedBuffer<u8> makeReliablePacket(SharedBuffer<u8> data, u16 seqnum);
115+
SharedBuffer<u8> makeReliablePacket(const SharedBuffer<u8> &data, u16 seqnum);
116116

117117
struct IncomingSplitPacket
118118
{

Diff for: ‎src/network/connectionthreads.cpp

+9-9
Original file line numberDiff line numberDiff line change
@@ -327,7 +327,7 @@ void ConnectionSendThread::sendAsPacketReliable(BufferedPacket &p, Channel *chan
327327
}
328328

329329
bool ConnectionSendThread::rawSendAsPacket(session_t peer_id, u8 channelnum,
330-
SharedBuffer<u8> data, bool reliable)
330+
const SharedBuffer<u8> &data, bool reliable)
331331
{
332332
PeerHelper peer = m_connection->getPeerNoEx(peer_id);
333333
if (!peer) {
@@ -575,7 +575,7 @@ void ConnectionSendThread::disconnect_peer(session_t peer_id)
575575
}
576576

577577
void ConnectionSendThread::send(session_t peer_id, u8 channelnum,
578-
SharedBuffer<u8> data)
578+
const SharedBuffer<u8> &data)
579579
{
580580
assert(channelnum < CHANNEL_COUNT); // Pre-condition
581581

@@ -615,7 +615,7 @@ void ConnectionSendThread::sendReliable(ConnectionCommand &c)
615615
peer->PutReliableSendCommand(c, m_max_packet_size);
616616
}
617617

618-
void ConnectionSendThread::sendToAll(u8 channelnum, SharedBuffer<u8> data)
618+
void ConnectionSendThread::sendToAll(u8 channelnum, const SharedBuffer<u8> &data)
619619
{
620620
std::list<session_t> peerids = m_connection->getPeerIDs();
621621

@@ -776,7 +776,7 @@ void ConnectionSendThread::sendPackets(float dtime)
776776
}
777777

778778
void ConnectionSendThread::sendAsPacket(session_t peer_id, u8 channelnum,
779-
SharedBuffer<u8> data, bool ack)
779+
const SharedBuffer<u8> &data, bool ack)
780780
{
781781
OutgoingPacket packet(peer_id, channelnum, data, false, ack);
782782
m_outgoing_queue.push(packet);
@@ -1086,7 +1086,7 @@ bool ConnectionReceiveThread::checkIncomingBuffers(Channel *channel,
10861086
}
10871087

10881088
SharedBuffer<u8> ConnectionReceiveThread::processPacket(Channel *channel,
1089-
SharedBuffer<u8> packetdata, session_t peer_id, u8 channelnum, bool reliable)
1089+
const SharedBuffer<u8> &packetdata, session_t peer_id, u8 channelnum, bool reliable)
10901090
{
10911091
PeerHelper peer = m_connection->getPeerNoEx(peer_id);
10921092

@@ -1125,7 +1125,7 @@ const ConnectionReceiveThread::PacketTypeHandler
11251125
};
11261126

11271127
SharedBuffer<u8> ConnectionReceiveThread::handlePacketType_Control(Channel *channel,
1128-
SharedBuffer<u8> packetdata, Peer *peer, u8 channelnum, bool reliable)
1128+
const SharedBuffer<u8> &packetdata, Peer *peer, u8 channelnum, bool reliable)
11291129
{
11301130
if (packetdata.getSize() < 2)
11311131
throw InvalidIncomingDataException("packetdata.getSize() < 2");
@@ -1224,7 +1224,7 @@ SharedBuffer<u8> ConnectionReceiveThread::handlePacketType_Control(Channel *chan
12241224
}
12251225

12261226
SharedBuffer<u8> ConnectionReceiveThread::handlePacketType_Original(Channel *channel,
1227-
SharedBuffer<u8> packetdata, Peer *peer, u8 channelnum, bool reliable)
1227+
const SharedBuffer<u8> &packetdata, Peer *peer, u8 channelnum, bool reliable)
12281228
{
12291229
if (packetdata.getSize() <= ORIGINAL_HEADER_SIZE)
12301230
throw InvalidIncomingDataException
@@ -1238,7 +1238,7 @@ SharedBuffer<u8> ConnectionReceiveThread::handlePacketType_Original(Channel *cha
12381238
}
12391239

12401240
SharedBuffer<u8> ConnectionReceiveThread::handlePacketType_Split(Channel *channel,
1241-
SharedBuffer<u8> packetdata, Peer *peer, u8 channelnum, bool reliable)
1241+
const SharedBuffer<u8> &packetdata, Peer *peer, u8 channelnum, bool reliable)
12421242
{
12431243
Address peer_address;
12441244

@@ -1269,7 +1269,7 @@ SharedBuffer<u8> ConnectionReceiveThread::handlePacketType_Split(Channel *channe
12691269
}
12701270

12711271
SharedBuffer<u8> ConnectionReceiveThread::handlePacketType_Reliable(Channel *channel,
1272-
SharedBuffer<u8> packetdata, Peer *peer, u8 channelnum, bool reliable)
1272+
const SharedBuffer<u8> &packetdata, Peer *peer, u8 channelnum, bool reliable)
12731273
{
12741274
assert(channel != NULL);
12751275

Diff for: ‎src/network/connectionthreads.h

+13-12
Original file line numberDiff line numberDiff line change
@@ -52,23 +52,23 @@ class ConnectionSendThread : public Thread
5252
private:
5353
void runTimeouts(float dtime);
5454
void rawSend(const BufferedPacket &packet);
55-
bool rawSendAsPacket(session_t peer_id, u8 channelnum, SharedBuffer<u8> data,
56-
bool reliable);
55+
bool rawSendAsPacket(session_t peer_id, u8 channelnum,
56+
const SharedBuffer<u8> &data, bool reliable);
5757

5858
void processReliableCommand(ConnectionCommand &c);
5959
void processNonReliableCommand(ConnectionCommand &c);
6060
void serve(Address bind_address);
6161
void connect(Address address);
6262
void disconnect();
6363
void disconnect_peer(session_t peer_id);
64-
void send(session_t peer_id, u8 channelnum, SharedBuffer<u8> data);
64+
void send(session_t peer_id, u8 channelnum, const SharedBuffer<u8> &data);
6565
void sendReliable(ConnectionCommand &c);
66-
void sendToAll(u8 channelnum, SharedBuffer<u8> data);
66+
void sendToAll(u8 channelnum, const SharedBuffer<u8> &data);
6767
void sendToAllReliable(ConnectionCommand &c);
6868

6969
void sendPackets(float dtime);
7070

71-
void sendAsPacket(session_t peer_id, u8 channelnum, SharedBuffer<u8> data,
71+
void sendAsPacket(session_t peer_id, u8 channelnum, const SharedBuffer<u8> &data,
7272
bool ack = false);
7373

7474
void sendAsPacketReliable(BufferedPacket &p, Channel *channel);
@@ -119,26 +119,27 @@ class ConnectionReceiveThread : public Thread
119119
channelnum: channel on which the packet was sent
120120
reliable: true if recursing into a reliable packet
121121
*/
122-
SharedBuffer<u8> processPacket(Channel *channel, SharedBuffer<u8> packetdata,
123-
session_t peer_id, u8 channelnum, bool reliable);
122+
SharedBuffer<u8> processPacket(Channel *channel,
123+
const SharedBuffer<u8> &packetdata, session_t peer_id,
124+
u8 channelnum, bool reliable);
124125

125126
SharedBuffer<u8> handlePacketType_Control(Channel *channel,
126-
SharedBuffer<u8> packetdata, Peer *peer, u8 channelnum,
127+
const SharedBuffer<u8> &packetdata, Peer *peer, u8 channelnum,
127128
bool reliable);
128129
SharedBuffer<u8> handlePacketType_Original(Channel *channel,
129-
SharedBuffer<u8> packetdata, Peer *peer, u8 channelnum,
130+
const SharedBuffer<u8> &packetdata, Peer *peer, u8 channelnum,
130131
bool reliable);
131132
SharedBuffer<u8> handlePacketType_Split(Channel *channel,
132-
SharedBuffer<u8> packetdata, Peer *peer, u8 channelnum,
133+
const SharedBuffer<u8> &packetdata, Peer *peer, u8 channelnum,
133134
bool reliable);
134135
SharedBuffer<u8> handlePacketType_Reliable(Channel *channel,
135-
SharedBuffer<u8> packetdata, Peer *peer, u8 channelnum,
136+
const SharedBuffer<u8> &packetdata, Peer *peer, u8 channelnum,
136137
bool reliable);
137138

138139
struct PacketTypeHandler
139140
{
140141
SharedBuffer<u8> (ConnectionReceiveThread::*handler)(Channel *channel,
141-
SharedBuffer<u8> packet, Peer *peer, u8 channelnum,
142+
const SharedBuffer<u8> &packet, Peer *peer, u8 channelnum,
142143
bool reliable);
143144
};
144145

0 commit comments

Comments
 (0)
Please sign in to comment.