From 86d4347af8532ef85472e47c01d645fa5ad1b3b1 Mon Sep 17 00:00:00 2001 From: Florent Le Coz Date: Fri, 28 Feb 2014 01:14:38 +0100 Subject: [PATCH] Avoid unnecessary copies by recv()ing data directly into the expat buffer --- src/irc/irc_client.cpp | 2 +- src/irc/irc_client.hpp | 2 +- src/network/socket_handler.cpp | 26 +++++++++++++++++++++----- src/network/socket_handler.hpp | 20 +++++++++++++++++--- src/xmpp/xmpp_component.cpp | 21 ++++++++++++++++++--- src/xmpp/xmpp_component.hpp | 8 ++++++-- src/xmpp/xmpp_parser.cpp | 22 ++++++++++++++++++++-- src/xmpp/xmpp_parser.hpp | 10 +++++++++- 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 #include +#include + /** * 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(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. */ -- 2.45.2