Avoid unnecessary copies by recv()ing data directly into the expat buffer

8 files changed, 93 insertions, 18 deletions

diff --git a/src/irc/irc_client.cpp b/src/irc/irc_client.cpp
index 232b87f..0c36372 100644
--- a/src/irc/irc_client.cpp
+++ b/src/irc/irc_client.cpp
@@ -80,7 +80,7 @@ std::string IrcClient::get_own_nick() const
   return this->current_nick;
 }
 
-void IrcClient::parse_in_buffer()
+void IrcClient::parse_in_buffer(const size_t)
 {
   while (true)
     {
diff --git a/src/irc/irc_client.hpp b/src/irc/irc_client.hpp
index 28a1424..908db8e 100644
--- a/src/irc/irc_client.hpp
+++ b/src/irc/irc_client.hpp
@@ -45,7 +45,7 @@ public:
    * Parse the data we have received so far and try to get one or more
    * complete messages from it.
    */
-  void parse_in_buffer() override final;
+  void parse_in_buffer(const size_t) override final;
   /**
    * Return the channel with this name, create it if it does not yet exist
    */
diff --git a/src/network/socket_handler.cpp b/src/network/socket_handler.cpp
index 6f7cc3f..eb427b8 100644
--- a/src/network/socket_handler.cpp
+++ b/src/network/socket_handler.cpp
@@ -138,11 +138,16 @@ void SocketHandler::set_poller(Poller* poller)
   this->poller = poller;
 }
 
-void SocketHandler::on_recv(const size_t nb)
+void SocketHandler::on_recv()
 {
-  char buf[4096];
+  static constexpr size_t buf_size = 4096;
+  char buf[buf_size];
+  void* recv_buf = this->get_receive_buffer(buf_size);
 
-  ssize_t size = ::recv(this->socket, buf, nb, 0);
+  if (recv_buf == nullptr)
+    recv_buf = buf;
+
+  ssize_t size = ::recv(this->socket, recv_buf, buf_size, 0);
   if (0 == size)
     {
       this->on_connection_close();
@@ -156,8 +161,14 @@ void SocketHandler::on_recv(const size_t nb)
     }
   else
     {
-      this->in_buf += std::string(buf, size);
-      this->parse_in_buffer();
+      if (buf == recv_buf)
+        {
+          // data needs to be placed in the in_buf string, because no buffer
+          // was provided to receive that data directly. The in_buf buffer
+          // will be handled in parse_in_buffer()
+          this->in_buf += std::string(buf, size);
+        }
+      this->parse_in_buffer(size);
     }
 }
 
@@ -238,3 +249,8 @@ bool SocketHandler::is_connecting() const
 {
   return this->connecting;
 }
+
+void* SocketHandler::get_receive_buffer(const size_t) const
+{
+  return nullptr;
+}
diff --git a/src/network/socket_handler.hpp b/src/network/socket_handler.hpp
index 8828596..35b0fdc 100644
--- a/src/network/socket_handler.hpp
+++ b/src/network/socket_handler.hpp
@@ -41,7 +41,7 @@ public:
    * Reads data in our in_buf and the call parse_in_buf, for the implementor
    * to handle the data received so far.
    */
-  void on_recv(const size_t nb = 4096);
+  void on_recv();
   /**
    * Write as much data from out_buf as possible, in the socket.
    */
@@ -73,14 +73,28 @@ public:
    */
   virtual void on_connection_close() = 0;
   /**
-   * Handle/consume (some of) the data received so far. If some data is used, the in_buf
+   * Handle/consume (some of) the data received so far.  The data to handle
+   * may be in the in_buf buffer, or somewhere else, depending on what
+   * get_receive_buffer() returned.  If some data is used from in_buf, it
    * should be truncated, only the unused data should be left untouched.
+   *
+   * The size argument is the size of the last chunk of data that was added to the buffer.
    */
-  virtual void parse_in_buffer() = 0;
+  virtual void parse_in_buffer(const size_t size) = 0;
   bool is_connected() const;
   bool is_connecting() const;
 
 protected:
+  /**
+   * Provide a buffer in which data can be directly received. This can be
+   * used to avoid copying data into in_buf before using it. If no buffer
+   * can provided, nullptr is returned (the default implementation does
+   * that).
+   */
+  virtual void* get_receive_buffer(const size_t size) const;
+  /**
+   * The handled socket.
+   */
   socket_t socket;
   /**
    * Where data read from the socket is added until we can extract a full
diff --git a/src/xmpp/xmpp_component.cpp b/src/xmpp/xmpp_component.cpp
index 9b73626..cd424ed 100644
--- a/src/xmpp/xmpp_component.cpp
+++ b/src/xmpp/xmpp_component.cpp
@@ -89,10 +89,20 @@ void XmppComponent::on_connection_close()
   log_info("XMPP server closed connection");
 }
 
-void XmppComponent::parse_in_buffer()
+void XmppComponent::parse_in_buffer(const size_t size)
 {
-  this->parser.feed(this->in_buf.data(), this->in_buf.size(), false);
-  this->in_buf.clear();
+  if (!this->in_buf.empty())
+    { // This may happen if the parser could not allocate enough space for
+      // us. We try to feed it the data that was read into our in_buf
+      // instead. If this fails again we are in trouble.
+      this->parser.feed(this->in_buf.data(), this->in_buf.size(), false);
+      this->in_buf.clear();
+    }
+  else
+    { // Just tell the parser to parse the data that was placed into the
+      // buffer it provided to us with GetBuffer
+      this->parser.parse(size, false);
+    }
 }
 
 void XmppComponent::shutdown()
@@ -308,6 +318,11 @@ Bridge* XmppComponent::get_user_bridge(const std::string& user_jid)
     }
 }
 
+void* XmppComponent::get_receive_buffer(const size_t size) const
+{
+  return this->parser.get_buffer(size);
+}
+
 void XmppComponent::send_message(const std::string& from, Xmpp::body&& body, const std::string& to)
 {
   XmlNode node("message");
diff --git a/src/xmpp/xmpp_component.hpp b/src/xmpp/xmpp_component.hpp
index bc2b518..0c040f4 100644
--- a/src/xmpp/xmpp_component.hpp
+++ b/src/xmpp/xmpp_component.hpp
@@ -23,7 +23,7 @@ public:
   void on_connection_failed(const std::string& reason) override final;
   void on_connected() override final;
   void on_connection_close() override final;
-  void parse_in_buffer() override final;
+  void parse_in_buffer(const size_t size) override final;
   /**
    * Send a "close" message to all our connected peers.  That message
    * depends on the protocol used (this may be a QUIT irc message, or a
@@ -142,7 +142,11 @@ private:
    * if none already exist.
    */
   Bridge* get_user_bridge(const std::string& user_jid);
-
+  /**
+   * Return a buffer provided by the XML parser, to read data directly into
+   * it, and avoiding some unnecessary copy.
+   */
+  void* get_receive_buffer(const size_t size) const override final;
   XmppParser parser;
   std::string stream_id;
   std::string served_hostname;
diff --git a/src/xmpp/xmpp_parser.cpp b/src/xmpp/xmpp_parser.cpp
index 6e4809d..9fcc16c 100644
--- a/src/xmpp/xmpp_parser.cpp
+++ b/src/xmpp/xmpp_parser.cpp
@@ -1,6 +1,8 @@
 #include <xmpp/xmpp_parser.hpp>
 #include <xmpp/xmpp_stanza.hpp>
 
+#include <logger/logger.hpp>
+
 /**
  * Expat handlers. Called by the Expat library, never by ourself.
  * They just forward the call to the XmppParser corresponding methods.
@@ -45,9 +47,25 @@ XmppParser::~XmppParser()
   XML_ParserFree(this->parser);
 }
 
-void XmppParser::feed(const char* data, const int len, const bool is_final)
+int XmppParser::feed(const char* data, const int len, const bool is_final)
+{
+  int res = XML_Parse(this->parser, data, len, is_final);
+  if (res == 0)
+    log_error("Xml_Parse encountered an error");
+  return res;
+}
+
+int XmppParser::parse(const int len, const bool is_final)
+{
+  int res = XML_ParseBuffer(this->parser, len, is_final);
+  if (res == 0)
+    log_error("Xml_Parsebuffer encountered an error");
+  return res;
+}
+
+void* XmppParser::get_buffer(const size_t size) const
 {
-  XML_Parse(this->parser, data, len, is_final);
+  return XML_GetBuffer(this->parser, static_cast<int>(size));
 }
 
 void XmppParser::start_element(const XML_Char* name, const XML_Char** attribute)
diff --git a/src/xmpp/xmpp_parser.hpp b/src/xmpp/xmpp_parser.hpp
index afdfdfa..df9cda7 100644
--- a/src/xmpp/xmpp_parser.hpp
+++ b/src/xmpp/xmpp_parser.hpp
@@ -37,7 +37,15 @@ public:
   /**
    * Feed the parser with some XML data
    */
-  void feed(const char* data, const int len, const bool is_final);
+  int feed(const char* data, const int len, const bool is_final);
+  /**
+   * Parse the data placed in the parser buffer
+   */
+  int parse(const int size, const bool is_final);
+  /**
+   * Get a buffer provided by the xml parser.
+   */
+  void* get_buffer(const size_t size) const;
   /**
    * Add one callback for the various events that this parser can spawn.
    */