From 27d02ca0552888ffa8e77341fa48b4774d5ccb50 Mon Sep 17 00:00:00 2001 From: Fred Nicolson Date: Tue, 27 Mar 2018 12:03:55 +0100 Subject: [PATCH] fr::Packet optimisations & marking Sendable::send() as const Replaced instances of resize&memcpy with append, which gives a noticeable performance boost. fr::Packet::operator<<(const char *str) no longer converts str into an std::string before adding it, removing an unneeded copy. fr::Packet::clear no longer calls erase, should result in more of the internal buffer remaining allocated. Framing for std::vector's has been changed from a uint64_t to a uint32_t (breaking change for packet framing!) --- CMakeLists.txt | 2 +- include/frnetlib/Http.h | 2 +- include/frnetlib/Packet.h | 84 ++++++++++++++----------------------- include/frnetlib/Sendable.h | 2 +- include/frnetlib/Socket.h | 3 +- include/frnetlib/WebFrame.h | 4 +- main.cpp | 15 +++---- src/Http.cpp | 2 +- src/Socket.cpp | 10 +---- src/WebFrame.cpp | 2 +- tests/PacketTest.cpp | 22 ++++++++-- 11 files changed, 67 insertions(+), 81 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 755f4ce..9a2d8f4 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -50,7 +50,7 @@ if(USE_SSL) endif() if(NOT MSVC) - set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -std=c++14 -pthread") + set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -pthread") endif() # Set the library output directory diff --git a/include/frnetlib/Http.h b/include/frnetlib/Http.h index 9909017..0a518b7 100644 --- a/include/frnetlib/Http.h +++ b/include/frnetlib/Http.h @@ -298,7 +298,7 @@ namespace fr * @param socket The socket to send through * @return Status indicating if the send succeeded or not. */ - virtual Socket::Status send(Socket *socket) override; + virtual Socket::Status send(Socket *socket) const override; /*! * Overrideable receive, to allow diff --git a/include/frnetlib/Packet.h b/include/frnetlib/Packet.h index c7f8249..693ba42 100644 --- a/include/frnetlib/Packet.h +++ b/include/frnetlib/Packet.h @@ -20,8 +20,8 @@ namespace fr { public: Packet() noexcept - : buffer(PACKET_HEADER_LENGTH, '0'), - buffer_read_index(PACKET_HEADER_LENGTH) + : buffer(PACKET_HEADER_LENGTH, '0'), + buffer_read_index(PACKET_HEADER_LENGTH) { } @@ -73,7 +73,7 @@ namespace fr template inline void add_range(Iter begin, Iter end) { - *this << static_cast(std::distance(begin, end)); + *this << static_cast(std::distance(begin, end)); for(auto iter = begin; iter != end; ++iter) *this << *iter; } @@ -86,8 +86,7 @@ namespace fr */ inline void add_raw(const char *data, size_t datasz) { - buffer.resize(buffer.size() + datasz); - memcpy(&buffer[buffer.size() - datasz], data, datasz); + buffer.append(data, datasz); } /*! @@ -111,7 +110,7 @@ namespace fr inline Packet &operator<<(const std::vector &vec) { //First store its length - *this << static_cast(vec.size()); + *this << static_cast(vec.size()); //Now each of the elements for(const auto &iter : vec) @@ -128,14 +127,14 @@ namespace fr template inline Packet &operator>>(std::vector &vec) { - uint64_t length; + uint32_t length; //First extract the length *this >> length; vec.resize(length); //Now take each of the elements out of the packet - for(size_t a = 0; a < length; a++) + for(uint32_t a = 0; a < length; a++) { *this >> vec[a]; } @@ -171,8 +170,7 @@ namespace fr */ inline Packet &operator<<(bool var) { - buffer.resize(buffer.size() + sizeof(var)); - memcpy(&buffer[buffer.size() - sizeof(var)], &var, sizeof(var)); + buffer.append((char*)&var, sizeof(var)); return *this; } @@ -193,8 +191,7 @@ namespace fr */ inline Packet &operator<<(uint8_t var) { - buffer.resize(buffer.size() + sizeof(var)); - memcpy(&buffer[buffer.size() - sizeof(var)], &var, sizeof(var)); + buffer.append((char*)&var, sizeof(var)); return *this; } @@ -215,9 +212,8 @@ namespace fr */ inline Packet &operator<<(uint16_t var) { - buffer.resize(buffer.size() + sizeof(var)); var = htons(var); - memcpy(&buffer[buffer.size() - sizeof(var)], &var, sizeof(var)); + buffer.append((char*)&var, sizeof(var)); return *this; } @@ -240,8 +236,7 @@ namespace fr inline Packet &operator<<(uint32_t var) { var = htonl(var); - buffer.resize(buffer.size() + sizeof(var)); - memcpy(&buffer[buffer.size() - sizeof(var)], &var, sizeof(var)); + buffer.append((char*)&var, sizeof(var)); return *this; } @@ -251,7 +246,6 @@ namespace fr inline Packet &operator>>(uint32_t &var) { assert_data_remaining(sizeof(var)); - memcpy(&var, &buffer[buffer_read_index], sizeof(var)); buffer_read_index += sizeof(var); var = ntohl(var); @@ -263,9 +257,8 @@ namespace fr */ inline Packet &operator<<(uint64_t var) { - buffer.resize(buffer.size() + sizeof(var)); var = htonll(var); - memcpy(&buffer[buffer.size() - sizeof(var)], &var, sizeof(var)); + buffer.append((char*)&var, sizeof(var)); return *this; } @@ -287,9 +280,8 @@ namespace fr */ inline Packet &operator<<(int16_t var) { - buffer.resize(buffer.size() + sizeof(var)); var = htons((uint16_t)var); - memcpy(&buffer[buffer.size() - sizeof(var)], &var, sizeof(var)); + buffer.append((char*)&var, sizeof(var)); return *this; } @@ -311,9 +303,8 @@ namespace fr */ inline Packet &operator<<(int32_t var) { - buffer.resize(buffer.size() + sizeof(var)); var = htonl((uint32_t)var); - memcpy(&buffer[buffer.size() - sizeof(var)], &var, sizeof(var)); + buffer.append((char*)&var, sizeof(var)); return *this; } @@ -335,9 +326,8 @@ namespace fr */ inline Packet &operator<<(int64_t var) { - buffer.resize(buffer.size() + sizeof(var)); var = htonll((uint64_t)var); - memcpy(&buffer[buffer.size() - sizeof(var)], &var, sizeof(var)); + buffer.append((char*)&var, sizeof(var)); return *this; } @@ -392,9 +382,8 @@ namespace fr */ inline Packet &operator<<(float var) { - buffer.resize(buffer.size() + sizeof(var)); var = htonf(var); - memcpy(&buffer[buffer.size() - sizeof(var)], &var, sizeof(var)); + buffer.append((char*)&var, sizeof(var)); return *this; } @@ -416,9 +405,8 @@ namespace fr */ inline Packet &operator<<(double var) { - buffer.resize(buffer.size() + sizeof(var)); var = htond(var); - memcpy(&buffer[buffer.size() - sizeof(var)], &var, sizeof(var)); + buffer.append((char*)&var, sizeof(var)); return *this; } @@ -442,7 +430,7 @@ namespace fr { //Strings are prefixed with their length as a 32bit uint :) *this << (uint32_t)var.length(); - buffer += var; + buffer.append(var); return *this; } @@ -451,7 +439,9 @@ namespace fr */ inline Packet &operator<<(const char *var) { - *this << std::string(var); + auto len = (uint32_t)strlen(var); + *this << len; + buffer.append(var, len); return *this; } @@ -511,7 +501,10 @@ namespace fr */ inline void clear() { - buffer.erase(PACKET_HEADER_LENGTH, buffer.size() - PACKET_HEADER_LENGTH); + //Leave enough for the header + buffer.clear(); + for(auto a = 0; a < PACKET_HEADER_LENGTH; ++a) + buffer.push_back('0'); buffer_read_index = PACKET_HEADER_LENGTH; } @@ -548,10 +541,11 @@ namespace fr * @param socket The socket to send through * @return Status indicating if the send succeeded or not. */ - virtual Socket::Status send(Socket *socket) override + virtual Socket::Status send(Socket *socket) const override { - const std::string &data = get_buffer(); - return socket->send_raw(data.c_str(), data.size()); + uint32_t length = htonl((uint32_t)buffer.size() - PACKET_HEADER_LENGTH); + memcpy(&buffer[0], &length, sizeof(uint32_t)); + return socket->send_raw(buffer.c_str(), buffer.size()); } /*! @@ -578,7 +572,8 @@ namespace fr return fr::Socket::MaxPacketSizeExceeded; //Now we've got the length, read the rest of the data in - buffer.resize(packet_length + PACKET_HEADER_LENGTH); + if(packet_length + PACKET_HEADER_LENGTH > buffer.size()) + buffer.resize(packet_length + PACKET_HEADER_LENGTH); status = socket->receive_all(&buffer[PACKET_HEADER_LENGTH], packet_length); if(status != Socket::Status::Success) return status; @@ -586,21 +581,6 @@ namespace fr return Socket::Status::Success; } - /*! - * Gets the data added to the packet - * - * @return A string containing all of the data added to the packet - */ - inline std::string &get_buffer() - { - //Update packet length first - uint32_t length = htonl((uint32_t)buffer.size() - PACKET_HEADER_LENGTH); - memcpy(&buffer[0], &length, sizeof(uint32_t)); - - //Then return a reference to the buffer - return buffer; - } - /*! * Checks that there's enough data in the buffer to extract * a given number of bytes to prevent buffer overflows. @@ -615,7 +595,7 @@ namespace fr } - std::string buffer; //Packet data buffer + mutable std::string buffer; //Packet data buffer size_t buffer_read_index; //Current read position }; } diff --git a/include/frnetlib/Sendable.h b/include/frnetlib/Sendable.h index e0e4c62..85e9539 100644 --- a/include/frnetlib/Sendable.h +++ b/include/frnetlib/Sendable.h @@ -19,7 +19,7 @@ namespace fr * @param socket The socket to send through * @return Status indicating if the send succeeded or not. */ - virtual Socket::Status send(Socket *socket) = 0; + virtual Socket::Status send(Socket *socket) const = 0; /*! * Overrideable receive, to allow diff --git a/include/frnetlib/Socket.h b/include/frnetlib/Socket.h index f68921f..c1cb7cd 100644 --- a/include/frnetlib/Socket.h +++ b/include/frnetlib/Socket.h @@ -121,8 +121,7 @@ namespace fr * @param obj The object to send * @return The status of the send */ - virtual Status send(Sendable &obj); - virtual Status send(Sendable &&obj); + virtual Status send(const Sendable &obj); /*! * Receive a Sendable object through the socket diff --git a/include/frnetlib/WebFrame.h b/include/frnetlib/WebFrame.h index 103d4e1..7e5782b 100644 --- a/include/frnetlib/WebFrame.h +++ b/include/frnetlib/WebFrame.h @@ -102,7 +102,7 @@ namespace fr * @param socket The socket to send through * @return Status indicating if the send succeeded or not. */ - Socket::Status send(Socket *socket) override; + Socket::Status send(Socket *socket) const override; /*! * Overrideable receive, to allow @@ -116,7 +116,7 @@ namespace fr Socket::Status receive(Socket *socket) override; private: - std::string payload; + mutable std::string payload; Opcode opcode; bool final; static uint32_t current_mask_key; diff --git a/main.cpp b/main.cpp index 3286dc3..24a9adf 100644 --- a/main.cpp +++ b/main.cpp @@ -15,13 +15,14 @@ enum Enum : uint32_t E2 = 1, E3 = 2, }; + +size_t get_vector_size(const std::vector &vec) +{ + return vec.size(); +} + int main() { - fr::Packet packet; - std::vector> bob = {{1, 2}, {3, 4}}; - packet << bob; - bob.clear(); - - packet >> bob; - std::cout << bob[0].first << ", " << bob[0].second << ", " << bob[1].first << ", " << bob[1].second << std::endl; + std::vector source{1, 2, 3}; + std::cout << get_vector_size(source) << std::endl; } \ No newline at end of file diff --git a/src/Http.cpp b/src/Http.cpp index 1fdac96..756e2ba 100644 --- a/src/Http.cpp +++ b/src/Http.cpp @@ -990,7 +990,7 @@ namespace fr return iter->second; } - Socket::Status Http::send(Socket *socket) + Socket::Status Http::send(Socket *socket) const { std::string data = construct(socket->get_remote_address()); return socket->send_raw(&data[0], data.size()); diff --git a/src/Socket.cpp b/src/Socket.cpp index 238ceca..bf16c76 100644 --- a/src/Socket.cpp +++ b/src/Socket.cpp @@ -20,15 +20,7 @@ namespace fr init_wsa(); } - Socket::Status Socket::send(Sendable &obj) - { - if(!connected()) - return Socket::Disconnected; - - return obj.send(this); - } - - Socket::Status Socket::send(Sendable &&obj) + Socket::Status Socket::send(const Sendable &obj) { if(!connected()) return Socket::Disconnected; diff --git a/src/WebFrame.cpp b/src/WebFrame.cpp index ea44aa3..e6ac2c2 100644 --- a/src/WebFrame.cpp +++ b/src/WebFrame.cpp @@ -16,7 +16,7 @@ namespace fr } - fr::Socket::Status WebFrame::send(Socket *socket_) + fr::Socket::Status WebFrame::send(Socket *socket_) const { auto *socket = dynamic_cast(socket_); if(!socket) diff --git a/tests/PacketTest.cpp b/tests/PacketTest.cpp index c0499ab..ddeaab6 100644 --- a/tests/PacketTest.cpp +++ b/tests/PacketTest.cpp @@ -34,10 +34,10 @@ TEST(PacketTest, pack_and_unpack_ints) fr::Packet packet; double a1 = 11.5f, a2 = 0.f; float b1 = 11.52, b2 = 0.0; - uint8_t c1 = std::numeric_limits::max(), c2 = 0; - uint16_t d1 = std::numeric_limits::max(), d2 = 0; - uint32_t e1 = std::numeric_limits::max(), e2 = 0; - uint64_t f1 = std::numeric_limits::max(), f2 = 0; + uint8_t c1 = std::numeric_limits::max() - 50, c2 = 0; + uint16_t d1 = std::numeric_limits::max() - 50, d2 = 0; + uint32_t e1 = std::numeric_limits::max() - 50, e2 = 0; + uint64_t f1 = std::numeric_limits::max() - 50, f2 = 0; packet << a1 << b1 << c1 << d1 << e1 << f1; packet >> a2 >> b2 >> c2 >> d2 >> e2 >> f2; @@ -135,4 +135,18 @@ TEST(PacketTest, read_cursor) packet.reset_read_cursor((sizeof(a1))); packet >> b2; ASSERT_EQ(b1, b2); +} + +TEST(PacketTest, clear) +{ + uint32_t a = 20, b; + fr::Packet packet; + packet << a << a << a; + packet.clear(); + ASSERT_ANY_THROW(packet >> a); + + a = 20; + packet << a; + packet >> b; + ASSERT_EQ(b, 20); } \ No newline at end of file