From 9419c857fdbcf54de60fc0fa450d7aaa6af2bdf6 Mon Sep 17 00:00:00 2001 From: Nikolai Vladimirov Date: Wed, 2 Jul 2025 18:39:43 +0000 Subject: [PATCH] Corrected and cleaner implementation with comments --- example/cpp20_subscriber.cpp | 3 + include/boost/redis/impl/response.ipp | 37 ++++++--- include/boost/redis/resp3/node.hpp | 8 ++ include/boost/redis/response.hpp | 105 +++++++++----------------- 4 files changed, 71 insertions(+), 82 deletions(-) diff --git a/example/cpp20_subscriber.cpp b/example/cpp20_subscriber.cpp index d2ec462f..09536dd1 100644 --- a/example/cpp20_subscriber.cpp +++ b/example/cpp20_subscriber.cpp @@ -23,6 +23,7 @@ namespace asio = boost::asio; using namespace std::chrono_literals; using boost::redis::request; using boost::redis::generic_response; +using boost::redis::generic_flat_response; using boost::redis::consume_one; using boost::redis::logger; using boost::redis::config; @@ -54,6 +55,8 @@ auto receiver(std::shared_ptr conn) -> asio::awaitable request req; req.push("SUBSCRIBE", "channel"); + // Alternatively, you can use generic_flat_response here, but keep in mind + // that to access elements you need to call .view() on resp.value() generic_response resp; conn->set_receive_response(resp); diff --git a/include/boost/redis/impl/response.ipp b/include/boost/redis/impl/response.ipp index a4b09a6e..19194c68 100644 --- a/include/boost/redis/impl/response.ipp +++ b/include/boost/redis/impl/response.ipp @@ -11,15 +11,30 @@ namespace boost::redis { -void consume_one(generic_response& r, system::error_code& ec) +namespace { +template +auto& get_value(Container& c) +{ + return c; +} + +template <> +auto& get_value(flat_response_value& c) +{ + return c.view(); +} + +template +void consume_one_impl(Response& r, system::error_code& ec) { if (r.has_error()) return; // Nothing to consume. - if (std::empty(r.value())) + auto& value = get_value(r.value()); + if (std::empty(value)) return; // Nothing to consume. - auto const depth = r.value().front().depth; + auto const depth = value.front().depth; // To simplify we will refuse to consume any data-type that is not // a root node. I think there is no use for that and it is complex @@ -33,17 +48,15 @@ void consume_one(generic_response& r, system::error_code& ec) return e.depth == depth; }; - auto match = std::find_if(std::next(std::cbegin(r.value())), std::cend(r.value()), f); + auto match = std::find_if(std::next(std::cbegin(value)), std::cend(value), f); - r.value().erase(std::cbegin(r.value()), match); + value.erase(std::cbegin(value), match); } -void consume_one(generic_response& r) -{ - system::error_code ec; - consume_one(r, ec); - if (ec) - throw system::system_error(ec); -} +} // namespace + +void consume_one(generic_response& r, system::error_code& ec) { consume_one_impl(r, ec); } + +void consume_one(generic_flat_response& r, system::error_code& ec) { consume_one_impl(r, ec); } } // namespace boost::redis diff --git a/include/boost/redis/resp3/node.hpp b/include/boost/redis/resp3/node.hpp index 6e3f4aac..85147b1c 100644 --- a/include/boost/redis/resp3/node.hpp +++ b/include/boost/redis/resp3/node.hpp @@ -59,6 +59,14 @@ using node = basic_node; /// A node in the response tree that does not own its data. using node_view = basic_node; +struct offset_string { + std::string_view data; + std::size_t offset{}; + std::size_t size{}; +}; + +using offset_node = basic_node; + } // namespace boost::redis::resp3 #endif // BOOST_REDIS_RESP3_NODE_HPP diff --git a/include/boost/redis/response.hpp b/include/boost/redis/response.hpp index d54a3837..a8a8a414 100644 --- a/include/boost/redis/response.hpp +++ b/include/boost/redis/response.hpp @@ -32,89 +32,48 @@ using response = std::tuple...>; using generic_response = adapter::result>; struct flat_response_value { -private: - class iterator { - public: - using value_type = resp3::node_view; - using difference_type = std::ptrdiff_t; - using pointer = void; - using reference = value_type; - using iterator_category = std::forward_iterator_tag; - - explicit iterator(flat_response_value* owner, std::size_t i) noexcept - : owner_(owner) - , index_(i) - { } - - value_type operator*() const { return owner_->operator[](index_); } - - iterator& operator++() - { - ++index_; - return *this; - } - - bool operator==(const iterator& other) const { return index_ == other.index_; } - bool operator!=(const iterator& other) const { return !(*this == other); } - - private: - flat_response_value* owner_; - std::size_t index_; - }; - - struct offset_node : resp3::node_view { - std::size_t offset{}; - std::size_t size{}; - }; - public: - void reserve(std::size_t new_cap, std::size_t elem_length) + /// Reserve capacity for nodes and data storage. + void reserve(std::size_t num_nodes, std::size_t string_size) { - data_.reserve(new_cap * elem_length); - view_.reserve(new_cap); + data_.reserve(num_nodes * string_size); + view_.reserve(num_nodes); } - resp3::node_view at(std::size_t index) { return make_node_view(view_.at(index)); } - - std::size_t size() { return view_.size(); } - - resp3::node_view operator[](std::size_t index) { return make_node_view(view_[index]); } - - iterator begin() { return iterator{this, 0}; } - - iterator end() { return iterator{this, view_.size()}; } + std::vector const& view() const { return view_; } + std::vector& view() { return view_; } template void push_back(const resp3::basic_node& nd) { - offset_node new_node; + resp3::offset_string offset_string; + offset_string.offset = data_.size(); + offset_string.size = nd.value.size(); + + data_.append(nd.value.data()); + + offset_string.data = std::string_view{ + data_.data() + offset_string.offset, + offset_string.size}; + + resp3::offset_node new_node; new_node.data_type = nd.data_type; new_node.aggregate_size = nd.aggregate_size; new_node.depth = nd.depth; - new_node.offset = data_.size(); - new_node.size = nd.value.size(); + new_node.value = std::move(offset_string); - data_.append(nd.value.data()); view_.push_back(std::move(new_node)); } private: - resp3::node_view make_node_view(const offset_node& nd) - { - resp3::node_view result; - result.data_type = nd.data_type; - result.aggregate_size = nd.aggregate_size; - result.depth = nd.depth; - result.value = std::string_view{data_.data() + nd.offset, nd.size}; - return result; - } - std::string data_; - std::vector view_; + std::vector view_; }; -/** - * TODO: documentation +/** @brief A memory-efficient generic response to a request. + * @ingroup high-level-api + * + * Uses a compact buffer to store RESP3 data with reduced allocations. */ using generic_flat_response = adapter::result; @@ -159,12 +118,18 @@ using generic_flat_response = adapter::result; */ void consume_one(generic_response& r, system::error_code& ec); -/** - * @brief Throwing overload of `consume_one`. - * - * @param r The response to modify. - */ -void consume_one(generic_response& r); +/// Consume on response from a generic flat response +void consume_one(generic_flat_response& r, system::error_code& ec); + +/// Throwing overload of `consume_one`. +template +void consume_one(Response& r) +{ + system::error_code ec; + consume_one(r, ec); + if (ec) + throw system::system_error(ec); +} } // namespace boost::redis