From 95a1aeb03f342df782094183e1e2b77adb72decc Mon Sep 17 00:00:00 2001 From: "Eduardo Ramos Testillano (ert)" Date: Tue, 12 Jul 2022 17:51:16 +0200 Subject: [PATCH] Improve reception performance String have been probed to have better performance in: * object instantiation (lighter) * append operation And also has: * better usability on user interface Memory reservation could be the only drawback regarding stringstream, but tests done with utility gist [0] for the context of use in nghttp2 server application, demonstrates that this is not an issue. [0] https://gist.github.com/testillano/bc8944eec86fe4e857bf51d61d6c5e42 Issue: https://github.com/testillano/http2comm/issues/6 --- include/ert/http2comm/Http2Server.hpp | 10 +++++----- include/ert/http2comm/Stream.hpp | 15 +++++++++++---- src/Http2Server.cpp | 11 +++++------ src/Stream.cpp | 6 +----- 4 files changed, 22 insertions(+), 20 deletions(-) diff --git a/include/ert/http2comm/Http2Server.hpp b/include/ert/http2comm/Http2Server.hpp index a2bf169..bae88ef 100644 --- a/include/ert/http2comm/Http2Server.hpp +++ b/include/ert/http2comm/Http2Server.hpp @@ -197,9 +197,9 @@ class Http2Server /** * As possible optimization, our server could ignore the request body, so nghttp2 'on_data' - * could be lightened skipping the request body copy into internal stringstream. This is - * useful when huge requests are received and they are not actually needed (for example, - * a dummy server could mock valid static responses regardless the content received). + * could be lightened skipping the request body internal copy. This is useful when huge + * requests are received and they are not actually needed (for example, a dummy server + * could mock valid static responses regardless the content received). * * @param req nghttp2-asio request structure. * This could be used to store data received inside a server internal indexed map. @@ -224,7 +224,7 @@ class Http2Server */ virtual void receive(const std::uint64_t &receptionId, const nghttp2::asio_http2::server::request& req, - std::shared_ptr requestBody, + const std::string &requestBody, const std::chrono::microseconds &receptionTimestampUs, unsigned int& statusCode, nghttp2::asio_http2::header_map& headers, @@ -249,7 +249,7 @@ class Http2Server * @param allowedMethods allowed methods vector given by server implementation. Empty by default. */ virtual void receiveError(const nghttp2::asio_http2::server::request& req, - std::shared_ptr requestBody, + const std::string &requestBody, unsigned int& statusCode, nghttp2::asio_http2::header_map& headers, std::string& responseBody, diff --git a/include/ert/http2comm/Stream.hpp b/include/ert/http2comm/Stream.hpp index fab32b2..a3ae4c1 100644 --- a/include/ert/http2comm/Stream.hpp +++ b/include/ert/http2comm/Stream.hpp @@ -40,7 +40,6 @@ SOFTWARE. #pragma once #include -#include #include #include @@ -78,7 +77,7 @@ class Stream : public std::enable_shared_from_this std::mutex mutex_; const nghttp2::asio_http2::server::request& req_; const nghttp2::asio_http2::server::response& res_; - std::shared_ptr request_body_; + std::string request_body_; Http2Server *server_; bool closed_; @@ -98,8 +97,7 @@ class Stream : public std::enable_shared_from_this Stream(const nghttp2::asio_http2::server::request& req, const nghttp2::asio_http2::server::response& res, - std::shared_ptr requestBody, - Http2Server *server) : req_(req), res_(res), request_body_(requestBody), server_(server), closed_(false), timer_(nullptr) {} + Http2Server *server) : req_(req), res_(res), server_(server), closed_(false), timer_(nullptr) {} Stream(const Stream&) = delete; ~Stream() = default; @@ -110,6 +108,15 @@ class Stream : public std::enable_shared_from_this return req_; } + // append received data chunk + void appendData(const uint8_t* data, std::size_t len) { + // std::copy(data, data + len, std::ostream_iterator(*requestBody)); + // where we have std::shared_ptr request_body_ = std::make_shared(); + // + // BUT: std::string append has better performance than stringstream one (https://gist.github.com/testillano/bc8944eec86fe4e857bf51d61d6c5e42): + request_body_.append((const char *)data, len); + } + // set server sequence void setReceptionId(const std::uint64_t &id) { reception_id_ = id; diff --git a/src/Http2Server.cpp b/src/Http2Server.cpp index 7d75a7d..cfaeda2 100644 --- a/src/Http2Server.cpp +++ b/src/Http2Server.cpp @@ -125,7 +125,7 @@ std::string Http2Server::getApiPath() const } void Http2Server::receiveError(const nghttp2::asio_http2::server::request& req, - std::shared_ptr requestBody, + const std::string &requestBody, unsigned int& statusCode, nghttp2::asio_http2::header_map& headers, std::string& responseBody, @@ -185,17 +185,16 @@ nghttp2::asio_http2::server::request_cb Http2Server::handler() return [&](const nghttp2::asio_http2::server::request & req, const nghttp2::asio_http2::server::response & res) { - auto requestBody = std::make_shared(); - auto stream = std::make_shared(req, res, requestBody, this); - req.on_data([requestBody, stream, this](const uint8_t* data, std::size_t len) + auto stream = std::make_shared(req, res, this); + req.on_data([stream, this](const uint8_t* data, std::size_t len) { if (len > 0) // https://stackoverflow.com/a/72925875/2576671 { if (receiveDataLen(stream->getReq())) { // https://github.com/testillano/h2agent/issues/6 is caused when this is enabled, on high load and broke client connections: // (mutexes does not solves the problem neither std::move of data, and does not matter is shared_ptr requestBody is replaced - // by static type like stringstream; it seems that data is not correctly protected on lower layers, probably tatsuhiro-t nghttp2) - std::copy(data, data + len, std::ostream_iterator(*requestBody)); + // by static type; it seems that data is not correctly protected on lower layers, probably tatsuhiro-t nghttp2) + stream->appendData(data, len); } } else diff --git a/src/Stream.cpp b/src/Stream.cpp index 9fc1409..96295f2 100644 --- a/src/Stream.cpp +++ b/src/Stream.cpp @@ -164,12 +164,8 @@ void Stream::close() { ); server_->responses_delay_seconds_gauge_->Set(durationSeconds); - // Efficient size calculus (better than request_body_->str().size(), because the string don't have to be built): - request_body_->seekg(0, std::ios::end); - size_t requestBodySize = request_body_->tellg(); - //request_body_->seekg(0, std::ios::beg); // restore position (depends on what you will do) - size_t responseBodySize = response_body_.size(); + size_t requestBodySize = request_body_.size(); server_->messages_size_bytes_rx_gauge_->Set(requestBodySize); server_->messages_size_bytes_tx_gauge_->Set(responseBodySize);