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

@@ -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

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

include/frnetlib/Packet.h

@@ -20,8 +20,8 @@ namespace fr
     {
     public:
         Packet() noexcept
-        : buffer(PACKET_HEADER_LENGTH, '0'),
+                : buffer(PACKET_HEADER_LENGTH, '0'),
-          buffer_read_index(PACKET_HEADER_LENGTH)
+                  buffer_read_index(PACKET_HEADER_LENGTH)
         {
 
         }
@@ -73,7 +73,7 @@ namespace fr
         template<typename Iter>
         inline void add_range(Iter begin, Iter end)
         {
-            *this << static_cast<uint64_t>(std::distance(begin, end));
+            *this << static_cast<uint32_t>(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);
+            buffer.append(data, datasz);
-            memcpy(&buffer[buffer.size() - datasz], data, datasz);
         }
 
         /*!
@@ -111,7 +110,7 @@ namespace fr
         inline Packet &operator<<(const std::vector<T> &vec)
         {
             //First store its length
-            *this << static_cast<uint64_t>(vec.size());
+            *this << static_cast<uint32_t>(vec.size());
 
             //Now each of the elements
             for(const auto &iter : vec)
@@ -128,14 +127,14 @@ namespace fr
         template<typename T>
         inline Packet &operator>>(std::vector<T> &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));
+            buffer.append((char*)&var, sizeof(var));
-            memcpy(&buffer[buffer.size() - sizeof(var)], &var, sizeof(var));
             return *this;
         }
 
@@ -193,8 +191,7 @@ namespace fr
          */
         inline Packet &operator<<(uint8_t var)
         {
-            buffer.resize(buffer.size() + sizeof(var));
+            buffer.append((char*)&var, sizeof(var));
-            memcpy(&buffer[buffer.size() - sizeof(var)], &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));
+            buffer.append((char*)&var, sizeof(var));
-            memcpy(&buffer[buffer.size() - sizeof(var)], &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();
+            uint32_t length = htonl((uint32_t)buffer.size() - PACKET_HEADER_LENGTH);
-            return socket->send_raw(data.c_str(), data.size());
+            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
     };
 }

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

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(const Sendable &obj);
-        virtual Status send(Sendable &&obj);
 
         /*!
          * Receive a Sendable object through the socket

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;

main.cpp

@@ -15,13 +15,14 @@ enum Enum : uint32_t
     E2 = 1,
     E3 = 2,
 };
+
+size_t get_vector_size(const std::vector<int> &vec)
+{
+    return vec.size();
+}
+
 int main()
 {
-    fr::Packet packet;
+    std::vector<int> source{1, 2, 3};
-    std::vector<std::pair<int, int>> bob = {{1, 2}, {3, 4}};
+    std::cout << get_vector_size(source) << std::endl;
-    packet << bob;
-    bob.clear();
-
-    packet >> bob;
-    std::cout << bob[0].first << ", " << bob[0].second << ", " << bob[1].first << ", " << bob[1].second << std::endl;
 }

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());

src/Socket.cpp

@@ -20,15 +20,7 @@ namespace fr
         init_wsa();
     }
 
-    Socket::Status Socket::send(Sendable &obj)
+    Socket::Status Socket::send(const Sendable &obj)
-    {
-        if(!connected())
-            return Socket::Disconnected;
-
-        return obj.send(this);
-    }
-
-    Socket::Status Socket::send(Sendable &&obj)
     {
         if(!connected())
             return Socket::Disconnected;

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<WebSocketBase*>(socket_);
         if(!socket)

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<uint8_t>::max(), c2 = 0;
+    uint8_t c1 = std::numeric_limits<uint8_t>::max() - 50, c2 = 0;
-    uint16_t d1 = std::numeric_limits<uint16_t>::max(), d2 = 0;
+    uint16_t d1 = std::numeric_limits<uint16_t>::max() - 50, d2 = 0;
-    uint32_t e1 = std::numeric_limits<uint32_t>::max(), e2 = 0;
+    uint32_t e1 = std::numeric_limits<uint32_t>::max() - 50, e2 = 0;
-    uint64_t f1 = std::numeric_limits<uint64_t>::max(), f2 = 0;
+    uint64_t f1 = std::numeric_limits<uint64_t>::max() - 50, f2 = 0;
 
     packet << a1 << b1 << c1 << d1 << e1 << f1;
     packet >> a2 >> b2 >> c2 >> d2 >> e2 >> f2;
@@ -136,3 +136,17 @@ TEST(PacketTest, read_cursor)
     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);
+}