Allocation Optimizations

include/cpp-statsd-client/StatsdClient.hpp

@@ -90,6 +90,9 @@ class StatsdClient {
 
     //! The random number generator for handling sampling
     mutable std::mt19937 m_randomEngine;
+
+    //! The buffer string format our stats before sending them
+    mutable std::string m_buffer;
 };
 
 namespace detail {
@@ -109,6 +112,8 @@ inline StatsdClient::StatsdClient(const std::string& host,
     : m_prefix(detail::sanitizePrefix(prefix)), m_sender(new UDPSender{host, port, batchsize}) {
     // Initialize the random generator to be used for sampling
     seed();
+    // Avoid re-allocations by reserving a generous buffer
+    m_buffer.reserve(256);
 }
 
 inline void StatsdClient::setConfig(const std::string& host,
@@ -165,39 +170,27 @@ inline void StatsdClient::send(const std::string& key,
         return;
     }
 
-    // Prepare the buffer and include the sampling rate if it's not 1.f
-    const bool needs_dot = !m_prefix.empty() && !key.empty();
-    std::string buffer(256, '\0');
-    int string_len = -1;
-    if (isFrequencyOne) {
-        // Sampling rate is 1.0f, no need to specify it
-        string_len = std::snprintf(&buffer.front(),
-                                   buffer.size(),
-                                   "%s%s%s:%d|%s",
-                                   m_prefix.c_str(),
-                                   needs_dot ? "." : "",
-                                   key.c_str(),
-                                   value,
-                                   type.c_str());
-    } else {
-        // Sampling rate is different from 1.0f, hence specify it
-        string_len = std::snprintf(&buffer.front(),
-                                   buffer.size(),
-                                   "%s%s%s:%d|%s|@%.2f",
-                                   m_prefix.c_str(),
-                                   needs_dot ? "." : "",
-                                   key.c_str(),
-                                   value,
-                                   type.c_str(),
-                                   frequency);
+    // Format the stat message
+    m_buffer.clear();
+
+    m_buffer.append(m_prefix);
+    if (!m_prefix.empty() && !key.empty()) {
+        m_buffer.push_back('.');
     }
 
-    // Trim the trailing null chars
-    // TODO: perf improvement: send the len along and keep the buffer around as a member variable
-    buffer.resize(std::min(string_len, static_cast<int>(buffer.size())));
+    m_buffer.append(key);
+    m_buffer.push_back(':');
+    m_buffer.append(std::to_string(value));
+    m_buffer.push_back('|');
+    m_buffer.append(type);
+
+    if (frequency < 1.f) {
+        m_buffer.append("|@0.");
+        m_buffer.append(std::to_string(static_cast<int>(frequency * 100)));
+    }
 
     // Send the message via the UDP sender
-    m_sender->send(buffer);
+    m_sender->send(m_buffer);
 }
 
 inline void StatsdClient::seed(unsigned int seed) noexcept {

include/cpp-statsd-client/UDPSender.hpp

@@ -41,7 +41,7 @@ class UDPSender final {
     //! Send a message
     void send(const std::string& message) noexcept;
 
-    //! Returns the error message as an string
+    //! Returns the error message as a string
     const std::string& errorMessage() const noexcept;
 
     //! Returns true if the sender is initialized
@@ -56,6 +56,9 @@ class UDPSender final {
     //! Initialize the sender and returns true when it is initialized
     bool initialize() noexcept;
 
+    //! Queue a message to be sent to the daemon later
+    inline void queueMessage(const std::string& message) noexcept;
+
     //! Send a message to the daemon
     void sendToDaemon(const std::string& message) noexcept;
 
@@ -170,15 +173,25 @@ inline void UDPSender::send(const std::string& message) noexcept {
     m_errorMessage.clear();
     // If batching is on, accumulate messages in the queue
     if (m_batching) {
-        std::unique_lock<std::mutex> batchingLock(m_batchingMutex);
-        if (m_batchingMessageQueue.empty() || m_batchingMessageQueue.back().length() > m_batchsize) {
-            m_batchingMessageQueue.push_back(message);
-        } else {
-            m_batchingMessageQueue.rbegin()->append("\n").append(message);
-        }
-    } else {
-        sendToDaemon(message);
+        queueMessage(message);
+        return;
+    }
+
+    sendToDaemon(message);
+}
+
+inline void UDPSender::queueMessage(const std::string& message) noexcept {
+    std::unique_lock<std::mutex> batchingLock(m_batchingMutex);
+    // Either we don't have a place to batch our message or we exceeded the batch size, so make a new batch
+    if (m_batchingMessageQueue.empty() || m_batchingMessageQueue.back().length() > m_batchsize) {
+        m_batchingMessageQueue.emplace_back();
+        m_batchingMessageQueue.back().reserve(m_batchsize + 256);
+    }  // When there is already a batch open we need a separator when its not empty
+    else if (!m_batchingMessageQueue.back().empty()) {
+        m_batchingMessageQueue.back().push_back('\n');
     }
+    // Add the new message to the batch
+    m_batchingMessageQueue.back().append(message);
 }
 
 inline const std::string& UDPSender::errorMessage() const noexcept {
@@ -189,7 +202,7 @@ inline bool UDPSender::initialize() noexcept {
     // Connect the socket
     m_socket = socket(AF_INET, SOCK_DGRAM, IPPROTO_UDP);
     if (m_socket == k_invalidFd) {
-        m_errorMessage = std::string("Could not create socket, err=") + std::strerror(errno);
+        m_errorMessage = std::string("socket creation failed: err=") + std::strerror(errno);
         return false;
     }
 
@@ -213,7 +226,7 @@ inline bool UDPSender::initialize() noexcept {
             // An error code has been returned by getaddrinfo
             close(m_socket);
             m_socket = k_invalidFd;
-            m_errorMessage = "getaddrinfo failed: error=" + std::to_string(ret) + ", msg=" + gai_strerror(ret);
+            m_errorMessage = "getaddrinfo failed: err=" + std::to_string(ret) + ", msg=" + gai_strerror(ret);
             return false;
         }
 

tests/testStatsdClient.cpp

@@ -31,21 +31,17 @@ void mock(StatsdServer& server, std::vector<std::string>& messages) {
     } while (server.errorMessage().empty() && !messages.back().empty());
 }
 
-// TODO: would it be ok if we change the API of the client to throw on error instead of having to check a string?
-void throwOnError(const StatsdClient& client) {
-    if (!client.errorMessage().empty()) {
-        std::cerr << client.errorMessage() << std::endl;
-        throw std::runtime_error(client.errorMessage());
+void throwOnError(const StatsdClient& client, bool expectEmpty = true, const std::string& extraMessage = "") {
+    if (client.errorMessage().empty() != expectEmpty) {
+        std::cerr << (expectEmpty ? client.errorMessage() : extraMessage) << std::endl;
+        throw std::runtime_error(expectEmpty ? client.errorMessage() : extraMessage);
     }
 }
 
 void testErrorConditions() {
-    // Connect to a rubbish ip and make sure initialization failed
-    StatsdClient client{"256.256.256.256", 8125, "myPrefix.", 20};
-    if (client.errorMessage().empty()) {
-        std::cerr << "Should not be able to connect to ridiculous ip" << std::endl;
-        throw std::runtime_error("Should not be able to connect to ridiculous ip");
-    }
+    // Resolve a rubbish ip and make sure initialization failed
+    StatsdClient client{"256.256.256.256", 8125, "myPrefix", 20};
+    throwOnError(client, false, "Should not be able to resolve a ridiculous ip");
 }
 
 void testReconfigure() {