Simplify HTTP/2 buffering
Using the mem_send methods of nghttp2 can reduce the amount of buffering
we need to do. This is recommended by the nghttp2 docs.
Tested: Enabled experimental-http. Curl succeeds on /redfish/v1, and
shows:
* using HTTP/2
* [HTTP/2] [1] OPENED stream for https://localhost:18080/redfish/v1
Change-Id: I287d8c956f064d244116fac853055a17fca915a2
Signed-off-by: Ed Tanous <ed@tanous.net>
diff --git a/http/http2_connection.hpp b/http/http2_connection.hpp
index a9e91d7..97dcf4e 100644
--- a/http/http2_connection.hpp
+++ b/http/http2_connection.hpp
@@ -16,7 +16,6 @@
#include <boost/asio/ip/tcp.hpp>
#include <boost/asio/ssl/stream.hpp>
#include <boost/asio/steady_timer.hpp>
-#include <boost/beast/core/multi_buffer.hpp>
#include <boost/beast/http/error.hpp>
#include <boost/beast/http/parser.hpp>
#include <boost/beast/http/read.hpp>
@@ -24,9 +23,13 @@
#include <boost/beast/http/write.hpp>
#include <boost/beast/ssl/ssl_stream.hpp>
#include <boost/beast/websocket.hpp>
+#include <boost/system/error_code.hpp>
+#include <array>
#include <atomic>
#include <chrono>
+#include <functional>
+#include <memory>
#include <vector>
namespace crow
@@ -80,6 +83,7 @@
BMCWEB_LOG_ERROR("Fatal error: {}", nghttp2_strerror(rv));
return -1;
}
+ writeBuffer();
return 0;
}
@@ -165,8 +169,7 @@
for (const boost::beast::http::fields::value_type& header : fields)
{
hdr.emplace_back(headerFromStringViews(
- header.name_string(), header.value(),
- NGHTTP2_NV_FLAG_NO_COPY_VALUE | NGHTTP2_NV_FLAG_NO_COPY_NAME));
+ header.name_string(), header.value(), NGHTTP2_NV_FLAG_NONE));
}
Http2StreamData& stream = it->second;
crow::Response& res = stream.res;
@@ -185,7 +188,7 @@
close();
return -1;
}
- ngSession.send();
+ writeBuffer();
return 0;
}
@@ -197,7 +200,6 @@
callbacks.setOnStreamCloseCallback(onStreamCloseCallbackStatic);
callbacks.setOnHeaderCallback(onHeaderCallbackStatic);
callbacks.setOnBeginHeadersCallback(onBeginHeadersCallbackStatic);
- callbacks.setSendCallback(onSendCallbackStatic);
nghttp2_session session(callbacks);
session.setUserData(this);
@@ -433,7 +435,6 @@
self->close();
return;
}
- self->sendBuffer.consume(sendLength);
self->writeBuffer();
}
@@ -443,35 +444,17 @@
{
return;
}
- if (sendBuffer.size() <= 0)
+ std::span<const uint8_t> data = ngSession.memSend();
+ if (data.empty())
{
return;
}
isWriting = true;
adaptor.async_write_some(
- sendBuffer.data(),
+ boost::asio::buffer(data.data(), data.size()),
std::bind_front(afterWriteBuffer, shared_from_this()));
}
- ssize_t onSendCallback(nghttp2_session* /*session */, const uint8_t* data,
- size_t length, int /* flags */)
- {
- BMCWEB_LOG_DEBUG("On send callback size={}", length);
- size_t copied = boost::asio::buffer_copy(
- sendBuffer.prepare(length), boost::asio::buffer(data, length));
- sendBuffer.commit(copied);
- writeBuffer();
- return static_cast<ssize_t>(length);
- }
-
- static ssize_t onSendCallbackStatic(nghttp2_session* session,
- const uint8_t* data, size_t length,
- int flags, void* userData)
- {
- return userPtrToSelf(userData).onSendCallback(session, data, length,
- flags);
- }
-
void close()
{
if constexpr (std::is_same_v<Adaptor,
@@ -486,58 +469,47 @@
}
}
+ void afterDoRead(const std::shared_ptr<self_type>& /*self*/,
+ const boost::system::error_code& ec,
+ size_t bytesTransferred)
+ {
+ BMCWEB_LOG_DEBUG("{} async_read_some {} Bytes", logPtr(this),
+ bytesTransferred);
+
+ if (ec)
+ {
+ BMCWEB_LOG_ERROR("{} Error while reading: {}", logPtr(this),
+ ec.message());
+ close();
+ BMCWEB_LOG_DEBUG("{} from read(1)", logPtr(this));
+ return;
+ }
+ std::span<uint8_t> bufferSpan{inBuffer.data(), bytesTransferred};
+
+ ssize_t readLen = ngSession.memRecv(bufferSpan);
+ if (readLen < 0)
+ {
+ BMCWEB_LOG_ERROR("nghttp2_session_mem_recv returned {}", readLen);
+ close();
+ return;
+ }
+ writeBuffer();
+
+ doRead();
+ }
+
void doRead()
{
BMCWEB_LOG_DEBUG("{} doRead", logPtr(this));
adaptor.async_read_some(
- inBuffer.prepare(8192),
- [this, self(shared_from_this())](
- const boost::system::error_code& ec, size_t bytesTransferred) {
- BMCWEB_LOG_DEBUG("{} async_read_some {} Bytes", logPtr(this),
- bytesTransferred);
-
- if (ec)
- {
- BMCWEB_LOG_ERROR("{} Error while reading: {}", logPtr(this),
- ec.message());
- close();
- BMCWEB_LOG_DEBUG("{} from read(1)", logPtr(this));
- return;
- }
- inBuffer.commit(bytesTransferred);
- // NOLINTBEGIN(cppcoreguidelines-pro-bounds-pointer-arithmetic)
- for (const auto* it =
- boost::asio::buffer_sequence_begin(inBuffer.data());
- it != boost::asio::buffer_sequence_end(inBuffer.data()); it++)
- {
- // NOLINTEND(cppcoreguidelines-pro-bounds-pointer-arithmetic)
- while (inBuffer.size() > 0)
- {
- std::span<const uint8_t> bufferSpan{
- std::bit_cast<const uint8_t*>(it->data()), it->size()};
- BMCWEB_LOG_DEBUG("http2 is getting {} bytes",
- bufferSpan.size());
- ssize_t readLen = ngSession.memRecv(bufferSpan);
- if (readLen <= 0)
- {
- BMCWEB_LOG_ERROR("nghttp2_session_mem_recv returned {}",
- readLen);
- close();
- return;
- }
- inBuffer.consume(static_cast<size_t>(readLen));
- }
- }
-
- doRead();
- });
+ boost::asio::buffer(inBuffer),
+ std::bind_front(&self_type::afterDoRead, this, shared_from_this()));
}
// A mapping from http2 stream ID to Stream Data
std::map<int32_t, Http2StreamData> streams;
- boost::beast::multi_buffer sendBuffer;
- boost::beast::flat_static_buffer<8192> inBuffer;
+ std::array<uint8_t, 8192> inBuffer{};
Adaptor adaptor;
bool isWriting = false;
diff --git a/http/nghttp2_adapters.hpp b/http/nghttp2_adapters.hpp
index 9f4dc91..05ea68d 100644
--- a/http/nghttp2_adapters.hpp
+++ b/http/nghttp2_adapters.hpp
@@ -135,9 +135,11 @@
return nghttp2_session_mem_recv(ptr, buffer.data(), buffer.size());
}
- ssize_t send()
+ std::span<const uint8_t> memSend()
{
- return nghttp2_session_send(ptr);
+ const uint8_t* bytes = nullptr;
+ ssize_t size = nghttp2_session_mem_send(ptr, &bytes);
+ return {bytes, static_cast<size_t>(size)};
}
int submitResponse(int32_t streamId, std::span<const nghttp2_nv> headers,