Skip to content

Commit

Permalink
Improve reception performance
Browse files Browse the repository at this point in the history
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: #6
  • Loading branch information
testillano committed Jul 12, 2022
1 parent 2b71f1c commit 95a1aeb
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 20 deletions.
10 changes: 5 additions & 5 deletions include/ert/http2comm/Http2Server.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -224,7 +224,7 @@ class Http2Server
*/
virtual void receive(const std::uint64_t &receptionId,
const nghttp2::asio_http2::server::request& req,
std::shared_ptr<std::stringstream> requestBody,
const std::string &requestBody,
const std::chrono::microseconds &receptionTimestampUs,
unsigned int& statusCode,
nghttp2::asio_http2::header_map& headers,
Expand All @@ -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<std::stringstream> requestBody,
const std::string &requestBody,
unsigned int& statusCode,
nghttp2::asio_http2::header_map& headers,
std::string& responseBody,
Expand Down
15 changes: 11 additions & 4 deletions include/ert/http2comm/Stream.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@ SOFTWARE.
#pragma once

#include <mutex>
#include <sstream>
#include <memory>
#include <chrono>

Expand Down Expand Up @@ -78,7 +77,7 @@ class Stream : public std::enable_shared_from_this<Stream>
std::mutex mutex_;
const nghttp2::asio_http2::server::request& req_;
const nghttp2::asio_http2::server::response& res_;
std::shared_ptr<std::stringstream> request_body_;
std::string request_body_;
Http2Server *server_;
bool closed_;

Expand All @@ -98,8 +97,7 @@ class Stream : public std::enable_shared_from_this<Stream>

Stream(const nghttp2::asio_http2::server::request& req,
const nghttp2::asio_http2::server::response& res,
std::shared_ptr<std::stringstream> 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;
Expand All @@ -110,6 +108,15 @@ class Stream : public std::enable_shared_from_this<Stream>
return req_;
}

// append received data chunk
void appendData(const uint8_t* data, std::size_t len) {
// std::copy(data, data + len, std::ostream_iterator<std::uint8_t>(*requestBody));
// where we have std::shared_ptr request_body_ = std::make_shared<std::stringstream>();
//
// 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;
Expand Down
11 changes: 5 additions & 6 deletions src/Http2Server.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ std::string Http2Server::getApiPath() const
}

void Http2Server::receiveError(const nghttp2::asio_http2::server::request& req,
std::shared_ptr<std::stringstream> requestBody,
const std::string &requestBody,
unsigned int& statusCode,
nghttp2::asio_http2::header_map& headers,
std::string& responseBody,
Expand Down Expand Up @@ -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<std::stringstream>();
auto stream = std::make_shared<Stream>(req, res, requestBody, this);
req.on_data([requestBody, stream, this](const uint8_t* data, std::size_t len)
auto stream = std::make_shared<Stream>(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<std::uint8_t>(*requestBody));
// by static type; it seems that data is not correctly protected on lower layers, probably tatsuhiro-t nghttp2)
stream->appendData(data, len);
}
}
else
Expand Down
6 changes: 1 addition & 5 deletions src/Stream.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 95a1aeb

Please sign in to comment.