From c8b8e8fcbb1589c472aa3c7d5430ea09a60690f3 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Thu, 20 Nov 2025 17:12:55 +0100 Subject: [PATCH 01/36] Initial impl --- include/boost/redis/impl/flat_tree.ipp | 197 +++++++++++++++--------- include/boost/redis/resp3/flat_tree.hpp | 64 ++++---- test/test_low_level_sync_sans_io.cpp | 6 +- 3 files changed, 152 insertions(+), 115 deletions(-) diff --git a/include/boost/redis/impl/flat_tree.ipp b/include/boost/redis/impl/flat_tree.ipp index 095e48ae6..e5dffb9f3 100644 --- a/include/boost/redis/impl/flat_tree.ipp +++ b/include/boost/redis/impl/flat_tree.ipp @@ -7,119 +7,166 @@ // #include +#include + #include +#include +#include +#include + namespace boost::redis::resp3 { -flat_tree::flat_tree(flat_tree const& other) -: data_{other.data_} -, view_tree_{other.view_tree_} -, ranges_{other.ranges_} -, pos_{0u} -, reallocs_{0u} -, total_msgs_{other.total_msgs_} -{ - view_tree_.resize(ranges_.size()); - set_views(); -} +namespace detail { -flat_tree& -flat_tree::operator=(flat_tree other) +inline std::size_t compute_capacity(std::size_t requested_cap, std::size_t current_cap) { - swap(*this, other); - return *this; + // Prevent many small allocations when starting from an empty buffer + requested_cap = (std::max)(requested_cap, static_cast(512u)); + return (std::max)(requested_cap, 2 * current_cap); } -void flat_tree::reserve(std::size_t bytes, std::size_t nodes) +inline buffer_repr copy(const buffer_repr& rhs) { - data_.reserve(bytes); - view_tree_.reserve(nodes); - ranges_.reserve(nodes); + buffer_repr res{{}, rhs.size, rhs.capacity}; + + if (rhs.data) { + BOOST_ASSERT(rhs.size > 0u); + res.data.reset(new char[rhs.capacity]); + std::memcpy(res.data.get(), rhs.data.get(), rhs.size); + } + + return res; } -void flat_tree::clear() +inline void copy_nodes( + const view_tree& from, + const char* from_base, + view_tree& to, + const char* to_base) { - pos_ = 0u; - total_msgs_ = 0u; - reallocs_ = 0u; - data_.clear(); - view_tree_.clear(); - ranges_.clear(); + to.reserve(from.size()); + for (const auto& nd : from) { + const std::size_t offset = nd.value.data() - from_base; + to.push_back({ + nd.data_type, + nd.aggregate_size, + nd.depth, + std::string_view{to_base + offset, nd.value.size()} + }); + } } -void flat_tree::set_views() +} // namespace detail + +void flat_tree::grow(std::size_t new_capacity) { - BOOST_ASSERT_MSG(pos_ < view_tree_.size(), "notify_done called but no nodes added."); - BOOST_ASSERT_MSG(view_tree_.size() == ranges_.size(), "Incompatible sizes."); + BOOST_ASSERT(new_capacity > data_.capacity); + + // Compute the actual capacity that we will be using + new_capacity = detail::compute_capacity(new_capacity, data_.capacity); + + // Allocate space + std::unique_ptr new_buffer{new char[new_capacity]}; - for (; pos_ < view_tree_.size(); ++pos_) { - auto const& r = ranges_.at(pos_); - view_tree_.at(pos_).value = std::string_view{data_.data() + r.offset, r.size}; + if (data_.data) { + BOOST_ASSERT(data_.size > 0u); + + // Rebase strings + const char* data_before = data_.data.get(); + char* data_after = new_buffer.get(); + for (auto& nd : view_tree_) { + auto offset = nd.value.data() - data_before; + nd.value = {data_after + offset, nd.value.size()}; + } + + // Copy contents + std::memcpy(data_after, data_before, data_.size); } + + // Replace the buffer + data_.data = std::move(new_buffer); + data_.capacity = new_capacity; + ++reallocs_; } -void flat_tree::notify_done() +flat_tree::flat_tree(flat_tree const& other) +: data_{detail::copy(other.data_)} +, reallocs_{0u} +, total_msgs_{other.total_msgs_} { - total_msgs_ += 1; - set_views(); + detail::copy_nodes(other.view_tree_, other.data_.data.get(), view_tree_, data_.data.get()); } -void flat_tree::push(node_view const& node) +flat_tree& flat_tree::operator=(const flat_tree& other) { - auto data_before = data_.data(); - add_node_impl(node); - auto data_after = data_.data(); - - if (data_after != data_before) { - pos_ = 0; - reallocs_ += 1; + if (this != &other) { + // Copy the data + if (data_.capacity >= other.data_.capacity) { + std::memcpy(data_.data.get(), other.data_.data.get(), other.data_.size); + data_.size = other.data_.size; + } else { + data_ = detail::copy(other.data_); + } + + // Copy the nodes + view_tree_.clear(); + detail::copy_nodes(other.view_tree_, other.data_.data.get(), view_tree_, data_.data.get()); + + // Copy the other fields + reallocs_ = other.reallocs_; + total_msgs_ = other.total_msgs_; } + + return *this; } -void flat_tree::add_node_impl(node_view const& node) +void flat_tree::reserve(std::size_t bytes, std::size_t nodes) { - ranges_.push_back({data_.size(), node.value.size()}); - - // This must come after setting the offset above. - data_.insert(data_.end(), node.value.begin(), node.value.end()); + // Space for the strings + if (bytes > data_.capacity) { + grow(bytes); + } - view_tree_.push_back(node); + // Space for the nodes + view_tree_.reserve(nodes); } -void swap(flat_tree& a, flat_tree& b) +void flat_tree::clear() { - using std::swap; - - swap(a.data_, b.data_); - swap(a.view_tree_, b.view_tree_); - swap(a.ranges_, b.ranges_); - swap(a.pos_, b.pos_); - swap(a.reallocs_, b.reallocs_); - swap(a.total_msgs_, b.total_msgs_); + data_.size = 0u; + view_tree_.clear(); + reallocs_ = 0u; + total_msgs_ = 0u; } -bool -operator==( - flat_tree::range const& a, - flat_tree::range const& b) +void flat_tree::push(node_view const& nd) { - return a.offset == b.offset && a.size == b.size; + // Add the string first + const std::size_t offset = data_.size; + const std::size_t new_size = data_.size + nd.value.size(); + if (new_size > data_.capacity) { + grow(new_size); + } + std::memcpy(data_.data.get() + offset, nd.value.data(), nd.value.size()); + data_.size = new_size; + + // Add the node + view_tree_.push_back({ + nd.data_type, + nd.aggregate_size, + nd.depth, + std::string_view{data_.data.get() + offset, nd.value.size()} + }); } bool operator==(flat_tree const& a, flat_tree const& b) { - return - a.data_ == b.data_ && - a.view_tree_ == b.view_tree_ && - a.ranges_ == b.ranges_ && - a.pos_ == b.pos_ && - //a.reallocs_ == b.reallocs_ && - a.total_msgs_ == b.total_msgs_; + // reallocs is not taken into account. + // data is already taken into account by comparing the nodes. + return a.view_tree_ == b.view_tree_ && a.total_msgs_ == b.total_msgs_; } -bool operator!=(flat_tree const& a, flat_tree const& b) -{ - return !(a == b); -} +bool operator!=(flat_tree const& a, flat_tree const& b) { return !(a == b); } -} // namespace boost::redis +} // namespace boost::redis::resp3 diff --git a/include/boost/redis/resp3/flat_tree.hpp b/include/boost/redis/resp3/flat_tree.hpp index 4635b95b9..4a5a468a9 100644 --- a/include/boost/redis/resp3/flat_tree.hpp +++ b/include/boost/redis/resp3/flat_tree.hpp @@ -12,16 +12,25 @@ #include #include -#include +#include +#include namespace boost::redis { namespace adapter::detail { - template class general_aggregate; -} +template class general_aggregate; +} // namespace adapter::detail namespace resp3 { +namespace detail { +struct buffer_repr { + std::unique_ptr data; + std::size_t size = 0u; + std::size_t capacity = 0u; +}; +} // namespace detail + /** @brief A generic-response that stores data contiguously * * Similar to the @ref boost::redis::resp3::tree but data is @@ -38,16 +47,17 @@ struct flat_tree { /// Copy constructor flat_tree(flat_tree const& other); + /// Move assignment + flat_tree& operator=(flat_tree&& other) = default; + /// Copy assignment - flat_tree& operator=(flat_tree other); + flat_tree& operator=(const flat_tree& other); friend void swap(flat_tree&, flat_tree&); - friend - bool operator==(flat_tree const&, flat_tree const&); + friend bool operator==(flat_tree const&, flat_tree const&); - friend - bool operator!=(flat_tree const&, flat_tree const&); + friend bool operator!=(flat_tree const&, flat_tree const&); /** @brief Reserve capacity * @@ -67,64 +77,44 @@ struct flat_tree { void clear(); /// Returns the size of the data buffer - auto data_size() const noexcept -> std::size_t - { return data_.size(); } + auto data_size() const noexcept -> std::size_t { return data_.size; } /// Returns the RESP3 response - auto get_view() const -> view_tree const& - { return view_tree_; } + auto get_view() const -> view_tree const& { return view_tree_; } /** @brief Returns the number of times reallocation took place * * This function returns how many reallocations were performed and * can be useful to determine how much memory to reserve upfront. */ - auto get_reallocs() const noexcept -> std::size_t - { return reallocs_; } + auto get_reallocs() const noexcept -> std::size_t { return reallocs_; } /// Returns the number of complete RESP3 messages contained in this object. - std::size_t get_total_msgs() const noexcept - { return total_msgs_; } + std::size_t get_total_msgs() const noexcept { return total_msgs_; } private: template friend class adapter::detail::general_aggregate; - // Notify the object that all nodes were pushed. - void notify_done(); + void notify_done() { ++total_msgs_; } // Push a new node to the response void push(node_view const& node); - void add_node_impl(node_view const& node); - - void set_views(); + void grow(std::size_t target_size); - // Range into the data buffer. - struct range { - std::size_t offset; - std::size_t size; - }; - - friend bool operator==(range const&, range const&); - - std::vector data_; + detail::buffer_repr data_; view_tree view_tree_; - std::vector ranges_; - std::size_t pos_ = 0u; std::size_t reallocs_ = 0u; std::size_t total_msgs_ = 0u; }; -/// Swaps two responses -void swap(flat_tree&, flat_tree&); - /// Equality operator bool operator==(flat_tree const&, flat_tree const&); /// Inequality operator bool operator!=(flat_tree const&, flat_tree const&); -} // resp3 -} // namespace boost::redis +} // namespace resp3 +} // namespace boost::redis #endif // BOOST_REDIS_RESP3_FLAT_TREE_HPP diff --git a/test/test_low_level_sync_sans_io.cpp b/test/test_low_level_sync_sans_io.cpp index 687263d5b..9d659b16f 100644 --- a/test/test_low_level_sync_sans_io.cpp +++ b/test/test_low_level_sync_sans_io.cpp @@ -395,10 +395,10 @@ BOOST_AUTO_TEST_CASE(flat_tree_views_are_set) deserialize(resp3_set, adapt2(resp3), ec); BOOST_CHECK_EQUAL(ec, error_code{}); - BOOST_CHECK_EQUAL(resp2.get_reallocs(), 4u); + BOOST_CHECK_EQUAL(resp2.get_reallocs(), 1u); BOOST_CHECK_EQUAL(resp2.get_total_msgs(), 1u); - BOOST_CHECK_EQUAL(resp3.value().get_reallocs(), 4u); + BOOST_CHECK_EQUAL(resp3.value().get_reallocs(), 1u); BOOST_CHECK_EQUAL(resp3.value().get_total_msgs(), 1u); auto const tmp2 = from_flat(resp2); @@ -418,7 +418,7 @@ BOOST_AUTO_TEST_CASE(flat_tree_reuse) deserialize(resp3_set, adapt2(tmp), ec); BOOST_CHECK_EQUAL(ec, error_code{}); - BOOST_CHECK_EQUAL(tmp.get_reallocs(), 4u); + BOOST_CHECK_EQUAL(tmp.get_reallocs(), 1u); BOOST_CHECK_EQUAL(tmp.get_total_msgs(), 1u); // Copy to compare after the reuse. From 2ebe82eece8d86785c43f9083b9ba0087dd7d73f Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Thu, 20 Nov 2025 17:24:04 +0100 Subject: [PATCH 02/36] Make buffer a member --- include/boost/redis/impl/flat_tree.ipp | 30 ++++++++++++------------- include/boost/redis/resp3/flat_tree.hpp | 18 +++++++-------- 2 files changed, 24 insertions(+), 24 deletions(-) diff --git a/include/boost/redis/impl/flat_tree.ipp b/include/boost/redis/impl/flat_tree.ipp index e5dffb9f3..3bbf26030 100644 --- a/include/boost/redis/impl/flat_tree.ipp +++ b/include/boost/redis/impl/flat_tree.ipp @@ -26,19 +26,6 @@ inline std::size_t compute_capacity(std::size_t requested_cap, std::size_t curre return (std::max)(requested_cap, 2 * current_cap); } -inline buffer_repr copy(const buffer_repr& rhs) -{ - buffer_repr res{{}, rhs.size, rhs.capacity}; - - if (rhs.data) { - BOOST_ASSERT(rhs.size > 0u); - res.data.reset(new char[rhs.capacity]); - std::memcpy(res.data.get(), rhs.data.get(), rhs.size); - } - - return res; -} - inline void copy_nodes( const view_tree& from, const char* from_base, @@ -59,6 +46,19 @@ inline void copy_nodes( } // namespace detail +flat_tree::buffer flat_tree::copy(const buffer& other) +{ + buffer res{{}, other.size, other.capacity}; + + if (other.data) { + BOOST_ASSERT(other.capacity > 0u); + res.data.reset(new char[other.capacity]); + std::memcpy(res.data.get(), other.data.get(), other.size); + } + + return res; +} + void flat_tree::grow(std::size_t new_capacity) { BOOST_ASSERT(new_capacity > data_.capacity); @@ -91,7 +91,7 @@ void flat_tree::grow(std::size_t new_capacity) } flat_tree::flat_tree(flat_tree const& other) -: data_{detail::copy(other.data_)} +: data_{copy(other.data_)} , reallocs_{0u} , total_msgs_{other.total_msgs_} { @@ -106,7 +106,7 @@ flat_tree& flat_tree::operator=(const flat_tree& other) std::memcpy(data_.data.get(), other.data_.data.get(), other.data_.size); data_.size = other.data_.size; } else { - data_ = detail::copy(other.data_); + data_ = copy(other.data_); } // Copy the nodes diff --git a/include/boost/redis/resp3/flat_tree.hpp b/include/boost/redis/resp3/flat_tree.hpp index 4a5a468a9..fb100ec7d 100644 --- a/include/boost/redis/resp3/flat_tree.hpp +++ b/include/boost/redis/resp3/flat_tree.hpp @@ -23,14 +23,6 @@ template class general_aggregate; namespace resp3 { -namespace detail { -struct buffer_repr { - std::unique_ptr data; - std::size_t size = 0u; - std::size_t capacity = 0u; -}; -} // namespace detail - /** @brief A generic-response that stores data contiguously * * Similar to the @ref boost::redis::resp3::tree but data is @@ -95,6 +87,12 @@ struct flat_tree { private: template friend class adapter::detail::general_aggregate; + struct buffer { + std::unique_ptr data; + std::size_t size = 0u; + std::size_t capacity = 0u; + }; + void notify_done() { ++total_msgs_; } // Push a new node to the response @@ -102,7 +100,9 @@ struct flat_tree { void grow(std::size_t target_size); - detail::buffer_repr data_; + static buffer copy(const buffer& other); + + buffer data_; view_tree view_tree_; std::size_t reallocs_ = 0u; std::size_t total_msgs_ = 0u; From a028853826697b8e77604f86d2682d6e4b00257f Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Thu, 20 Nov 2025 17:39:01 +0100 Subject: [PATCH 03/36] Factor out rebase_tring --- include/boost/redis/impl/flat_tree.ipp | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/include/boost/redis/impl/flat_tree.ipp b/include/boost/redis/impl/flat_tree.ipp index 3bbf26030..07b7d56b9 100644 --- a/include/boost/redis/impl/flat_tree.ipp +++ b/include/boost/redis/impl/flat_tree.ipp @@ -7,6 +7,7 @@ // #include +#include #include #include @@ -26,6 +27,18 @@ inline std::size_t compute_capacity(std::size_t requested_cap, std::size_t curre return (std::max)(requested_cap, 2 * current_cap); } +inline std::string_view rebase_string( + std::string_view value, + const char* old_base, + const char* new_base) +{ + if (value.empty()) + return value; + const auto offset = value.data() - old_base; + BOOST_ASSERT(offset >= 0); + return {new_base + offset, value.size()}; +} + inline void copy_nodes( const view_tree& from, const char* from_base, @@ -34,12 +47,11 @@ inline void copy_nodes( { to.reserve(from.size()); for (const auto& nd : from) { - const std::size_t offset = nd.value.data() - from_base; to.push_back({ nd.data_type, nd.aggregate_size, nd.depth, - std::string_view{to_base + offset, nd.value.size()} + detail::rebase_string(nd.value, from_base, to_base), }); } } @@ -70,15 +82,13 @@ void flat_tree::grow(std::size_t new_capacity) std::unique_ptr new_buffer{new char[new_capacity]}; if (data_.data) { - BOOST_ASSERT(data_.size > 0u); + BOOST_ASSERT(data_.capacity > 0u); // Rebase strings const char* data_before = data_.data.get(); char* data_after = new_buffer.get(); - for (auto& nd : view_tree_) { - auto offset = nd.value.data() - data_before; - nd.value = {data_after + offset, nd.value.size()}; - } + for (auto& nd : view_tree_) + nd.value = detail::rebase_string(nd.value, data_before, data_after); // Copy contents std::memcpy(data_after, data_before, data_.size); From be4d0ff219f2c6ee18c60b284075bf0581a983fb Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Thu, 20 Nov 2025 17:44:49 +0100 Subject: [PATCH 04/36] Simplify copy ops --- include/boost/redis/impl/flat_tree.ipp | 27 ++++++++------------------ 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/include/boost/redis/impl/flat_tree.ipp b/include/boost/redis/impl/flat_tree.ipp index 07b7d56b9..b9e679337 100644 --- a/include/boost/redis/impl/flat_tree.ipp +++ b/include/boost/redis/impl/flat_tree.ipp @@ -39,21 +39,10 @@ inline std::string_view rebase_string( return {new_base + offset, value.size()}; } -inline void copy_nodes( - const view_tree& from, - const char* from_base, - view_tree& to, - const char* to_base) +inline void rebase_strings(view_tree& nodes, const char* old_base, const char* new_base) { - to.reserve(from.size()); - for (const auto& nd : from) { - to.push_back({ - nd.data_type, - nd.aggregate_size, - nd.depth, - detail::rebase_string(nd.value, from_base, to_base), - }); - } + for (auto& nd : nodes) + nd.value = rebase_string(nd.value, old_base, new_base); } } // namespace detail @@ -87,8 +76,7 @@ void flat_tree::grow(std::size_t new_capacity) // Rebase strings const char* data_before = data_.data.get(); char* data_after = new_buffer.get(); - for (auto& nd : view_tree_) - nd.value = detail::rebase_string(nd.value, data_before, data_after); + detail::rebase_strings(view_tree_, data_before, data_after); // Copy contents std::memcpy(data_after, data_before, data_.size); @@ -102,10 +90,11 @@ void flat_tree::grow(std::size_t new_capacity) flat_tree::flat_tree(flat_tree const& other) : data_{copy(other.data_)} +, view_tree_{other.view_tree_} , reallocs_{0u} , total_msgs_{other.total_msgs_} { - detail::copy_nodes(other.view_tree_, other.data_.data.get(), view_tree_, data_.data.get()); + detail::rebase_strings(view_tree_, other.data_.data.get(), data_.data.get()); } flat_tree& flat_tree::operator=(const flat_tree& other) @@ -120,8 +109,8 @@ flat_tree& flat_tree::operator=(const flat_tree& other) } // Copy the nodes - view_tree_.clear(); - detail::copy_nodes(other.view_tree_, other.data_.data.get(), view_tree_, data_.data.get()); + view_tree_ = other.view_tree_; + detail::rebase_strings(view_tree_, other.data_.data.get(), data_.data.get()); // Copy the other fields reallocs_ = other.reallocs_; From 97fe56efe2d4ab4cad44e8ca92d053491a660bda Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Thu, 20 Nov 2025 18:01:20 +0100 Subject: [PATCH 05/36] Better comment --- include/boost/redis/impl/flat_tree.ipp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/include/boost/redis/impl/flat_tree.ipp b/include/boost/redis/impl/flat_tree.ipp index b9e679337..e466c3bb3 100644 --- a/include/boost/redis/impl/flat_tree.ipp +++ b/include/boost/redis/impl/flat_tree.ipp @@ -73,7 +73,9 @@ void flat_tree::grow(std::size_t new_capacity) if (data_.data) { BOOST_ASSERT(data_.capacity > 0u); - // Rebase strings + // Rebase strings. This operation must be performed after allocating + // the new buffer and before freeing the old one. Otherwise, we're + // comparing invalid pointers, which is UB. const char* data_before = data_.data.get(); char* data_after = new_buffer.get(); detail::rebase_strings(view_tree_, data_before, data_after); From 98ea8a8829de0e1216609f9f2da7c2e89a543532 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Thu, 20 Nov 2025 18:30:08 +0100 Subject: [PATCH 06/36] Separate the buffer --- include/boost/redis/impl/flat_tree.ipp | 127 +++++++++++++----------- include/boost/redis/resp3/flat_tree.hpp | 26 ++--- test/test_low_level_sync_sans_io.cpp | 4 +- 3 files changed, 83 insertions(+), 74 deletions(-) diff --git a/include/boost/redis/impl/flat_tree.ipp b/include/boost/redis/impl/flat_tree.ipp index e466c3bb3..94906d24b 100644 --- a/include/boost/redis/impl/flat_tree.ipp +++ b/include/boost/redis/impl/flat_tree.ipp @@ -13,6 +13,7 @@ #include #include +#include #include #include @@ -20,11 +21,64 @@ namespace boost::redis::resp3 { namespace detail { -inline std::size_t compute_capacity(std::size_t requested_cap, std::size_t current_cap) +// --- Operations in flat_buffer --- + +// Copies the entire buffer +inline flat_buffer copy(const flat_buffer& other) { + flat_buffer res{{}, other.size, other.capacity, other.reallocs}; + + if (other.data) { + BOOST_ASSERT(other.capacity > 0u); + res.data.reset(new char[other.capacity]); + std::memcpy(res.data.get(), other.data.get(), other.size); + } + + return res; +} + +// Grows the buffer until reaching a target size +template +inline void grow(flat_buffer& buff, std::size_t new_capacity, Callable on_realloc) +{ + if (new_capacity <= buff.capacity) + return; + + // Compute the actual capacity that we will be using // Prevent many small allocations when starting from an empty buffer - requested_cap = (std::max)(requested_cap, static_cast(512u)); - return (std::max)(requested_cap, 2 * current_cap); + new_capacity = (std::max)(new_capacity, static_cast(512u)); + new_capacity = (std::max)(new_capacity, 2 * buff.capacity); + + // Allocate space + std::unique_ptr new_buffer{new char[new_capacity]}; + + if (buff.size > 0u) { + // Copy any data into the newly allocated space + const char* data_before = buff.data.get(); + char* data_after = new_buffer.get(); + std::memcpy(data_after, data_before, buff.size); + + // Inform the caller that there has been a reallocation + on_realloc(data_before, static_cast(data_after)); + } + + // Replace the buffer. Note that size hasn't changed here + buff.data = std::move(new_buffer); + buff.capacity = new_capacity; + ++buff.reallocs; +} + +// Appends a string to the buffer. There should be enough size for it. +inline std::string_view append(flat_buffer& buff, std::string_view value) +{ + const std::size_t offset = buff.size; + const std::size_t new_size = buff.size + value.size(); + BOOST_ASSERT(buff.capacity >= new_size); + if (!value.empty()) { + std::memmove(buff.data.get() + offset, value.data(), value.size()); + } + buff.size = new_size; + return {buff.data.get() + offset, value.size()}; } inline std::string_view rebase_string( @@ -47,53 +101,16 @@ inline void rebase_strings(view_tree& nodes, const char* old_base, const char* n } // namespace detail -flat_tree::buffer flat_tree::copy(const buffer& other) +void flat_tree::reserve_data(std::size_t new_capacity) { - buffer res{{}, other.size, other.capacity}; - - if (other.data) { - BOOST_ASSERT(other.capacity > 0u); - res.data.reset(new char[other.capacity]); - std::memcpy(res.data.get(), other.data.get(), other.size); - } - - return res; -} - -void flat_tree::grow(std::size_t new_capacity) -{ - BOOST_ASSERT(new_capacity > data_.capacity); - - // Compute the actual capacity that we will be using - new_capacity = detail::compute_capacity(new_capacity, data_.capacity); - - // Allocate space - std::unique_ptr new_buffer{new char[new_capacity]}; - - if (data_.data) { - BOOST_ASSERT(data_.capacity > 0u); - - // Rebase strings. This operation must be performed after allocating - // the new buffer and before freeing the old one. Otherwise, we're - // comparing invalid pointers, which is UB. - const char* data_before = data_.data.get(); - char* data_after = new_buffer.get(); - detail::rebase_strings(view_tree_, data_before, data_after); - - // Copy contents - std::memcpy(data_after, data_before, data_.size); - } - - // Replace the buffer - data_.data = std::move(new_buffer); - data_.capacity = new_capacity; - ++reallocs_; + detail::grow(data_, new_capacity, [this](const char* old_base, const char* new_base) { + detail::rebase_strings(view_tree_, old_base, new_base); + }); } flat_tree::flat_tree(flat_tree const& other) -: data_{copy(other.data_)} +: data_{detail::copy(other.data_)} , view_tree_{other.view_tree_} -, reallocs_{0u} , total_msgs_{other.total_msgs_} { detail::rebase_strings(view_tree_, other.data_.data.get(), data_.data.get()); @@ -115,7 +132,6 @@ flat_tree& flat_tree::operator=(const flat_tree& other) detail::rebase_strings(view_tree_, other.data_.data.get(), data_.data.get()); // Copy the other fields - reallocs_ = other.reallocs_; total_msgs_ = other.total_msgs_; } @@ -125,9 +141,7 @@ flat_tree& flat_tree::operator=(const flat_tree& other) void flat_tree::reserve(std::size_t bytes, std::size_t nodes) { // Space for the strings - if (bytes > data_.capacity) { - grow(bytes); - } + reserve_data(bytes); // Space for the nodes view_tree_.reserve(nodes); @@ -137,33 +151,26 @@ void flat_tree::clear() { data_.size = 0u; view_tree_.clear(); - reallocs_ = 0u; total_msgs_ = 0u; } void flat_tree::push(node_view const& nd) { - // Add the string first - const std::size_t offset = data_.size; - const std::size_t new_size = data_.size + nd.value.size(); - if (new_size > data_.capacity) { - grow(new_size); - } - std::memcpy(data_.data.get() + offset, nd.value.data(), nd.value.size()); - data_.size = new_size; + // Add the string + reserve_data(data_.size + nd.value.size()); + const std::string_view str = detail::append(data_, nd.value); // Add the node view_tree_.push_back({ nd.data_type, nd.aggregate_size, nd.depth, - std::string_view{data_.data.get() + offset, nd.value.size()} + str, }); } bool operator==(flat_tree const& a, flat_tree const& b) { - // reallocs is not taken into account. // data is already taken into account by comparing the nodes. return a.view_tree_ == b.view_tree_ && a.total_msgs_ == b.total_msgs_; } diff --git a/include/boost/redis/resp3/flat_tree.hpp b/include/boost/redis/resp3/flat_tree.hpp index fb100ec7d..1548dc43d 100644 --- a/include/boost/redis/resp3/flat_tree.hpp +++ b/include/boost/redis/resp3/flat_tree.hpp @@ -23,6 +23,17 @@ template class general_aggregate; namespace resp3 { +namespace detail { + +struct flat_buffer { + std::unique_ptr data; + std::size_t size = 0u; + std::size_t capacity = 0u; + std::size_t reallocs = 0u; +}; + +} // namespace detail + /** @brief A generic-response that stores data contiguously * * Similar to the @ref boost::redis::resp3::tree but data is @@ -79,7 +90,7 @@ struct flat_tree { * This function returns how many reallocations were performed and * can be useful to determine how much memory to reserve upfront. */ - auto get_reallocs() const noexcept -> std::size_t { return reallocs_; } + auto get_reallocs() const noexcept -> std::size_t { return data_.reallocs; } /// Returns the number of complete RESP3 messages contained in this object. std::size_t get_total_msgs() const noexcept { return total_msgs_; } @@ -87,24 +98,15 @@ struct flat_tree { private: template friend class adapter::detail::general_aggregate; - struct buffer { - std::unique_ptr data; - std::size_t size = 0u; - std::size_t capacity = 0u; - }; - void notify_done() { ++total_msgs_; } // Push a new node to the response void push(node_view const& node); - void grow(std::size_t target_size); - - static buffer copy(const buffer& other); + void reserve_data(std::size_t target_size); - buffer data_; + detail::flat_buffer data_; view_tree view_tree_; - std::size_t reallocs_ = 0u; std::size_t total_msgs_ = 0u; }; diff --git a/test/test_low_level_sync_sans_io.cpp b/test/test_low_level_sync_sans_io.cpp index 9d659b16f..aa109dede 100644 --- a/test/test_low_level_sync_sans_io.cpp +++ b/test/test_low_level_sync_sans_io.cpp @@ -429,8 +429,8 @@ BOOST_AUTO_TEST_CASE(flat_tree_reuse) deserialize(resp3_set, adapt2(tmp), ec); BOOST_CHECK_EQUAL(ec, error_code{}); - // No reallocation this time - BOOST_CHECK_EQUAL(tmp.get_reallocs(), 0u); + // No reallocation this time. TODO: check this + BOOST_CHECK_EQUAL(tmp.get_reallocs(), 1u); BOOST_CHECK_EQUAL(tmp.get_total_msgs(), 1u); BOOST_CHECK_EQUAL(resp1, tmp.get_view()); From bb57e206bef2ac12f972da7da8adfa83a6f27fea Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Thu, 20 Nov 2025 18:38:29 +0100 Subject: [PATCH 07/36] encapsulate copy assign --- include/boost/redis/impl/flat_tree.ipp | 33 +++++++++++++++++++------- 1 file changed, 24 insertions(+), 9 deletions(-) diff --git a/include/boost/redis/impl/flat_tree.ipp b/include/boost/redis/impl/flat_tree.ipp index 94906d24b..e4190d244 100644 --- a/include/boost/redis/impl/flat_tree.ipp +++ b/include/boost/redis/impl/flat_tree.ipp @@ -23,8 +23,8 @@ namespace detail { // --- Operations in flat_buffer --- -// Copies the entire buffer -inline flat_buffer copy(const flat_buffer& other) +// Copy construction +inline flat_buffer copy_construct(const flat_buffer& other) { flat_buffer res{{}, other.size, other.capacity, other.reallocs}; @@ -37,6 +37,26 @@ inline flat_buffer copy(const flat_buffer& other) return res; } +// Copy assignment +inline void copy_assign(flat_buffer& buff, const flat_buffer& other) +{ + // Make space if required + if (buff.capacity < other.capacity) { + buff.data.reset(new char[other.capacity]); + buff.capacity = other.capacity; + } + + // Copy the contents + if (other.size > 0u) { + BOOST_ASSERT(other.data.get() != nullptr); + std::memcpy(buff.data.get(), other.data.get(), other.size); + } + + // Copy the other fields + buff.size = other.size; + buff.reallocs = other.reallocs; +} + // Grows the buffer until reaching a target size template inline void grow(flat_buffer& buff, std::size_t new_capacity, Callable on_realloc) @@ -109,7 +129,7 @@ void flat_tree::reserve_data(std::size_t new_capacity) } flat_tree::flat_tree(flat_tree const& other) -: data_{detail::copy(other.data_)} +: data_{detail::copy_construct(other.data_)} , view_tree_{other.view_tree_} , total_msgs_{other.total_msgs_} { @@ -120,12 +140,7 @@ flat_tree& flat_tree::operator=(const flat_tree& other) { if (this != &other) { // Copy the data - if (data_.capacity >= other.data_.capacity) { - std::memcpy(data_.data.get(), other.data_.data.get(), other.data_.size); - data_.size = other.data_.size; - } else { - data_ = copy(other.data_); - } + detail::copy_assign(data_, other.data_); // Copy the nodes view_tree_ = other.view_tree_; From 61b8cff1ca2d100b2c2ee37a732568f5e0dbbcf3 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Thu, 20 Nov 2025 18:47:00 +0100 Subject: [PATCH 08/36] Finish encapsulating --- include/boost/redis/impl/flat_tree.ipp | 72 +++++++++++-------------- include/boost/redis/resp3/flat_tree.hpp | 2 - 2 files changed, 32 insertions(+), 42 deletions(-) diff --git a/include/boost/redis/impl/flat_tree.ipp b/include/boost/redis/impl/flat_tree.ipp index e4190d244..6caafede4 100644 --- a/include/boost/redis/impl/flat_tree.ipp +++ b/include/boost/redis/impl/flat_tree.ipp @@ -21,6 +21,18 @@ namespace boost::redis::resp3 { namespace detail { +// Updates string views by performing pointer arithmetic +inline void rebase_strings(view_tree& nodes, const char* old_base, const char* new_base) +{ + for (auto& nd : nodes) { + if (!nd.value.empty()) { + const auto offset = nd.value.data() - old_base; + BOOST_ASSERT(offset >= 0); + nd.value = {new_base + offset, nd.value.size()}; + } + } +} + // --- Operations in flat_buffer --- // Copy construction @@ -57,9 +69,9 @@ inline void copy_assign(flat_buffer& buff, const flat_buffer& other) buff.reallocs = other.reallocs; } -// Grows the buffer until reaching a target size -template -inline void grow(flat_buffer& buff, std::size_t new_capacity, Callable on_realloc) +// Grows the buffer until reaching a target size. +// Might rebase the strings in nodes +inline void grow(flat_buffer& buff, std::size_t new_capacity, view_tree& nodes) { if (new_capacity <= buff.capacity) return; @@ -78,8 +90,8 @@ inline void grow(flat_buffer& buff, std::size_t new_capacity, Callable on_reallo char* data_after = new_buffer.get(); std::memcpy(data_after, data_before, buff.size); - // Inform the caller that there has been a reallocation - on_realloc(data_before, static_cast(data_after)); + // Update the string views so they don't dangle + rebase_strings(nodes, data_before, data_after); } // Replace the buffer. Note that size hasn't changed here @@ -88,46 +100,27 @@ inline void grow(flat_buffer& buff, std::size_t new_capacity, Callable on_reallo ++buff.reallocs; } -// Appends a string to the buffer. There should be enough size for it. -inline std::string_view append(flat_buffer& buff, std::string_view value) -{ - const std::size_t offset = buff.size; - const std::size_t new_size = buff.size + value.size(); - BOOST_ASSERT(buff.capacity >= new_size); - if (!value.empty()) { - std::memmove(buff.data.get() + offset, value.data(), value.size()); - } - buff.size = new_size; - return {buff.data.get() + offset, value.size()}; -} - -inline std::string_view rebase_string( - std::string_view value, - const char* old_base, - const char* new_base) +// Appends a string to the buffer. +// Might rebase the string in nodes, but doesn't append any new node. +inline std::string_view append(flat_buffer& buff, std::string_view value, view_tree& nodes) { + // If there is nothing to copy, do nothing if (value.empty()) return value; - const auto offset = value.data() - old_base; - BOOST_ASSERT(offset >= 0); - return {new_base + offset, value.size()}; -} -inline void rebase_strings(view_tree& nodes, const char* old_base, const char* new_base) -{ - for (auto& nd : nodes) - nd.value = rebase_string(nd.value, old_base, new_base); + // Make space for the new string + const std::size_t new_size = buff.size + value.size(); + grow(buff, new_size, nodes); + + // Copy the new value + const std::size_t offset = buff.size; + std::memmove(buff.data.get() + offset, value.data(), value.size()); + buff.size = new_size; + return {buff.data.get() + offset, value.size()}; } } // namespace detail -void flat_tree::reserve_data(std::size_t new_capacity) -{ - detail::grow(data_, new_capacity, [this](const char* old_base, const char* new_base) { - detail::rebase_strings(view_tree_, old_base, new_base); - }); -} - flat_tree::flat_tree(flat_tree const& other) : data_{detail::copy_construct(other.data_)} , view_tree_{other.view_tree_} @@ -156,7 +149,7 @@ flat_tree& flat_tree::operator=(const flat_tree& other) void flat_tree::reserve(std::size_t bytes, std::size_t nodes) { // Space for the strings - reserve_data(bytes); + detail::grow(data_, bytes, view_tree_); // Space for the nodes view_tree_.reserve(nodes); @@ -172,8 +165,7 @@ void flat_tree::clear() void flat_tree::push(node_view const& nd) { // Add the string - reserve_data(data_.size + nd.value.size()); - const std::string_view str = detail::append(data_, nd.value); + const std::string_view str = detail::append(data_, nd.value, view_tree_); // Add the node view_tree_.push_back({ diff --git a/include/boost/redis/resp3/flat_tree.hpp b/include/boost/redis/resp3/flat_tree.hpp index 1548dc43d..4116d009d 100644 --- a/include/boost/redis/resp3/flat_tree.hpp +++ b/include/boost/redis/resp3/flat_tree.hpp @@ -103,8 +103,6 @@ struct flat_tree { // Push a new node to the response void push(node_view const& node); - void reserve_data(std::size_t target_size); - detail::flat_buffer data_; view_tree view_tree_; std::size_t total_msgs_ = 0u; From cf6798ae730bd26a5339789a764f01df442418d3 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Thu, 20 Nov 2025 18:48:05 +0100 Subject: [PATCH 09/36] remove swap --- include/boost/redis/resp3/flat_tree.hpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/include/boost/redis/resp3/flat_tree.hpp b/include/boost/redis/resp3/flat_tree.hpp index 4116d009d..d488fa787 100644 --- a/include/boost/redis/resp3/flat_tree.hpp +++ b/include/boost/redis/resp3/flat_tree.hpp @@ -56,8 +56,6 @@ struct flat_tree { /// Copy assignment flat_tree& operator=(const flat_tree& other); - friend void swap(flat_tree&, flat_tree&); - friend bool operator==(flat_tree const&, flat_tree const&); friend bool operator!=(flat_tree const&, flat_tree const&); From 3a0c0ba89644c60f95fded16ffb261ce623b5511 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Fri, 21 Nov 2025 17:32:17 +0100 Subject: [PATCH 10/36] Use std::copy --- include/boost/redis/impl/flat_tree.ipp | 26 ++++++++++---------------- 1 file changed, 10 insertions(+), 16 deletions(-) diff --git a/include/boost/redis/impl/flat_tree.ipp b/include/boost/redis/impl/flat_tree.ipp index 6caafede4..378741c09 100644 --- a/include/boost/redis/impl/flat_tree.ipp +++ b/include/boost/redis/impl/flat_tree.ipp @@ -40,10 +40,9 @@ inline flat_buffer copy_construct(const flat_buffer& other) { flat_buffer res{{}, other.size, other.capacity, other.reallocs}; - if (other.data) { - BOOST_ASSERT(other.capacity > 0u); + if (other.capacity > 0u) { res.data.reset(new char[other.capacity]); - std::memcpy(res.data.get(), other.data.get(), other.size); + std::copy(other.data.get(), other.data.get() + other.size, res.data.get()); } return res; @@ -59,10 +58,7 @@ inline void copy_assign(flat_buffer& buff, const flat_buffer& other) } // Copy the contents - if (other.size > 0u) { - BOOST_ASSERT(other.data.get() != nullptr); - std::memcpy(buff.data.get(), other.data.get(), other.size); - } + std::copy(other.data.get(), other.data.get() + other.size, buff.data.get()); // Copy the other fields buff.size = other.size; @@ -84,15 +80,13 @@ inline void grow(flat_buffer& buff, std::size_t new_capacity, view_tree& nodes) // Allocate space std::unique_ptr new_buffer{new char[new_capacity]}; - if (buff.size > 0u) { - // Copy any data into the newly allocated space - const char* data_before = buff.data.get(); - char* data_after = new_buffer.get(); - std::memcpy(data_after, data_before, buff.size); + // Copy any data into the newly allocated space + const char* data_before = buff.data.get(); + char* data_after = new_buffer.get(); + std::copy(data_before, data_before + buff.size, data_after); - // Update the string views so they don't dangle - rebase_strings(nodes, data_before, data_after); - } + // Update the string views so they don't dangle + rebase_strings(nodes, data_before, data_after); // Replace the buffer. Note that size hasn't changed here buff.data = std::move(new_buffer); @@ -114,7 +108,7 @@ inline std::string_view append(flat_buffer& buff, std::string_view value, view_t // Copy the new value const std::size_t offset = buff.size; - std::memmove(buff.data.get() + offset, value.data(), value.size()); + std::copy(value.data(), value.data() + value.size(), buff.data.get() + offset); buff.size = new_size; return {buff.data.get() + offset, value.size()}; } From 100ae044bf1dbb91b67efffd5e93c444c09b89d5 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Fri, 21 Nov 2025 17:41:30 +0100 Subject: [PATCH 11/36] Move flat_tree tests --- test/CMakeLists.txt | 1 + test/Jamfile | 1 + test/test_flat_tree.cpp | 197 +++++++++++++++++++++++++++ test/test_low_level_sync_sans_io.cpp | 144 -------------------- 4 files changed, 199 insertions(+), 144 deletions(-) create mode 100644 test/test_flat_tree.cpp diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 58ad78a4f..72a28a29d 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -51,6 +51,7 @@ make_test(test_setup_adapter) make_test(test_multiplexer) make_test(test_parse_sentinel_response) make_test(test_update_sentinel_list) +make_test(test_flat_tree) # Tests that require a real Redis server make_test(test_conn_quit) diff --git a/test/Jamfile b/test/Jamfile index 4ac93900a..5db6c0ae4 100644 --- a/test/Jamfile +++ b/test/Jamfile @@ -68,6 +68,7 @@ local tests = test_multiplexer test_parse_sentinel_response test_update_sentinel_list + test_flat_tree ; # Build and run the tests diff --git a/test/test_flat_tree.cpp b/test/test_flat_tree.cpp new file mode 100644 index 000000000..266a768cc --- /dev/null +++ b/test/test_flat_tree.cpp @@ -0,0 +1,197 @@ +/* Copyright (c) 2018-2022 Marcelo Zimbres Silva (mzimbres@gmail.com) + * + * Distributed under the Boost Software License, Version 1.0. (See + * accompanying file LICENSE.txt) + */ + +#include +#include + +#include "print_node.hpp" + +#define BOOST_TEST_MODULE low_level_sync_sans_io +#include + +// #include + +using boost::redis::adapter::adapt2; +using boost::redis::adapter::result; +using boost::redis::resp3::tree; +using boost::redis::resp3::flat_tree; +using boost::redis::generic_flat_response; +using boost::redis::ignore_t; +using boost::redis::resp3::detail::deserialize; +using boost::redis::resp3::node; +using boost::redis::resp3::node_view; +using boost::redis::resp3::to_string; +using boost::redis::response; +using boost::system::error_code; + +namespace boost::redis::resp3 { + +template +std::ostream& operator<<(std::ostream& os, basic_tree const& resp) +{ + for (auto const& e : resp) + os << e << ","; + return os; +} + +} // namespace boost::redis::resp3 + +namespace { + +#define RESP3_SET_PART1 "~6\r\n+orange\r" +#define RESP3_SET_PART2 "\n+apple\r\n+one" +#define RESP3_SET_PART3 "\r\n+two\r" +#define RESP3_SET_PART4 "\n+three\r\n+orange\r\n" +char const* resp3_set = RESP3_SET_PART1 RESP3_SET_PART2 RESP3_SET_PART3 RESP3_SET_PART4; + +node from_node_view(node_view const& v) +{ + node ret; + ret.data_type = v.data_type; + ret.aggregate_size = v.aggregate_size; + ret.depth = v.depth; + ret.value = v.value; + return ret; +} + +tree from_flat(generic_flat_response const& resp) +{ + tree ret; + for (auto const& e : resp.value().get_view()) + ret.push_back(from_node_view(e)); + + return ret; +} + +tree from_flat(flat_tree const& resp) +{ + tree ret; + for (auto const& e : resp.get_view()) + ret.push_back(from_node_view(e)); + + return ret; +} + +// Parses the same data into a tree and a +// flat_tree, they should be equal to each other. +BOOST_AUTO_TEST_CASE(flat_tree_views_are_set) +{ + tree resp1; + flat_tree resp2; + generic_flat_response resp3; + + error_code ec; + deserialize(resp3_set, adapt2(resp1), ec); + BOOST_CHECK_EQUAL(ec, error_code{}); + + deserialize(resp3_set, adapt2(resp2), ec); + BOOST_CHECK_EQUAL(ec, error_code{}); + + deserialize(resp3_set, adapt2(resp3), ec); + BOOST_CHECK_EQUAL(ec, error_code{}); + + BOOST_CHECK_EQUAL(resp2.get_reallocs(), 1u); + BOOST_CHECK_EQUAL(resp2.get_total_msgs(), 1u); + + BOOST_CHECK_EQUAL(resp3.value().get_reallocs(), 1u); + BOOST_CHECK_EQUAL(resp3.value().get_total_msgs(), 1u); + + auto const tmp2 = from_flat(resp2); + BOOST_CHECK_EQUAL(resp1, tmp2); + + auto const tmp3 = from_flat(resp3); + BOOST_CHECK_EQUAL(resp1, tmp3); +} + +// The response should be reusable. +BOOST_AUTO_TEST_CASE(flat_tree_reuse) +{ + flat_tree tmp; + + // First use + error_code ec; + deserialize(resp3_set, adapt2(tmp), ec); + BOOST_CHECK_EQUAL(ec, error_code{}); + + BOOST_CHECK_EQUAL(tmp.get_reallocs(), 1u); + BOOST_CHECK_EQUAL(tmp.get_total_msgs(), 1u); + + // Copy to compare after the reuse. + auto const resp1 = tmp.get_view(); + tmp.clear(); + + // Second use + deserialize(resp3_set, adapt2(tmp), ec); + BOOST_CHECK_EQUAL(ec, error_code{}); + + // No reallocation this time. TODO: check this + BOOST_CHECK_EQUAL(tmp.get_reallocs(), 1u); + BOOST_CHECK_EQUAL(tmp.get_total_msgs(), 1u); + + BOOST_CHECK_EQUAL(resp1, tmp.get_view()); +} + +BOOST_AUTO_TEST_CASE(flat_tree_copy_assign) +{ + flat_tree ref1; + flat_tree ref2; + flat_tree ref3; + flat_tree ref4; + + error_code ec; + deserialize(resp3_set, adapt2(ref1), ec); + deserialize(resp3_set, adapt2(ref2), ec); + deserialize(resp3_set, adapt2(ref3), ec); + deserialize(resp3_set, adapt2(ref4), ec); + BOOST_CHECK_EQUAL(ec, error_code{}); + + // Copy ctor + flat_tree copy1{ref1}; + + // Move ctor + flat_tree move1{std::move(ref2)}; + + // Copy assignment + flat_tree copy2 = ref1; + + // Move assignment + flat_tree move2 = std::move(ref3); + + // Assignment + flat_tree copy3; + copy3 = ref1; + + // Move assignment + flat_tree move3; + move3 = std::move(ref4); + + BOOST_TEST((copy1 == ref1)); + BOOST_TEST((copy2 == ref1)); + BOOST_TEST((copy3 == ref1)); + + BOOST_TEST((move1 == ref1)); + BOOST_TEST((move2 == ref1)); + BOOST_TEST((move3 == ref1)); +} + +} // namespace + +// int main() +// { +// test_push_no_args(); +// test_push_int(); +// test_push_multiple_args(); +// test_push_range(); + +// test_append(); +// test_append_no_response(); +// test_append_flags(); +// test_append_target_empty(); +// test_append_source_empty(); +// test_append_both_empty(); + +// return boost::report_errors(); +// } diff --git a/test/test_low_level_sync_sans_io.cpp b/test/test_low_level_sync_sans_io.cpp index aa109dede..f840fa555 100644 --- a/test/test_low_level_sync_sans_io.cpp +++ b/test/test_low_level_sync_sans_io.cpp @@ -13,8 +13,6 @@ #include #include -#include "print_node.hpp" - #define BOOST_TEST_MODULE low_level_sync_sans_io #include @@ -337,148 +335,6 @@ BOOST_AUTO_TEST_CASE(check_counter_adapter) BOOST_CHECK_EQUAL(done, 1); } -namespace boost::redis::resp3 { - -template -std::ostream& operator<<(std::ostream& os, basic_tree const& resp) -{ - for (auto const& e : resp) - os << e << ","; - return os; -} - -} // namespace boost::redis::resp3 - -node from_node_view(node_view const& v) -{ - node ret; - ret.data_type = v.data_type; - ret.aggregate_size = v.aggregate_size; - ret.depth = v.depth; - ret.value = v.value; - return ret; -} - -tree from_flat(flat_tree const& resp) -{ - tree ret; - for (auto const& e : resp.get_view()) - ret.push_back(from_node_view(e)); - - return ret; -} - -tree from_flat(generic_flat_response const& resp) -{ - tree ret; - for (auto const& e : resp.value().get_view()) - ret.push_back(from_node_view(e)); - - return ret; -} - -// Parses the same data into a tree and a -// flat_tree, they should be equal to each other. -BOOST_AUTO_TEST_CASE(flat_tree_views_are_set) -{ - tree resp1; - flat_tree resp2; - generic_flat_response resp3; - - error_code ec; - deserialize(resp3_set, adapt2(resp1), ec); - BOOST_CHECK_EQUAL(ec, error_code{}); - - deserialize(resp3_set, adapt2(resp2), ec); - BOOST_CHECK_EQUAL(ec, error_code{}); - - deserialize(resp3_set, adapt2(resp3), ec); - BOOST_CHECK_EQUAL(ec, error_code{}); - - BOOST_CHECK_EQUAL(resp2.get_reallocs(), 1u); - BOOST_CHECK_EQUAL(resp2.get_total_msgs(), 1u); - - BOOST_CHECK_EQUAL(resp3.value().get_reallocs(), 1u); - BOOST_CHECK_EQUAL(resp3.value().get_total_msgs(), 1u); - - auto const tmp2 = from_flat(resp2); - BOOST_CHECK_EQUAL(resp1, tmp2); - - auto const tmp3 = from_flat(resp3); - BOOST_CHECK_EQUAL(resp1, tmp3); -} - -// The response should be reusable. -BOOST_AUTO_TEST_CASE(flat_tree_reuse) -{ - flat_tree tmp; - - // First use - error_code ec; - deserialize(resp3_set, adapt2(tmp), ec); - BOOST_CHECK_EQUAL(ec, error_code{}); - - BOOST_CHECK_EQUAL(tmp.get_reallocs(), 1u); - BOOST_CHECK_EQUAL(tmp.get_total_msgs(), 1u); - - // Copy to compare after the reuse. - auto const resp1 = tmp.get_view(); - tmp.clear(); - - // Second use - deserialize(resp3_set, adapt2(tmp), ec); - BOOST_CHECK_EQUAL(ec, error_code{}); - - // No reallocation this time. TODO: check this - BOOST_CHECK_EQUAL(tmp.get_reallocs(), 1u); - BOOST_CHECK_EQUAL(tmp.get_total_msgs(), 1u); - - BOOST_CHECK_EQUAL(resp1, tmp.get_view()); -} - -BOOST_AUTO_TEST_CASE(flat_tree_copy_assign) -{ - flat_tree ref1; - flat_tree ref2; - flat_tree ref3; - flat_tree ref4; - - error_code ec; - deserialize(resp3_set, adapt2(ref1), ec); - deserialize(resp3_set, adapt2(ref2), ec); - deserialize(resp3_set, adapt2(ref3), ec); - deserialize(resp3_set, adapt2(ref4), ec); - BOOST_CHECK_EQUAL(ec, error_code{}); - - // Copy ctor - resp3::flat_tree copy1{ref1}; - - // Move ctor - resp3::flat_tree move1{std::move(ref2)}; - - // Copy assignment - resp3::flat_tree copy2 = ref1; - - // Move assignment - resp3::flat_tree move2 = std::move(ref3); - - // Assignment - resp3::flat_tree copy3; - copy3 = ref1; - - // Move assignment - resp3::flat_tree move3; - move3 = std::move(ref4); - - BOOST_TEST((copy1 == ref1)); - BOOST_TEST((copy2 == ref1)); - BOOST_TEST((copy3 == ref1)); - - BOOST_TEST((move1 == ref1)); - BOOST_TEST((move2 == ref1)); - BOOST_TEST((move3 == ref1)); -} - BOOST_AUTO_TEST_CASE(generic_flat_response_simple_error) { generic_flat_response resp; From f8e71b13789055c8303c994375fe4b339b13e435 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Fri, 21 Nov 2025 17:43:46 +0100 Subject: [PATCH 12/36] use lightweight test --- test/test_flat_tree.cpp | 71 +++++++++++++++++------------------------ 1 file changed, 30 insertions(+), 41 deletions(-) diff --git a/test/test_flat_tree.cpp b/test/test_flat_tree.cpp index 266a768cc..a328fdc55 100644 --- a/test/test_flat_tree.cpp +++ b/test/test_flat_tree.cpp @@ -7,12 +7,9 @@ #include #include -#include "print_node.hpp" - -#define BOOST_TEST_MODULE low_level_sync_sans_io -#include +#include -// #include +#include "print_node.hpp" using boost::redis::adapter::adapt2; using boost::redis::adapter::result; @@ -77,7 +74,7 @@ tree from_flat(flat_tree const& resp) // Parses the same data into a tree and a // flat_tree, they should be equal to each other. -BOOST_AUTO_TEST_CASE(flat_tree_views_are_set) +void test_views_are_set() { tree resp1; flat_tree resp2; @@ -85,39 +82,39 @@ BOOST_AUTO_TEST_CASE(flat_tree_views_are_set) error_code ec; deserialize(resp3_set, adapt2(resp1), ec); - BOOST_CHECK_EQUAL(ec, error_code{}); + BOOST_TEST_EQ(ec, error_code{}); deserialize(resp3_set, adapt2(resp2), ec); - BOOST_CHECK_EQUAL(ec, error_code{}); + BOOST_TEST_EQ(ec, error_code{}); deserialize(resp3_set, adapt2(resp3), ec); - BOOST_CHECK_EQUAL(ec, error_code{}); + BOOST_TEST_EQ(ec, error_code{}); - BOOST_CHECK_EQUAL(resp2.get_reallocs(), 1u); - BOOST_CHECK_EQUAL(resp2.get_total_msgs(), 1u); + BOOST_TEST_EQ(resp2.get_reallocs(), 1u); + BOOST_TEST_EQ(resp2.get_total_msgs(), 1u); - BOOST_CHECK_EQUAL(resp3.value().get_reallocs(), 1u); - BOOST_CHECK_EQUAL(resp3.value().get_total_msgs(), 1u); + BOOST_TEST_EQ(resp3.value().get_reallocs(), 1u); + BOOST_TEST_EQ(resp3.value().get_total_msgs(), 1u); auto const tmp2 = from_flat(resp2); - BOOST_CHECK_EQUAL(resp1, tmp2); + BOOST_TEST_EQ(resp1, tmp2); auto const tmp3 = from_flat(resp3); - BOOST_CHECK_EQUAL(resp1, tmp3); + BOOST_TEST_EQ(resp1, tmp3); } // The response should be reusable. -BOOST_AUTO_TEST_CASE(flat_tree_reuse) +void test_reuse() { flat_tree tmp; // First use error_code ec; deserialize(resp3_set, adapt2(tmp), ec); - BOOST_CHECK_EQUAL(ec, error_code{}); + BOOST_TEST_EQ(ec, error_code{}); - BOOST_CHECK_EQUAL(tmp.get_reallocs(), 1u); - BOOST_CHECK_EQUAL(tmp.get_total_msgs(), 1u); + BOOST_TEST_EQ(tmp.get_reallocs(), 1u); + BOOST_TEST_EQ(tmp.get_total_msgs(), 1u); // Copy to compare after the reuse. auto const resp1 = tmp.get_view(); @@ -125,16 +122,16 @@ BOOST_AUTO_TEST_CASE(flat_tree_reuse) // Second use deserialize(resp3_set, adapt2(tmp), ec); - BOOST_CHECK_EQUAL(ec, error_code{}); + BOOST_TEST_EQ(ec, error_code{}); // No reallocation this time. TODO: check this - BOOST_CHECK_EQUAL(tmp.get_reallocs(), 1u); - BOOST_CHECK_EQUAL(tmp.get_total_msgs(), 1u); + BOOST_TEST_EQ(tmp.get_reallocs(), 1u); + BOOST_TEST_EQ(tmp.get_total_msgs(), 1u); - BOOST_CHECK_EQUAL(resp1, tmp.get_view()); + BOOST_TEST_EQ(resp1, tmp.get_view()); } -BOOST_AUTO_TEST_CASE(flat_tree_copy_assign) +void test_copy_assign() { flat_tree ref1; flat_tree ref2; @@ -146,7 +143,7 @@ BOOST_AUTO_TEST_CASE(flat_tree_copy_assign) deserialize(resp3_set, adapt2(ref2), ec); deserialize(resp3_set, adapt2(ref3), ec); deserialize(resp3_set, adapt2(ref4), ec); - BOOST_CHECK_EQUAL(ec, error_code{}); + BOOST_TEST_EQ(ec, error_code{}); // Copy ctor flat_tree copy1{ref1}; @@ -179,19 +176,11 @@ BOOST_AUTO_TEST_CASE(flat_tree_copy_assign) } // namespace -// int main() -// { -// test_push_no_args(); -// test_push_int(); -// test_push_multiple_args(); -// test_push_range(); - -// test_append(); -// test_append_no_response(); -// test_append_flags(); -// test_append_target_empty(); -// test_append_source_empty(); -// test_append_both_empty(); - -// return boost::report_errors(); -// } +int main() +{ + test_views_are_set(); + test_copy_assign(); + test_reuse(); + + return boost::report_errors(); +} From 6646b6984d20909ea9cddc29ff8ccbac34b824e9 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Fri, 21 Nov 2025 17:45:41 +0100 Subject: [PATCH 13/36] Use container checks --- test/test_flat_tree.cpp | 18 +++--------------- 1 file changed, 3 insertions(+), 15 deletions(-) diff --git a/test/test_flat_tree.cpp b/test/test_flat_tree.cpp index a328fdc55..f55374fda 100644 --- a/test/test_flat_tree.cpp +++ b/test/test_flat_tree.cpp @@ -24,18 +24,6 @@ using boost::redis::resp3::to_string; using boost::redis::response; using boost::system::error_code; -namespace boost::redis::resp3 { - -template -std::ostream& operator<<(std::ostream& os, basic_tree const& resp) -{ - for (auto const& e : resp) - os << e << ","; - return os; -} - -} // namespace boost::redis::resp3 - namespace { #define RESP3_SET_PART1 "~6\r\n+orange\r" @@ -97,10 +85,10 @@ void test_views_are_set() BOOST_TEST_EQ(resp3.value().get_total_msgs(), 1u); auto const tmp2 = from_flat(resp2); - BOOST_TEST_EQ(resp1, tmp2); + BOOST_TEST_ALL_EQ(resp1.begin(), resp1.end(), tmp2.begin(), tmp2.end()); auto const tmp3 = from_flat(resp3); - BOOST_TEST_EQ(resp1, tmp3); + BOOST_TEST_ALL_EQ(resp1.begin(), resp1.end(), tmp3.begin(), tmp3.end()); } // The response should be reusable. @@ -128,7 +116,7 @@ void test_reuse() BOOST_TEST_EQ(tmp.get_reallocs(), 1u); BOOST_TEST_EQ(tmp.get_total_msgs(), 1u); - BOOST_TEST_EQ(resp1, tmp.get_view()); + BOOST_TEST_ALL_EQ(resp1.begin(), resp1.end(), tmp.get_view().begin(), tmp.get_view().end()); } void test_copy_assign() From 20c230c047b4a9dee2a58cc0a5d9f7668b6c1881 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Fri, 21 Nov 2025 17:46:45 +0100 Subject: [PATCH 14/36] simplify the constant --- test/test_flat_tree.cpp | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/test/test_flat_tree.cpp b/test/test_flat_tree.cpp index f55374fda..00f7ddd78 100644 --- a/test/test_flat_tree.cpp +++ b/test/test_flat_tree.cpp @@ -11,6 +11,8 @@ #include "print_node.hpp" +#include + using boost::redis::adapter::adapt2; using boost::redis::adapter::result; using boost::redis::resp3::tree; @@ -26,11 +28,8 @@ using boost::system::error_code; namespace { -#define RESP3_SET_PART1 "~6\r\n+orange\r" -#define RESP3_SET_PART2 "\n+apple\r\n+one" -#define RESP3_SET_PART3 "\r\n+two\r" -#define RESP3_SET_PART4 "\n+three\r\n+orange\r\n" -char const* resp3_set = RESP3_SET_PART1 RESP3_SET_PART2 RESP3_SET_PART3 RESP3_SET_PART4; +constexpr std::string_view + resp3_set = "~6\r\n+orange\r\n+apple\r\n+one\r\n+two\r\n+three\r\n+orange\r\n"; node from_node_view(node_view const& v) { From 5ac045e1a20a04653d289ebede85cb60e20287dc Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Fri, 21 Nov 2025 17:56:32 +0100 Subject: [PATCH 15/36] Default ctor --- test/test_flat_tree.cpp | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/test/test_flat_tree.cpp b/test/test_flat_tree.cpp index 00f7ddd78..5d0b34e49 100644 --- a/test/test_flat_tree.cpp +++ b/test/test_flat_tree.cpp @@ -59,6 +59,17 @@ tree from_flat(flat_tree const& resp) return ret; } +// Constructors +void test_default_constructor() +{ + flat_tree t; + + BOOST_TEST(t.get_view().empty()); + BOOST_TEST_EQ(t.data_size(), 0u); + BOOST_TEST_EQ(t.get_reallocs(), 0u); + BOOST_TEST_EQ(t.get_total_msgs(), 0u); +} + // Parses the same data into a tree and a // flat_tree, they should be equal to each other. void test_views_are_set() @@ -165,6 +176,8 @@ void test_copy_assign() int main() { + test_default_constructor(); + test_views_are_set(); test_copy_assign(); test_reuse(); From 4bf90d4c00aec5943ae133312a058de8f14429bf Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Fri, 21 Nov 2025 19:42:07 +0100 Subject: [PATCH 16/36] Test add nodes --- include/boost/redis/resp3/flat_tree.hpp | 3 + test/test_flat_tree.cpp | 109 +++++++++++++++++++++++- 2 files changed, 109 insertions(+), 3 deletions(-) diff --git a/include/boost/redis/resp3/flat_tree.hpp b/include/boost/redis/resp3/flat_tree.hpp index d488fa787..bc9a7157a 100644 --- a/include/boost/redis/resp3/flat_tree.hpp +++ b/include/boost/redis/resp3/flat_tree.hpp @@ -80,6 +80,9 @@ struct flat_tree { /// Returns the size of the data buffer auto data_size() const noexcept -> std::size_t { return data_.size; } + // TODO: document + auto data_capacity() const noexcept -> std::size_t { return data_.capacity; } + /// Returns the RESP3 response auto get_view() const -> view_tree const& { return view_tree_; } diff --git a/test/test_flat_tree.cpp b/test/test_flat_tree.cpp index 5d0b34e49..b2e38fcb4 100644 --- a/test/test_flat_tree.cpp +++ b/test/test_flat_tree.cpp @@ -6,19 +6,27 @@ #include #include +#include +#include +#include #include +#include #include "print_node.hpp" +#include +#include +#include #include +#include using boost::redis::adapter::adapt2; using boost::redis::adapter::result; using boost::redis::resp3::tree; using boost::redis::resp3::flat_tree; using boost::redis::generic_flat_response; -using boost::redis::ignore_t; +using boost::redis::resp3::type; using boost::redis::resp3::detail::deserialize; using boost::redis::resp3::node; using boost::redis::resp3::node_view; @@ -59,17 +67,111 @@ tree from_flat(flat_tree const& resp) return ret; } -// Constructors +void add_nodes( + flat_tree& to, + std::string_view data, + boost::source_location loc = BOOST_CURRENT_LOCATION) +{ + error_code ec; + deserialize(data, adapt2(to), ec); + if (!BOOST_TEST_EQ(ec, error_code{})) + std::cerr << "Called from " << loc << std::endl; +} + +void check_nodes( + const flat_tree& tree, + boost::span expected, + boost::source_location loc = BOOST_CURRENT_LOCATION) +{ + if (!BOOST_TEST_ALL_EQ( + tree.get_view().begin(), + tree.get_view().end(), + expected.begin(), + expected.end())) + std::cerr << "Called from " << loc << std::endl; +} + void test_default_constructor() { flat_tree t; - BOOST_TEST(t.get_view().empty()); + check_nodes(t, {}); BOOST_TEST_EQ(t.data_size(), 0u); BOOST_TEST_EQ(t.get_reallocs(), 0u); BOOST_TEST_EQ(t.get_total_msgs(), 0u); } +// Adding nodes works, even when reallocations happen. +// Empty nodes don't cause trouble +void test_add_nodes() +{ + flat_tree t; + + // Add a bunch of nodes. Single allocation. Some nodes are empty. + add_nodes(t, "*2\r\n+hello\r\n+world\r\n"); + std::vector expected_nodes{ + {type::array, 2u, 0u, "" }, + {type::simple_string, 1u, 1u, "hello"}, + {type::simple_string, 1u, 1u, "world"}, + }; + check_nodes(t, expected_nodes); + BOOST_TEST_EQ(t.data_size(), 10u); + BOOST_TEST_EQ(t.data_capacity(), 512u); + BOOST_TEST_EQ(t.get_reallocs(), 1u); + BOOST_TEST_EQ(t.get_total_msgs(), 1u); + + // Capacity will have raised to 512 bytes, at least. Add some more without reallocations + add_nodes(t, "$3\r\nbye\r\n"); + expected_nodes.push_back({type::blob_string, 1u, 0u, "bye"}); + check_nodes(t, expected_nodes); + BOOST_TEST_EQ(t.data_size(), 13u); + BOOST_TEST_EQ(t.data_capacity(), 512u); + BOOST_TEST_EQ(t.get_reallocs(), 1u); + BOOST_TEST_EQ(t.get_total_msgs(), 2u); + + // Add nodes above the first reallocation threshold. Node strings are still valid + const std::string long_value(600u, 'a'); + add_nodes(t, "+" + long_value + "\r\n"); + expected_nodes.push_back({type::simple_string, 1u, 0u, long_value}); + check_nodes(t, expected_nodes); + BOOST_TEST_EQ(t.data_size(), 613u); + BOOST_TEST_EQ(t.data_capacity(), 1024u); + BOOST_TEST_EQ(t.get_reallocs(), 2u); + BOOST_TEST_EQ(t.get_total_msgs(), 3u); + + // Add some more nodes, still within the reallocation threshold + add_nodes(t, "+some_other_value\r\n"); + expected_nodes.push_back({type::simple_string, 1u, 0u, "some_other_value"}); + check_nodes(t, expected_nodes); + BOOST_TEST_EQ(t.data_size(), 629u); + BOOST_TEST_EQ(t.data_capacity(), 1024u); + BOOST_TEST_EQ(t.get_reallocs(), 2u); + BOOST_TEST_EQ(t.get_total_msgs(), 4u); + + // Add some more, causing another reallocation + add_nodes(t, "+" + long_value + "\r\n"); + expected_nodes.push_back({type::simple_string, 1u, 0u, long_value}); + check_nodes(t, expected_nodes); + BOOST_TEST_EQ(t.data_size(), 1229u); + BOOST_TEST_EQ(t.data_capacity(), 2048u); + BOOST_TEST_EQ(t.get_reallocs(), 3u); + BOOST_TEST_EQ(t.get_total_msgs(), 5u); +} + +// --- Move +// void test_move_ctor() +// { +// flat_tree t; + +// add_nodes(t, "$2\r\n+hello\r\n+world\r\n"); +// flat_tree t2{std::move(t)}; + +// check_nodes(t2, {}); +// BOOST_TEST_EQ(t2.data_size(), 0u); +// BOOST_TEST_EQ(t2.get_reallocs(), 0u); +// BOOST_TEST_EQ(t2.get_total_msgs(), 0u); +// } + // Parses the same data into a tree and a // flat_tree, they should be equal to each other. void test_views_are_set() @@ -177,6 +279,7 @@ void test_copy_assign() int main() { test_default_constructor(); + test_add_nodes(); test_views_are_set(); test_copy_assign(); From 58f5ed76a4391e425f78a326ff8697d52b288abb Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Fri, 21 Nov 2025 19:48:02 +0100 Subject: [PATCH 17/36] add nodes copies --- test/test_flat_tree.cpp | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/test/test_flat_tree.cpp b/test/test_flat_tree.cpp index b2e38fcb4..203e76684 100644 --- a/test/test_flat_tree.cpp +++ b/test/test_flat_tree.cpp @@ -15,8 +15,10 @@ #include "print_node.hpp" +#include #include #include +#include #include #include #include @@ -158,6 +160,29 @@ void test_add_nodes() BOOST_TEST_EQ(t.get_total_msgs(), 5u); } +// Strings are really copied into the object +void test_add_nodes_copies() +{ + flat_tree t; + + // Place the message in dynamic memory + constexpr std::string_view const_msg = "+some_long_value_for_a_node\r\n"; + std::unique_ptr data{new char[100]{}}; + std::copy(const_msg.begin(), const_msg.end(), data.get()); + + // Add nodes pointing into this message + add_nodes(t, data.get()); + + // Invalidate the original message + data.reset(); + + // Check + std::vector expected_nodes{ + {type::simple_string, 1u, 0u, "some_long_value_for_a_node"}, + }; + check_nodes(t, expected_nodes); +} + // --- Move // void test_move_ctor() // { @@ -278,8 +303,10 @@ void test_copy_assign() int main() { - test_default_constructor(); test_add_nodes(); + test_add_nodes_copies(); + + test_default_constructor(); test_views_are_set(); test_copy_assign(); From c72f42dded53d2e4a885861ceefcb7ede2dd2c5c Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Fri, 21 Nov 2025 19:55:20 +0100 Subject: [PATCH 18/36] Capacity increase --- test/test_flat_tree.cpp | 42 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/test/test_flat_tree.cpp b/test/test_flat_tree.cpp index 203e76684..a55ec3754 100644 --- a/test/test_flat_tree.cpp +++ b/test/test_flat_tree.cpp @@ -183,6 +183,47 @@ void test_add_nodes_copies() check_nodes(t, expected_nodes); } +// Reallocations happen only when we would exceed capacity +void test_add_nodes_capacity_limit() +{ + flat_tree t; + + // Add a node to reach capacity 512 + add_nodes(t, "+hello\r\n"); + BOOST_TEST_EQ(t.data_size(), 5u); + BOOST_TEST_EQ(t.data_capacity(), 512u); + + // Fill the rest of the capacity + add_nodes(t, "+" + std::string(507u, 'b') + "\r\n"); + BOOST_TEST_EQ(t.data_size(), 512u); + BOOST_TEST_EQ(t.data_capacity(), 512u); + + // Adding an empty node here doesn't change capacity + add_nodes(t, "_\r\n"); + BOOST_TEST_EQ(t.data_size(), 512u); + BOOST_TEST_EQ(t.data_capacity(), 512u); + + // Adding more data causes a reallocation + add_nodes(t, "+a\r\n"); + BOOST_TEST_EQ(t.data_size(), 513u); + BOOST_TEST_EQ(t.data_capacity(), 1024); + + // Same goes for the next capacity limit + add_nodes(t, "+" + std::string(511u, 'c') + "\r\n"); + BOOST_TEST_EQ(t.data_size(), 1024); + BOOST_TEST_EQ(t.data_capacity(), 1024); + + // Reallocation + add_nodes(t, "+u\r\n"); + BOOST_TEST_EQ(t.data_size(), 1025u); + BOOST_TEST_EQ(t.data_capacity(), 2048u); + + // This would continue + add_nodes(t, "+" + std::string(1024u, 'd') + "\r\n"); + BOOST_TEST_EQ(t.data_size(), 2049u); + BOOST_TEST_EQ(t.data_capacity(), 4096u); +} + // --- Move // void test_move_ctor() // { @@ -305,6 +346,7 @@ int main() { test_add_nodes(); test_add_nodes_copies(); + test_add_nodes_capacity_limit(); test_default_constructor(); From e0c3753c7ce7531a5202e22be488d76e2eea1f6e Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Fri, 21 Nov 2025 20:10:09 +0100 Subject: [PATCH 19/36] Make reallocations powers of 2 --- include/boost/redis/impl/flat_tree.ipp | 14 +++++++++++--- test/test_flat_tree.cpp | 19 +++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) diff --git a/include/boost/redis/impl/flat_tree.ipp b/include/boost/redis/impl/flat_tree.ipp index 378741c09..80820e565 100644 --- a/include/boost/redis/impl/flat_tree.ipp +++ b/include/boost/redis/impl/flat_tree.ipp @@ -65,6 +65,16 @@ inline void copy_assign(flat_buffer& buff, const flat_buffer& other) buff.reallocs = other.reallocs; } +// Compute the new capacity upon reallocation. We always use powers of 2, +// starting in 512, to prevent many small allocations +inline std::size_t compute_capacity(std::size_t current, std::size_t requested) +{ + std::size_t res = (std::max)(current, static_cast(512u)); + while (res < requested) + res *= 2u; + return res; +} + // Grows the buffer until reaching a target size. // Might rebase the strings in nodes inline void grow(flat_buffer& buff, std::size_t new_capacity, view_tree& nodes) @@ -73,9 +83,7 @@ inline void grow(flat_buffer& buff, std::size_t new_capacity, view_tree& nodes) return; // Compute the actual capacity that we will be using - // Prevent many small allocations when starting from an empty buffer - new_capacity = (std::max)(new_capacity, static_cast(512u)); - new_capacity = (std::max)(new_capacity, 2 * buff.capacity); + new_capacity = compute_capacity(buff.capacity, new_capacity); // Allocate space std::unique_ptr new_buffer{new char[new_capacity]}; diff --git a/test/test_flat_tree.cpp b/test/test_flat_tree.cpp index a55ec3754..6be8705c8 100644 --- a/test/test_flat_tree.cpp +++ b/test/test_flat_tree.cpp @@ -224,6 +224,24 @@ void test_add_nodes_capacity_limit() BOOST_TEST_EQ(t.data_capacity(), 4096u); } +// It's no problem if a node is big enough to surpass several reallocation limits +void test_add_nodes_big_node() +{ + flat_tree t; + + // Add a bunch of nodes. Single allocation. Some nodes are empty. + const std::string long_value(1500u, 'h'); + add_nodes(t, "+" + long_value + "\r\n"); + std::vector expected_nodes{ + {type::simple_string, 1u, 0u, long_value}, + }; + check_nodes(t, expected_nodes); + BOOST_TEST_EQ(t.data_size(), 1500u); + BOOST_TEST_EQ(t.data_capacity(), 2048u); + BOOST_TEST_EQ(t.get_reallocs(), 1u); + BOOST_TEST_EQ(t.get_total_msgs(), 1u); +} + // --- Move // void test_move_ctor() // { @@ -347,6 +365,7 @@ int main() test_add_nodes(); test_add_nodes_copies(); test_add_nodes_capacity_limit(); + test_add_nodes_big_node(); test_default_constructor(); From 5d727ae689243cb41ccd4878f28a03acdba02da3 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Fri, 21 Nov 2025 20:20:00 +0100 Subject: [PATCH 20/36] Reserve tests --- test/test_flat_tree.cpp | 109 ++++++++++++++++++++++++++++++++++++---- 1 file changed, 99 insertions(+), 10 deletions(-) diff --git a/test/test_flat_tree.cpp b/test/test_flat_tree.cpp index 6be8705c8..c157386be 100644 --- a/test/test_flat_tree.cpp +++ b/test/test_flat_tree.cpp @@ -93,16 +93,7 @@ void check_nodes( std::cerr << "Called from " << loc << std::endl; } -void test_default_constructor() -{ - flat_tree t; - - check_nodes(t, {}); - BOOST_TEST_EQ(t.data_size(), 0u); - BOOST_TEST_EQ(t.get_reallocs(), 0u); - BOOST_TEST_EQ(t.get_total_msgs(), 0u); -} - +// --- Adding nodes --- // Adding nodes works, even when reallocations happen. // Empty nodes don't cause trouble void test_add_nodes() @@ -242,6 +233,99 @@ void test_add_nodes_big_node() BOOST_TEST_EQ(t.get_total_msgs(), 1u); } +// --- Reserving space --- +// The usual case, calling it before using it +void test_reserve() +{ + flat_tree t; + + t.reserve(1024u, 5u); + check_nodes(t, {}); + BOOST_TEST_EQ(t.get_view().capacity(), 5u); + BOOST_TEST_EQ(t.data_size(), 0u); + BOOST_TEST_EQ(t.data_capacity(), 1024); + BOOST_TEST_EQ(t.get_reallocs(), 1u); + BOOST_TEST_EQ(t.get_total_msgs(), 0u); + + // Adding some nodes now works + add_nodes(t, "+hello\r\n"); + std::vector expected_nodes{ + {type::simple_string, 1u, 0u, "hello"}, + }; + check_nodes(t, expected_nodes); +} + +// Reserving space uses the same allocation thresholds +void test_reserve_not_power_of_2() +{ + flat_tree t; + + // First threshold at 512 + t.reserve(200u, 5u); + BOOST_TEST_EQ(t.data_capacity(), 512u); + BOOST_TEST_EQ(t.get_reallocs(), 1u); + + // Second threshold at 1024 + t.reserve(600u, 5u); + BOOST_TEST_EQ(t.data_capacity(), 1024u); + BOOST_TEST_EQ(t.get_reallocs(), 2u); +} + +// Requesting a capacity below the current one does nothing +void test_reserve_below_current_capacity() +{ + flat_tree t; + + // Reserving with a zero capacity does nothing + t.reserve(0u, 0u); + BOOST_TEST_EQ(t.data_capacity(), 0u); + BOOST_TEST_EQ(t.get_reallocs(), 0u); + + // Increase capacity + t.reserve(400u, 5u); + BOOST_TEST_EQ(t.data_capacity(), 512u); + BOOST_TEST_EQ(t.get_reallocs(), 1u); + + // Reserving again does nothing + t.reserve(400u, 5u); + t.reserve(512u, 5u); + t.reserve(0u, 5u); + BOOST_TEST_EQ(t.data_capacity(), 512u); + BOOST_TEST_EQ(t.get_reallocs(), 1u); +} + +// Reserving might reallocate. If there are nodes, strings remain valid +void test_reserve_with_data() +{ + flat_tree t; + + // Add a bunch of nodes, and then reserve + add_nodes(t, "*2\r\n+hello\r\n+world\r\n"); + t.reserve(1000u, 10u); + + // Check + std::vector expected_nodes{ + {type::array, 2u, 0u, "" }, + {type::simple_string, 1u, 1u, "hello"}, + {type::simple_string, 1u, 1u, "world"}, + }; + check_nodes(t, expected_nodes); + BOOST_TEST_EQ(t.data_size(), 10u); + BOOST_TEST_EQ(t.data_capacity(), 1024u); + BOOST_TEST_EQ(t.get_reallocs(), 2u); + BOOST_TEST_EQ(t.get_total_msgs(), 1u); +} + +void test_default_constructor() +{ + flat_tree t; + + check_nodes(t, {}); + BOOST_TEST_EQ(t.data_size(), 0u); + BOOST_TEST_EQ(t.get_reallocs(), 0u); + BOOST_TEST_EQ(t.get_total_msgs(), 0u); +} + // --- Move // void test_move_ctor() // { @@ -367,6 +451,11 @@ int main() test_add_nodes_capacity_limit(); test_add_nodes_big_node(); + test_reserve(); + test_reserve_not_power_of_2(); + test_reserve_below_current_capacity(); + test_reserve_with_data(); + test_default_constructor(); test_views_are_set(); From 1a1452f0fe16e1884405f291beac43f80699c6b7 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Fri, 21 Nov 2025 20:23:14 +0100 Subject: [PATCH 21/36] clear tests --- test/test_flat_tree.cpp | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) diff --git a/test/test_flat_tree.cpp b/test/test_flat_tree.cpp index c157386be..ad9ab70a7 100644 --- a/test/test_flat_tree.cpp +++ b/test/test_flat_tree.cpp @@ -326,6 +326,36 @@ void test_default_constructor() BOOST_TEST_EQ(t.get_total_msgs(), 0u); } +// --- Clear --- +void test_clear() +{ + flat_tree t; + + // Add a bunch of nodes, then clear + add_nodes(t, "*2\r\n+hello\r\n+world\r\n"); + t.clear(); + + // Nodes are no longer there, but memory hasn't been fred + check_nodes(t, {}); + BOOST_TEST_EQ(t.data_size(), 0u); + BOOST_TEST_EQ(t.data_capacity(), 512u); + BOOST_TEST_EQ(t.get_reallocs(), 1u); + BOOST_TEST_EQ(t.get_total_msgs(), 0u); +} + +// Clearing an empty tree doesn't cause trouble +void test_clear_empty() +{ + flat_tree t; + t.clear(); + + check_nodes(t, {}); + BOOST_TEST_EQ(t.data_size(), 0u); + BOOST_TEST_EQ(t.data_capacity(), 0u); + BOOST_TEST_EQ(t.get_reallocs(), 0u); + BOOST_TEST_EQ(t.get_total_msgs(), 0u); +} + // --- Move // void test_move_ctor() // { @@ -456,6 +486,9 @@ int main() test_reserve_below_current_capacity(); test_reserve_with_data(); + test_clear(); + test_clear_empty(); + test_default_constructor(); test_views_are_set(); From 9e19635ff1338acb80665d5bebfc98beffebf48e Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Tue, 25 Nov 2025 11:08:18 +0100 Subject: [PATCH 22/36] Copy ctor tests --- include/boost/redis/impl/flat_tree.ipp | 30 ++++++----- test/test_flat_tree.cpp | 75 ++++++++++++++++++++++---- 2 files changed, 82 insertions(+), 23 deletions(-) diff --git a/include/boost/redis/impl/flat_tree.ipp b/include/boost/redis/impl/flat_tree.ipp index 80820e565..d6457b287 100644 --- a/include/boost/redis/impl/flat_tree.ipp +++ b/include/boost/redis/impl/flat_tree.ipp @@ -35,13 +35,26 @@ inline void rebase_strings(view_tree& nodes, const char* old_base, const char* n // --- Operations in flat_buffer --- +// Compute the new capacity upon reallocation. We always use powers of 2, +// starting in 512, to prevent many small allocations +inline std::size_t compute_capacity(std::size_t current, std::size_t requested) +{ + std::size_t res = (std::max)(current, static_cast(512u)); + while (res < requested) + res *= 2u; + return res; +} + // Copy construction inline flat_buffer copy_construct(const flat_buffer& other) { - flat_buffer res{{}, other.size, other.capacity, other.reallocs}; + flat_buffer res{{}, other.size, 0u, 0u}; - if (other.capacity > 0u) { - res.data.reset(new char[other.capacity]); + if (other.size > 0u) { + const std::size_t capacity = compute_capacity(0u, other.size); + res.data.reset(new char[capacity]); + res.capacity = capacity; + res.reallocs = 1u; std::copy(other.data.get(), other.data.get() + other.size, res.data.get()); } @@ -55,6 +68,7 @@ inline void copy_assign(flat_buffer& buff, const flat_buffer& other) if (buff.capacity < other.capacity) { buff.data.reset(new char[other.capacity]); buff.capacity = other.capacity; + ++buff.reallocs; } // Copy the contents @@ -65,16 +79,6 @@ inline void copy_assign(flat_buffer& buff, const flat_buffer& other) buff.reallocs = other.reallocs; } -// Compute the new capacity upon reallocation. We always use powers of 2, -// starting in 512, to prevent many small allocations -inline std::size_t compute_capacity(std::size_t current, std::size_t requested) -{ - std::size_t res = (std::max)(current, static_cast(512u)); - while (res < requested) - res *= 2u; - return res; -} - // Grows the buffer until reaching a target size. // Might rebase the strings in nodes inline void grow(flat_buffer& buff, std::size_t new_capacity, view_tree& nodes) diff --git a/test/test_flat_tree.cpp b/test/test_flat_tree.cpp index ad9ab70a7..eae03a090 100644 --- a/test/test_flat_tree.cpp +++ b/test/test_flat_tree.cpp @@ -316,16 +316,6 @@ void test_reserve_with_data() BOOST_TEST_EQ(t.get_total_msgs(), 1u); } -void test_default_constructor() -{ - flat_tree t; - - check_nodes(t, {}); - BOOST_TEST_EQ(t.data_size(), 0u); - BOOST_TEST_EQ(t.get_reallocs(), 0u); - BOOST_TEST_EQ(t.get_total_msgs(), 0u); -} - // --- Clear --- void test_clear() { @@ -356,6 +346,67 @@ void test_clear_empty() BOOST_TEST_EQ(t.get_total_msgs(), 0u); } +// --- Default ctor --- +void test_default_constructor() +{ + flat_tree t; + + check_nodes(t, {}); + BOOST_TEST_EQ(t.data_size(), 0u); + BOOST_TEST_EQ(t.get_reallocs(), 0u); + BOOST_TEST_EQ(t.get_total_msgs(), 0u); +} + +// --- Copy ctor --- +void test_copy_ctor() +{ + // Setup + auto t = std::make_unique(); + add_nodes(*t, "*2\r\n+hello\r\n+world\r\n"); + std::vector expected_nodes{ + {type::array, 2u, 0u, "" }, + {type::simple_string, 1u, 1u, "hello"}, + {type::simple_string, 1u, 1u, "world"}, + }; + + // Construct, then destroy the original copy + flat_tree t2{*t}; + t.reset(); + + // Check + check_nodes(t2, expected_nodes); + BOOST_TEST_EQ(t2.data_size(), 10u); + BOOST_TEST_EQ(t2.data_capacity(), 512u); + BOOST_TEST_EQ(t2.get_reallocs(), 1u); + BOOST_TEST_EQ(t2.get_total_msgs(), 1u); +} + +// Copying an empty tree doesn't cause problems +void test_copy_ctor_empty() +{ + flat_tree t; + flat_tree t2{t}; + check_nodes(t2, {}); + BOOST_TEST_EQ(t2.data_size(), 0u); + BOOST_TEST_EQ(t2.data_capacity(), 0u); + BOOST_TEST_EQ(t2.get_reallocs(), 0u); + BOOST_TEST_EQ(t2.get_total_msgs(), 0u); +} + +// Copying an object that has no elements but some capacity doesn't cause trouble +void test_copy_ctor_empty_with_capacity() +{ + flat_tree t; + t.reserve(300u, 8u); + + flat_tree t2{t}; + check_nodes(t2, {}); + BOOST_TEST_EQ(t2.data_size(), 0u); + BOOST_TEST_EQ(t2.data_capacity(), 0u); + BOOST_TEST_EQ(t2.get_reallocs(), 0u); + BOOST_TEST_EQ(t2.get_total_msgs(), 0u); +} + // --- Move // void test_move_ctor() // { @@ -491,6 +542,10 @@ int main() test_default_constructor(); + test_copy_ctor(); + test_copy_ctor_empty(); + test_copy_ctor_empty_with_capacity(); + test_views_are_set(); test_copy_assign(); test_reuse(); From f83a4e36d59bb07f3ec09bc56a11c3475a267d3e Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Tue, 25 Nov 2025 11:12:46 +0100 Subject: [PATCH 23/36] More copy tests --- test/test_flat_tree.cpp | 28 ++++++++++++++++++++++++++++ 1 file changed, 28 insertions(+) diff --git a/test/test_flat_tree.cpp b/test/test_flat_tree.cpp index eae03a090..75eaed0f3 100644 --- a/test/test_flat_tree.cpp +++ b/test/test_flat_tree.cpp @@ -407,6 +407,33 @@ void test_copy_ctor_empty_with_capacity() BOOST_TEST_EQ(t2.get_total_msgs(), 0u); } +// Copying an object with more capacity than required adjusts its capacity +void test_copy_ctor_adjust_capacity() +{ + // Setup + flat_tree t; + add_nodes(t, "+hello\r\n"); + std::vector expected_nodes{ + {type::simple_string, 1u, 0u, "hello"}, + }; + + // Cause reallocations + t.reserve(1000u, 10u); + t.reserve(2000u, 10u); + t.reserve(4000u, 10u); + + // Copy + flat_tree t2{t}; + + // The target object has the minimum required capacity, + // and the number of reallocs has been reset + check_nodes(t2, expected_nodes); + BOOST_TEST_EQ(t2.data_size(), 5u); + BOOST_TEST_EQ(t2.data_capacity(), 512u); + BOOST_TEST_EQ(t2.get_reallocs(), 1u); + BOOST_TEST_EQ(t2.get_total_msgs(), 1u); +} + // --- Move // void test_move_ctor() // { @@ -545,6 +572,7 @@ int main() test_copy_ctor(); test_copy_ctor_empty(); test_copy_ctor_empty_with_capacity(); + test_copy_ctor_adjust_capacity(); test_views_are_set(); test_copy_assign(); From 8f7e95da29c94b03eeeb8b062a25560c29a3c067 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Tue, 25 Nov 2025 11:17:17 +0100 Subject: [PATCH 24/36] Move ctor tests --- test/test_flat_tree.cpp | 61 +++++++++++++++++++++++++++++++++-------- 1 file changed, 50 insertions(+), 11 deletions(-) diff --git a/test/test_flat_tree.cpp b/test/test_flat_tree.cpp index 75eaed0f3..471770935 100644 --- a/test/test_flat_tree.cpp +++ b/test/test_flat_tree.cpp @@ -434,19 +434,54 @@ void test_copy_ctor_adjust_capacity() BOOST_TEST_EQ(t2.get_total_msgs(), 1u); } -// --- Move -// void test_move_ctor() -// { -// flat_tree t; +// --- Move ctor --- +void test_move_ctor() +{ + flat_tree t; + add_nodes(t, "*2\r\n+hello\r\n+world\r\n"); -// add_nodes(t, "$2\r\n+hello\r\n+world\r\n"); -// flat_tree t2{std::move(t)}; + flat_tree t2{std::move(t)}; -// check_nodes(t2, {}); -// BOOST_TEST_EQ(t2.data_size(), 0u); -// BOOST_TEST_EQ(t2.get_reallocs(), 0u); -// BOOST_TEST_EQ(t2.get_total_msgs(), 0u); -// } + std::vector expected_nodes{ + {type::array, 2u, 0u, "" }, + {type::simple_string, 1u, 1u, "hello"}, + {type::simple_string, 1u, 1u, "world"}, + }; + check_nodes(t2, expected_nodes); + BOOST_TEST_EQ(t2.data_size(), 10u); + BOOST_TEST_EQ(t2.data_capacity(), 512u); + BOOST_TEST_EQ(t2.get_reallocs(), 1u); + BOOST_TEST_EQ(t2.get_total_msgs(), 1u); +} + +// Moving an empty object doesn't cause trouble +void test_move_ctor_empty() +{ + flat_tree t; + + flat_tree t2{std::move(t)}; + + check_nodes(t2, {}); + BOOST_TEST_EQ(t2.data_size(), 0u); + BOOST_TEST_EQ(t2.data_capacity(), 0u); + BOOST_TEST_EQ(t2.get_reallocs(), 0u); + BOOST_TEST_EQ(t2.get_total_msgs(), 0u); +} + +// Moving an object with capacity but no data doesn't cause trouble +void test_move_ctor_with_capacity() +{ + flat_tree t; + t.reserve(1000u, 10u); + + flat_tree t2{std::move(t)}; + + check_nodes(t2, {}); + BOOST_TEST_EQ(t2.data_size(), 0u); + BOOST_TEST_EQ(t2.data_capacity(), 1024u); + BOOST_TEST_EQ(t2.get_reallocs(), 1u); + BOOST_TEST_EQ(t2.get_total_msgs(), 0u); +} // Parses the same data into a tree and a // flat_tree, they should be equal to each other. @@ -574,6 +609,10 @@ int main() test_copy_ctor_empty_with_capacity(); test_copy_ctor_adjust_capacity(); + test_move_ctor(); + test_move_ctor_empty(); + test_move_ctor_with_capacity(); + test_views_are_set(); test_copy_assign(); test_reuse(); From 5e3ec9fcfe3ef7b4eb1a33fae9daffbea340afbb Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Tue, 25 Nov 2025 11:28:33 +0100 Subject: [PATCH 25/36] copy assign --- include/boost/redis/impl/flat_tree.ipp | 3 - test/test_flat_tree.cpp | 94 +++++++++++++++++++++++++- 2 files changed, 92 insertions(+), 5 deletions(-) diff --git a/include/boost/redis/impl/flat_tree.ipp b/include/boost/redis/impl/flat_tree.ipp index d6457b287..8354534ba 100644 --- a/include/boost/redis/impl/flat_tree.ipp +++ b/include/boost/redis/impl/flat_tree.ipp @@ -73,10 +73,7 @@ inline void copy_assign(flat_buffer& buff, const flat_buffer& other) // Copy the contents std::copy(other.data.get(), other.data.get() + other.size, buff.data.get()); - - // Copy the other fields buff.size = other.size; - buff.reallocs = other.reallocs; } // Grows the buffer until reaching a target size. diff --git a/test/test_flat_tree.cpp b/test/test_flat_tree.cpp index 471770935..dcc896a8e 100644 --- a/test/test_flat_tree.cpp +++ b/test/test_flat_tree.cpp @@ -483,6 +483,91 @@ void test_move_ctor_with_capacity() BOOST_TEST_EQ(t2.get_total_msgs(), 0u); } +// --- Copy assignment --- +void test_copy_assign() +{ + flat_tree t; + add_nodes(t, "+some_data\r\n"); + + auto t2 = std::make_unique(); + add_nodes(*t2, "*2\r\n+hello\r\n+world\r\n"); + + t = *t2; + + // Delete the source object, to check that we copied the contents + t2.reset(); + + std::vector expected_nodes{ + {type::array, 2u, 0u, "" }, + {type::simple_string, 1u, 1u, "hello"}, + {type::simple_string, 1u, 1u, "world"}, + }; + check_nodes(t, expected_nodes); + BOOST_TEST_EQ(t.data_size(), 10u); + BOOST_TEST_EQ(t.data_capacity(), 512u); + BOOST_TEST_EQ(t.get_reallocs(), 1u); + BOOST_TEST_EQ(t.get_total_msgs(), 1u); +} + +// The lhs is empty and doesn't have any capacity +void test_copy_assign_target_empty() +{ + flat_tree t; + + flat_tree t2; + add_nodes(t2, "+hello\r\n"); + + t = t2; + + std::vector expected_nodes{ + {type::simple_string, 1u, 0u, "hello"}, + }; + check_nodes(t, expected_nodes); + BOOST_TEST_EQ(t.data_size(), 5u); + BOOST_TEST_EQ(t.data_capacity(), 512u); + BOOST_TEST_EQ(t.get_reallocs(), 1u); + BOOST_TEST_EQ(t.get_total_msgs(), 1u); +} + +// If the target doesn't have enough capacity, a reallocation happens +void test_copy_assign_target_not_enough_capacity() +{ + flat_tree t; + add_nodes(t, "+hello\r\n"); + + const std::string big_node(2000u, 'a'); + flat_tree t2; + add_nodes(t2, "+" + big_node + "\r\n"); + + t = t2; + + std::vector expected_nodes{ + {type::simple_string, 1u, 0u, big_node}, + }; + check_nodes(t, expected_nodes); + BOOST_TEST_EQ(t.data_size(), 2000u); + BOOST_TEST_EQ(t.data_capacity(), 2048u); + BOOST_TEST_EQ(t.get_reallocs(), 2u); // initial + assignment + BOOST_TEST_EQ(t.get_total_msgs(), 1u); +} + +// If the source of the assignment is empty, nothing bad happens +void test_copy_assign_source_empty() +{ + flat_tree t; + add_nodes(t, "+hello\r\n"); + + flat_tree t2; + + t = t2; + + check_nodes(t, {}); + BOOST_TEST_EQ(t.data_size(), 0u); + BOOST_TEST_EQ(t.data_capacity(), 512u); // capacity is kept + BOOST_TEST_EQ(t.get_reallocs(), 1u); + BOOST_TEST_EQ(t.get_total_msgs(), 0u); +} + // Parses the same data into a tree and a // flat_tree, they should be equal to each other. void test_views_are_set() @@ -542,7 +627,7 @@ void test_reuse() BOOST_TEST_ALL_EQ(resp1.begin(), resp1.end(), tmp.get_view().begin(), tmp.get_view().end()); } -void test_copy_assign() +void test_copy_assign_2() { flat_tree ref1; flat_tree ref2; @@ -613,8 +698,13 @@ int main() test_move_ctor_empty(); test_move_ctor_with_capacity(); - test_views_are_set(); test_copy_assign(); + test_copy_assign_target_empty(); + test_copy_assign_target_not_enough_capacity(); + test_copy_assign_source_empty(); + + test_views_are_set(); + test_copy_assign_2(); test_reuse(); return boost::report_errors(); From 38f49eb43d3dbe27bad52bb66af840ef5d131be3 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Tue, 25 Nov 2025 11:34:06 +0100 Subject: [PATCH 26/36] copy assign corrections --- include/boost/redis/impl/flat_tree.ipp | 7 +++-- test/test_flat_tree.cpp | 43 ++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/include/boost/redis/impl/flat_tree.ipp b/include/boost/redis/impl/flat_tree.ipp index 8354534ba..6709a2bad 100644 --- a/include/boost/redis/impl/flat_tree.ipp +++ b/include/boost/redis/impl/flat_tree.ipp @@ -65,9 +65,10 @@ inline flat_buffer copy_construct(const flat_buffer& other) inline void copy_assign(flat_buffer& buff, const flat_buffer& other) { // Make space if required - if (buff.capacity < other.capacity) { - buff.data.reset(new char[other.capacity]); - buff.capacity = other.capacity; + if (buff.capacity < other.size) { + const std::size_t capacity = compute_capacity(buff.capacity, other.size); + buff.data.reset(new char[capacity]); + buff.capacity = capacity; ++buff.reallocs; } diff --git a/test/test_flat_tree.cpp b/test/test_flat_tree.cpp index dcc896a8e..73204cad3 100644 --- a/test/test_flat_tree.cpp +++ b/test/test_flat_tree.cpp @@ -568,6 +568,47 @@ void test_copy_assign_source_empty() BOOST_TEST_EQ(t.get_total_msgs(), 0u); } +// If the source of the assignment has capacity but no data, we're OK +void test_copy_assign_source_with_capacity() +{ + flat_tree t; + add_nodes(t, "+hello\r\n"); + + flat_tree t2; + t2.reserve(1000u, 4u); + t2.reserve(4000u, 8u); + + t = t2; + + check_nodes(t, {}); + BOOST_TEST_EQ(t.data_size(), 0u); + BOOST_TEST_EQ(t.data_capacity(), 512u); // capacity is kept + BOOST_TEST_EQ(t.get_reallocs(), 1u); // not propagated + BOOST_TEST_EQ(t.get_total_msgs(), 0u); +} + +// If the source of the assignment has data with extra capacity +// and a reallocation is needed, the minimum amount of space is allocated +void test_copy_assign_source_with_extra_capacity() +{ + flat_tree t; + + flat_tree t2; + add_nodes(t2, "+hello\r\n"); + t2.reserve(4000u, 8u); + + t = t2; + + std::vector expected_nodes{ + {type::simple_string, 1u, 0u, "hello"}, + }; + check_nodes(t, expected_nodes); + BOOST_TEST_EQ(t.data_size(), 5u); + BOOST_TEST_EQ(t.data_capacity(), 512u); + BOOST_TEST_EQ(t.get_reallocs(), 1u); + BOOST_TEST_EQ(t.get_total_msgs(), 1u); +} + // Parses the same data into a tree and a // flat_tree, they should be equal to each other. void test_views_are_set() @@ -702,6 +743,8 @@ int main() test_copy_assign_target_empty(); test_copy_assign_target_not_enough_capacity(); test_copy_assign_source_empty(); + test_copy_assign_source_with_capacity(); + test_copy_assign_source_with_extra_capacity(); test_views_are_set(); test_copy_assign_2(); From 4df05ee8e5002868ed721d46c4f8654d2996bca7 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Tue, 25 Nov 2025 11:37:04 +0100 Subject: [PATCH 27/36] final copy assign tests --- test/test_flat_tree.cpp | 35 +++++++++++++++++++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/test/test_flat_tree.cpp b/test/test_flat_tree.cpp index 73204cad3..d35ed2b6d 100644 --- a/test/test_flat_tree.cpp +++ b/test/test_flat_tree.cpp @@ -609,6 +609,39 @@ void test_copy_assign_source_with_extra_capacity() BOOST_TEST_EQ(t.get_total_msgs(), 1u); } +void test_copy_assign_both_empty() +{ + flat_tree t; + flat_tree t2; + + t = t2; + + check_nodes(t, {}); + BOOST_TEST_EQ(t.data_size(), 0u); + BOOST_TEST_EQ(t.data_capacity(), 0u); + BOOST_TEST_EQ(t.get_reallocs(), 0u); + BOOST_TEST_EQ(t.get_total_msgs(), 0u); +} + +// Self-assignment doesn't cause trouble +void test_copy_assign_self() +{ + flat_tree t; + add_nodes(t, "+hello\r\n"); + + const auto& tref = t; + t = tref; + + std::vector expected_nodes{ + {type::simple_string, 1u, 0u, "hello"}, + }; + check_nodes(t, expected_nodes); + BOOST_TEST_EQ(t.data_size(), 5u); + BOOST_TEST_EQ(t.data_capacity(), 512u); + BOOST_TEST_EQ(t.get_reallocs(), 1u); + BOOST_TEST_EQ(t.get_total_msgs(), 1u); +} + // Parses the same data into a tree and a // flat_tree, they should be equal to each other. void test_views_are_set() @@ -745,6 +778,8 @@ int main() test_copy_assign_source_empty(); test_copy_assign_source_with_capacity(); test_copy_assign_source_with_extra_capacity(); + test_copy_assign_both_empty(); + test_copy_assign_self(); test_views_are_set(); test_copy_assign_2(); From 101f60e1a0fa9703ca67262b5973df09bb331147 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Tue, 25 Nov 2025 12:53:31 +0100 Subject: [PATCH 28/36] comparison tests --- test/test_flat_tree.cpp | 194 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 194 insertions(+) diff --git a/test/test_flat_tree.cpp b/test/test_flat_tree.cpp index d35ed2b6d..67c666a48 100644 --- a/test/test_flat_tree.cpp +++ b/test/test_flat_tree.cpp @@ -642,6 +642,187 @@ void test_copy_assign_self() BOOST_TEST_EQ(t.get_total_msgs(), 1u); } +// --- Move assignment --- +void test_move_assign() +{ + flat_tree t; + add_nodes(t, "+some_data\r\n"); + + flat_tree t2; + add_nodes(t2, "*2\r\n+hello\r\n+world\r\n"); + + t = std::move(t2); + + std::vector expected_nodes{ + {type::array, 2u, 0u, "" }, + {type::simple_string, 1u, 1u, "hello"}, + {type::simple_string, 1u, 1u, "world"}, + }; + check_nodes(t, expected_nodes); + BOOST_TEST_EQ(t.data_size(), 10u); + BOOST_TEST_EQ(t.data_capacity(), 512u); + BOOST_TEST_EQ(t.get_reallocs(), 1u); + BOOST_TEST_EQ(t.get_total_msgs(), 1u); +} + +// The lhs is empty and doesn't have any capacity +void test_move_assign_target_empty() +{ + flat_tree t; + + flat_tree t2; + add_nodes(t2, "+hello\r\n"); + + t = std::move(t2); + + std::vector expected_nodes{ + {type::simple_string, 1u, 0u, "hello"}, + }; + check_nodes(t, expected_nodes); + BOOST_TEST_EQ(t.data_size(), 5u); + BOOST_TEST_EQ(t.data_capacity(), 512u); + BOOST_TEST_EQ(t.get_reallocs(), 1u); + BOOST_TEST_EQ(t.get_total_msgs(), 1u); +} + +// If the source of the assignment is empty, nothing bad happens +void test_move_assign_source_empty() +{ + flat_tree t; + add_nodes(t, "+hello\r\n"); + + flat_tree t2; + + t = std::move(t2); + + check_nodes(t, {}); + BOOST_TEST_EQ(t.data_size(), 0u); + BOOST_TEST_EQ(t.data_capacity(), 0u); + BOOST_TEST_EQ(t.get_reallocs(), 0u); + BOOST_TEST_EQ(t.get_total_msgs(), 0u); +} + +// If both source and target are empty, nothing bad happens +void test_move_assign_both_empty() +{ + flat_tree t; + + flat_tree t2; + + t = std::move(t2); + + check_nodes(t, {}); + BOOST_TEST_EQ(t.data_size(), 0u); + BOOST_TEST_EQ(t.data_capacity(), 0u); + BOOST_TEST_EQ(t.get_reallocs(), 0u); + BOOST_TEST_EQ(t.get_total_msgs(), 0u); +} + +// --- Comparison --- +void test_comparison_different() +{ + flat_tree t; + add_nodes(t, "+some_data\r\n"); + + flat_tree t2; + add_nodes(t2, "*2\r\n+hello\r\n+world\r\n"); + + BOOST_TEST_NOT(t == t2); + BOOST_TEST(t != t2); + BOOST_TEST_NOT(t2 == t); + BOOST_TEST(t2 != t); +} + +// The only difference is node types +void test_comparison_different_node_types() +{ + flat_tree t; + add_nodes(t, "+hello\r\n"); + + flat_tree t2; + add_nodes(t2, "$5\r\nhello\r\n"); + + BOOST_TEST_NOT(t == t2); + BOOST_TEST(t != t2); +} + +void test_comparison_equal() +{ + flat_tree t; + add_nodes(t, "+some_data\r\n"); + + flat_tree t2; + add_nodes(t2, "+some_data\r\n"); + + BOOST_TEST(t == t2); + BOOST_TEST_NOT(t != t2); +} + +// Allocations are not taken into account when comparing +void test_comparison_equal_reallocations() +{ + const std::string big_node(2000u, 'a'); + flat_tree t; + t.reserve(100u, 5u); + add_nodes(t, "+" + big_node + "\r\n"); + BOOST_TEST_EQ(t.get_reallocs(), 2u); + + flat_tree t2; + t2.reserve(2048u, 5u); + add_nodes(t2, "+" + big_node + "\r\n"); + BOOST_TEST_EQ(t2.get_reallocs(), 1u); + + BOOST_TEST(t == t2); + BOOST_TEST_NOT(t != t2); +} + +// Capacity is not taken into account when comparing +void test_comparison_equal_capacity() +{ + flat_tree t; + add_nodes(t, "+hello\r\n"); + + flat_tree t2; + t2.reserve(2048u, 5u); + add_nodes(t2, "+hello\r\n"); + + BOOST_TEST(t == t2); + BOOST_TEST_NOT(t != t2); +} + +// Empty containers don't cause trouble +void test_comparison_empty() +{ + flat_tree t; + add_nodes(t, "$5\r\nhello\r\n"); + + flat_tree tempty, tempty2; + + BOOST_TEST_NOT(t == tempty); + BOOST_TEST(t != tempty); + + BOOST_TEST_NOT(tempty == t); + BOOST_TEST(tempty != t); + + BOOST_TEST(tempty == tempty2); + BOOST_TEST_NOT(tempty != tempty2); +} + +// Self comparisons don't cause trouble +void test_comparison_self() +{ + flat_tree t; + add_nodes(t, "$5\r\nhello\r\n"); + + flat_tree tempty; + + BOOST_TEST(t == t); + BOOST_TEST_NOT(t != t); + + BOOST_TEST(tempty == tempty); + BOOST_TEST_NOT(tempty != tempty); +} + // Parses the same data into a tree and a // flat_tree, they should be equal to each other. void test_views_are_set() @@ -772,6 +953,11 @@ int main() test_move_ctor_empty(); test_move_ctor_with_capacity(); + test_move_assign(); + test_move_assign_target_empty(); + test_move_assign_source_empty(); + test_move_assign_both_empty(); + test_copy_assign(); test_copy_assign_target_empty(); test_copy_assign_target_not_enough_capacity(); @@ -781,6 +967,14 @@ int main() test_copy_assign_both_empty(); test_copy_assign_self(); + test_comparison_different(); + test_comparison_different_node_types(); + test_comparison_equal(); + test_comparison_equal_reallocations(); + test_comparison_equal_capacity(); + test_comparison_empty(); + test_comparison_self(); + test_views_are_set(); test_copy_assign_2(); test_reuse(); From 82a3f8b810a6f9a2040f566d53262d2e2904906d Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Tue, 25 Nov 2025 12:55:45 +0100 Subject: [PATCH 29/36] cleanup --- test/test_flat_tree.cpp | 105 +--------------------------------------- 1 file changed, 1 insertion(+), 104 deletions(-) diff --git a/test/test_flat_tree.cpp b/test/test_flat_tree.cpp index 67c666a48..43e2281d6 100644 --- a/test/test_flat_tree.cpp +++ b/test/test_flat_tree.cpp @@ -41,34 +41,6 @@ namespace { constexpr std::string_view resp3_set = "~6\r\n+orange\r\n+apple\r\n+one\r\n+two\r\n+three\r\n+orange\r\n"; -node from_node_view(node_view const& v) -{ - node ret; - ret.data_type = v.data_type; - ret.aggregate_size = v.aggregate_size; - ret.depth = v.depth; - ret.value = v.value; - return ret; -} - -tree from_flat(generic_flat_response const& resp) -{ - tree ret; - for (auto const& e : resp.value().get_view()) - ret.push_back(from_node_view(e)); - - return ret; -} - -tree from_flat(flat_tree const& resp) -{ - tree ret; - for (auto const& e : resp.get_view()) - ret.push_back(from_node_view(e)); - - return ret; -} - void add_nodes( flat_tree& to, std::string_view data, @@ -823,38 +795,8 @@ void test_comparison_self() BOOST_TEST_NOT(tempty != tempty); } -// Parses the same data into a tree and a -// flat_tree, they should be equal to each other. -void test_views_are_set() -{ - tree resp1; - flat_tree resp2; - generic_flat_response resp3; - - error_code ec; - deserialize(resp3_set, adapt2(resp1), ec); - BOOST_TEST_EQ(ec, error_code{}); - - deserialize(resp3_set, adapt2(resp2), ec); - BOOST_TEST_EQ(ec, error_code{}); - - deserialize(resp3_set, adapt2(resp3), ec); - BOOST_TEST_EQ(ec, error_code{}); - - BOOST_TEST_EQ(resp2.get_reallocs(), 1u); - BOOST_TEST_EQ(resp2.get_total_msgs(), 1u); - - BOOST_TEST_EQ(resp3.value().get_reallocs(), 1u); - BOOST_TEST_EQ(resp3.value().get_total_msgs(), 1u); - - auto const tmp2 = from_flat(resp2); - BOOST_TEST_ALL_EQ(resp1.begin(), resp1.end(), tmp2.begin(), tmp2.end()); - - auto const tmp3 = from_flat(resp3); - BOOST_TEST_ALL_EQ(resp1.begin(), resp1.end(), tmp3.begin(), tmp3.end()); -} - // The response should be reusable. +// TODO: this belongs to clear void test_reuse() { flat_tree tmp; @@ -882,49 +824,6 @@ void test_reuse() BOOST_TEST_ALL_EQ(resp1.begin(), resp1.end(), tmp.get_view().begin(), tmp.get_view().end()); } -void test_copy_assign_2() -{ - flat_tree ref1; - flat_tree ref2; - flat_tree ref3; - flat_tree ref4; - - error_code ec; - deserialize(resp3_set, adapt2(ref1), ec); - deserialize(resp3_set, adapt2(ref2), ec); - deserialize(resp3_set, adapt2(ref3), ec); - deserialize(resp3_set, adapt2(ref4), ec); - BOOST_TEST_EQ(ec, error_code{}); - - // Copy ctor - flat_tree copy1{ref1}; - - // Move ctor - flat_tree move1{std::move(ref2)}; - - // Copy assignment - flat_tree copy2 = ref1; - - // Move assignment - flat_tree move2 = std::move(ref3); - - // Assignment - flat_tree copy3; - copy3 = ref1; - - // Move assignment - flat_tree move3; - move3 = std::move(ref4); - - BOOST_TEST((copy1 == ref1)); - BOOST_TEST((copy2 == ref1)); - BOOST_TEST((copy3 == ref1)); - - BOOST_TEST((move1 == ref1)); - BOOST_TEST((move2 == ref1)); - BOOST_TEST((move3 == ref1)); -} - } // namespace int main() @@ -975,8 +874,6 @@ int main() test_comparison_empty(); test_comparison_self(); - test_views_are_set(); - test_copy_assign_2(); test_reuse(); return boost::report_errors(); From 5e1f4dca3aa33112ac165c80abdfdb7bf3e01faf Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Thu, 27 Nov 2025 12:52:50 +0100 Subject: [PATCH 30/36] Finished tests --- test/test_flat_tree.cpp | 69 +++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 34 deletions(-) diff --git a/test/test_flat_tree.cpp b/test/test_flat_tree.cpp index 43e2281d6..095af2c7f 100644 --- a/test/test_flat_tree.cpp +++ b/test/test_flat_tree.cpp @@ -38,9 +38,6 @@ using boost::system::error_code; namespace { -constexpr std::string_view - resp3_set = "~6\r\n+orange\r\n+apple\r\n+one\r\n+two\r\n+three\r\n+orange\r\n"; - void add_nodes( flat_tree& to, std::string_view data, @@ -318,6 +315,40 @@ void test_clear_empty() BOOST_TEST_EQ(t.get_total_msgs(), 0u); } +// With clear, memory can be reused +// The response should be reusable. +void test_clear_reuse() +{ + flat_tree t; + + // First use + add_nodes(t, "~6\r\n+orange\r\n+apple\r\n+one\r\n+two\r\n+three\r\n+orange\r\n"); + std::vector expected_nodes{ + {type::set, 6u, 0u, "" }, + {type::simple_string, 1u, 1u, "orange"}, + {type::simple_string, 1u, 1u, "apple" }, + {type::simple_string, 1u, 1u, "one" }, + {type::simple_string, 1u, 1u, "two" }, + {type::simple_string, 1u, 1u, "three" }, + {type::simple_string, 1u, 1u, "orange"}, + }; + check_nodes(t, expected_nodes); + BOOST_TEST_EQ(t.get_reallocs(), 1u); + BOOST_TEST_EQ(t.get_total_msgs(), 1u); + + // Second use + t.clear(); + add_nodes(t, "*2\r\n+hello\r\n+world\r\n"); + expected_nodes = { + {type::array, 2u, 0u, "" }, + {type::simple_string, 1u, 1u, "hello"}, + {type::simple_string, 1u, 1u, "world"}, + }; + check_nodes(t, expected_nodes); + BOOST_TEST_EQ(t.get_reallocs(), 1u); + BOOST_TEST_EQ(t.get_total_msgs(), 1u); +} + // --- Default ctor --- void test_default_constructor() { @@ -795,35 +826,6 @@ void test_comparison_self() BOOST_TEST_NOT(tempty != tempty); } -// The response should be reusable. -// TODO: this belongs to clear -void test_reuse() -{ - flat_tree tmp; - - // First use - error_code ec; - deserialize(resp3_set, adapt2(tmp), ec); - BOOST_TEST_EQ(ec, error_code{}); - - BOOST_TEST_EQ(tmp.get_reallocs(), 1u); - BOOST_TEST_EQ(tmp.get_total_msgs(), 1u); - - // Copy to compare after the reuse. - auto const resp1 = tmp.get_view(); - tmp.clear(); - - // Second use - deserialize(resp3_set, adapt2(tmp), ec); - BOOST_TEST_EQ(ec, error_code{}); - - // No reallocation this time. TODO: check this - BOOST_TEST_EQ(tmp.get_reallocs(), 1u); - BOOST_TEST_EQ(tmp.get_total_msgs(), 1u); - - BOOST_TEST_ALL_EQ(resp1.begin(), resp1.end(), tmp.get_view().begin(), tmp.get_view().end()); -} - } // namespace int main() @@ -840,6 +842,7 @@ int main() test_clear(); test_clear_empty(); + test_clear_reuse(); test_default_constructor(); @@ -874,7 +877,5 @@ int main() test_comparison_empty(); test_comparison_self(); - test_reuse(); - return boost::report_errors(); } From 1f40390862fc730436b3ec9eae7c18dbf27aa0c0 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Thu, 27 Nov 2025 13:29:16 +0100 Subject: [PATCH 31/36] Reference docs --- include/boost/redis/impl/flat_tree.ipp | 4 +- include/boost/redis/resp3/flat_tree.hpp | 166 +++++++++++++++++++----- 2 files changed, 138 insertions(+), 32 deletions(-) diff --git a/include/boost/redis/impl/flat_tree.ipp b/include/boost/redis/impl/flat_tree.ipp index 6709a2bad..73aeabe30 100644 --- a/include/boost/redis/impl/flat_tree.ipp +++ b/include/boost/redis/impl/flat_tree.ipp @@ -159,7 +159,7 @@ void flat_tree::reserve(std::size_t bytes, std::size_t nodes) view_tree_.reserve(nodes); } -void flat_tree::clear() +void flat_tree::clear() noexcept { data_.size = 0u; view_tree_.clear(); @@ -186,6 +186,4 @@ bool operator==(flat_tree const& a, flat_tree const& b) return a.view_tree_ == b.view_tree_ && a.total_msgs_ == b.total_msgs_; } -bool operator!=(flat_tree const& a, flat_tree const& b) { return !(a == b); } - } // namespace boost::redis::resp3 diff --git a/include/boost/redis/resp3/flat_tree.hpp b/include/boost/redis/resp3/flat_tree.hpp index bc9a7157a..edc419a25 100644 --- a/include/boost/redis/resp3/flat_tree.hpp +++ b/include/boost/redis/resp3/flat_tree.hpp @@ -34,66 +34,159 @@ struct flat_buffer { } // namespace detail -/** @brief A generic-response that stores data contiguously +/** @brief A generic response that stores data contiguously. * - * Similar to the @ref boost::redis::resp3::tree but data is - * stored contiguously. + * Implements a container of RESP3 nodes. It's similar to @ref boost::redis::resp3::tree, + * but node data is stored contiguously. This allows for amortized no allocations + * when re-using `flat_tree` objects. Like `tree`, it can contain the response + * to several Redis commands or several server pushes. Use @ref get_total_msgs + * to obtain how many responses this object contains. + * + * Objects are typically created by the user and passed to @ref connection::async_exec + * to be used as response containers. Call @ref get_view to access the actual RESP3 nodes. + * Once populated, `flat_tree` can't be modified, except for @ref clear and assignment. + * + * A `flat_tree` is conceptually similar to a pair of `std::vector` objects, one holding + * @ref resp3::node_view objects, and another owning the the string data that these views + * point to. The node capacity and the data capacity are the capacities of these two vectors. */ -struct flat_tree { +class flat_tree { public: - /// Default constructor + /** + * @brief Default constructor. + * + * Constructs an empty tree, with no nodes, zero node capacity and zero data capacity. + * + * @par Exception safety + * No-throw guarantee. + */ flat_tree() = default; - /// Move constructor - flat_tree(flat_tree&&) noexcept = default; + /** + * @brief Move constructor. + * + * Constructs a tree by taking ownership of the nodes in `other`. + * + * @par Object lifetimes + * References to the nodes and strings in `other` remain valid. + * + * @par Exception safety + * No-throw guarantee. + */ + flat_tree(flat_tree&& other) noexcept = default; - /// Copy constructor + /** + * @brief Copy constructor. + * + * Constructs a tree by copying the nodes in `other`. After the copy, + * `*this` and `other` have independent lifetimes (usual copy semantics). + * + * @par Exception safety + * Strong guarantee. Memory allocations might throw. + */ flat_tree(flat_tree const& other); - /// Move assignment + /** + * @brief Move assignment. + * + * Replaces the nodes in `*this` by taking ownership of the nodes in `other`. + * `other` is left in a valid but unspecified state. + * + * @par Object lifetimes + * References to the nodes and strings in `other` remain valid. + * References to the nodes and strings in `*this` are invalidated. + * + * @par Exception safety + * No-throw guarantee. + */ flat_tree& operator=(flat_tree&& other) = default; - /// Copy assignment + /** + * @brief Copy assignment. + * + * Replaces the nodes in `*this` by copying the nodes in `other`. + * After the copy, `*this` and `other` have independent lifetimes (usual copy semantics). + * + * @par Object lifetimes + * References to the nodes and strings in `*this` are invalidated. + * + * @par Exception safety + * Basic guarantee. Memory allocations might throw. + */ flat_tree& operator=(const flat_tree& other); friend bool operator==(flat_tree const&, flat_tree const&); friend bool operator!=(flat_tree const&, flat_tree const&); - /** @brief Reserve capacity + /** @brief Reserves capacity for incoming data. * - * Reserve memory for incoming data. + * Adding nodes (e.g. by passing the tree to `async_exec`) + * won't cause reallocations until the data or node capacities + * are exceeded, following the usual vector semantics. + * The implementation might reserve more capacity than the one requested. + * + * @par Object lifetimes + * References to the nodes and strings in `*this` are invalidated. * - * @param bytes Number of bytes to reserve for data. - * @param nodes Number of nodes to reserve. + * @par Exception safety + * Basic guarantee. Memory allocations might throw. + * + * @param bytes Number of bytes to reserve for data. + * @param nodes Number of nodes to reserve. */ void reserve(std::size_t bytes, std::size_t nodes); - /** @brief Clear both the data and the node buffers - * - * @Note: A `boost::redis:.flat_tree` can contain the - * response to multiple Redis commands and server pushes. Calling - * this function will erase everything contained in it. + /** @brief Clears the tree so it contains no nodes. + * + * Calling this function removes every node, making + * @ref get_views return empty and @ref get_total_msgs + * return zero. It does not modify the object's capacity. + * + * To re-use a `flat_tree` for several requests, + * use `clear()` before each `async_exec` call. + * + * @par Object lifetimes + * References to the nodes and strings in `*this` are invalidated. + * + * @par Exception safety + * No-throw guarantee. */ - void clear(); + void clear() noexcept; + // TODO: this /// Returns the size of the data buffer auto data_size() const noexcept -> std::size_t { return data_.size; } // TODO: document auto data_capacity() const noexcept -> std::size_t { return data_.capacity; } - /// Returns the RESP3 response - auto get_view() const -> view_tree const& { return view_tree_; } + /** @brief Returns a vector with the nodes in the tree. + * + * This is the main way to access the contents of the tree. + * + * @par Exception safety + * No-throw guarantee. + */ + auto get_view() const noexcept -> view_tree const& { return view_tree_; } - /** @brief Returns the number of times reallocation took place + /** @brief Returns the number of memory reallocations that took place within this object. * - * This function returns how many reallocations were performed and - * can be useful to determine how much memory to reserve upfront. + * This function returns how many reallocations were performed and + * can be useful to determine how much memory to reserve upfront. + * + * @par Exception safety + * No-throw guarantee. */ auto get_reallocs() const noexcept -> std::size_t { return data_.reallocs; } - /// Returns the number of complete RESP3 messages contained in this object. + /** @brief Returns the number of complete RESP3 messages contained in this object. + * + * This value is equal to the number of nodes in the tree with a depth of zero. + * + * @par Exception safety + * No-throw guarantee. + */ std::size_t get_total_msgs() const noexcept { return total_msgs_; } private: @@ -109,11 +202,26 @@ struct flat_tree { std::size_t total_msgs_ = 0u; }; -/// Equality operator +/** + * @brief Equality operator. + * @relates flat_tree + * + * Two trees are equal if they contain the same nodes in the same order. + * Capacities are not taken into account. + * + * @par Exception safety + * No-throw guarantee. + */ bool operator==(flat_tree const&, flat_tree const&); -/// Inequality operator -bool operator!=(flat_tree const&, flat_tree const&); +/** + * @brief Inequality operator. + * @relates flat_tree + * + * @par Exception safety + * No-throw guarantee. + */ +inline bool operator!=(flat_tree const& lhs, flat_tree const& rhs) { return !(lhs == rhs); } } // namespace resp3 } // namespace boost::redis From cf60b9904a8b6102ab991e5027436aadb2800062 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Thu, 27 Nov 2025 13:35:20 +0100 Subject: [PATCH 32/36] quickref --- doc/modules/ROOT/pages/reference.adoc | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/doc/modules/ROOT/pages/reference.adoc b/doc/modules/ROOT/pages/reference.adoc index e87ee7cd8..de82944a0 100644 --- a/doc/modules/ROOT/pages/reference.adoc +++ b/doc/modules/ROOT/pages/reference.adoc @@ -55,6 +55,8 @@ xref:reference:boost/redis/response.adoc[`response`] xref:reference:boost/redis/generic_response.adoc[`generic_response`] +xref:reference:boost/redis/generic_flat_response.adoc[`generic_flat_response`] + xref:reference:boost/redis/consume_one-08.adoc[`consume_one`] @@ -76,6 +78,14 @@ xref:reference:boost/redis/resp3/node.adoc[`node`] xref:reference:boost/redis/resp3/node_view.adoc[`node_view`] +xref:reference:boost/redis/resp3/basic_tree.adoc[`basic_tree`] + +xref:reference:boost/redis/resp3/tree.adoc[`tree`] + +xref:reference:boost/redis/resp3/tree_view.adoc[`tree_view`] + +xref:reference:boost/redis/resp3/flat_tree.adoc[`flat_tree`] + xref:reference:boost/redis/resp3/boost_redis_to_bulk-08.adoc[`boost_redis_to_bulk`] xref:reference:boost/redis/resp3/type.adoc[`type`] From fc9602c1439665554273265d2106acf2e4f712b2 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Thu, 27 Nov 2025 13:47:39 +0100 Subject: [PATCH 33/36] doc fixes --- doc/modules/ROOT/pages/reference.adoc | 2 +- include/boost/redis/resp3/flat_tree.hpp | 35 ++++++++++++++++++++----- 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/doc/modules/ROOT/pages/reference.adoc b/doc/modules/ROOT/pages/reference.adoc index de82944a0..d4be252aa 100644 --- a/doc/modules/ROOT/pages/reference.adoc +++ b/doc/modules/ROOT/pages/reference.adoc @@ -82,7 +82,7 @@ xref:reference:boost/redis/resp3/basic_tree.adoc[`basic_tree`] xref:reference:boost/redis/resp3/tree.adoc[`tree`] -xref:reference:boost/redis/resp3/tree_view.adoc[`tree_view`] +xref:reference:boost/redis/resp3/view_tree.adoc[`view_tree`] xref:reference:boost/redis/resp3/flat_tree.adoc[`flat_tree`] diff --git a/include/boost/redis/resp3/flat_tree.hpp b/include/boost/redis/resp3/flat_tree.hpp index edc419a25..b0bff1014 100644 --- a/include/boost/redis/resp3/flat_tree.hpp +++ b/include/boost/redis/resp3/flat_tree.hpp @@ -140,7 +140,7 @@ class flat_tree { /** @brief Clears the tree so it contains no nodes. * * Calling this function removes every node, making - * @ref get_views return empty and @ref get_total_msgs + * @ref get_view return empty and @ref get_total_msgs * return zero. It does not modify the object's capacity. * * To re-use a `flat_tree` for several requests, @@ -154,11 +154,28 @@ class flat_tree { */ void clear() noexcept; - // TODO: this - /// Returns the size of the data buffer + /** @brief Returns the size of the data buffer, in bytes. + * + * You may use this function to calculate how much capacity + * should be reserved for data when calling @ref reserve. + * + * @par Exception safety + * No-throw guarantee. + * + * @returns The number of bytes in use in the data buffer. + */ auto data_size() const noexcept -> std::size_t { return data_.size; } - // TODO: document + /** @brief Returns the capacity of the data buffer, in bytes. + * + * Note that the actual capacity of the data buffer may be bigger + * than the one requested by @ref reserve. + * + * @par Exception safety + * No-throw guarantee. + * + * @returns The capacity of the data buffer, in bytes. + */ auto data_capacity() const noexcept -> std::size_t { return data_.capacity; } /** @brief Returns a vector with the nodes in the tree. @@ -167,16 +184,20 @@ class flat_tree { * * @par Exception safety * No-throw guarantee. + * + * @returns The nodes in the tree. */ auto get_view() const noexcept -> view_tree const& { return view_tree_; } - /** @brief Returns the number of memory reallocations that took place within this object. + /** @brief Returns the number of memory reallocations that took place in the data buffer. * - * This function returns how many reallocations were performed and + * This function returns how many reallocations in the data buffer were performed and * can be useful to determine how much memory to reserve upfront. * * @par Exception safety * No-throw guarantee. + * + * @returns The number of times that the data buffer reallocated its memory. */ auto get_reallocs() const noexcept -> std::size_t { return data_.reallocs; } @@ -186,6 +207,8 @@ class flat_tree { * * @par Exception safety * No-throw guarantee. + * + * @returns The number of complete RESP3 messages contained in this object. */ std::size_t get_total_msgs() const noexcept { return total_msgs_; } From a953a23cd1a402a81112a248d2e4045d42839b7e Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Thu, 27 Nov 2025 13:49:17 +0100 Subject: [PATCH 34/36] qualify resp3 names in doc --- doc/modules/ROOT/pages/reference.adoc | 22 +++++++++++----------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/doc/modules/ROOT/pages/reference.adoc b/doc/modules/ROOT/pages/reference.adoc index d4be252aa..6281911ee 100644 --- a/doc/modules/ROOT/pages/reference.adoc +++ b/doc/modules/ROOT/pages/reference.adoc @@ -72,33 +72,33 @@ xref:reference:boost/redis/adapter/result.adoc[`adapter::result`] xref:reference:boost/redis/any_adapter.adoc[`any_adapter`] | -xref:reference:boost/redis/resp3/basic_node.adoc[`basic_node`] +xref:reference:boost/redis/resp3/basic_node.adoc[`resp3::basic_node`] -xref:reference:boost/redis/resp3/node.adoc[`node`] +xref:reference:boost/redis/resp3/node.adoc[`resp3::node`] -xref:reference:boost/redis/resp3/node_view.adoc[`node_view`] +xref:reference:boost/redis/resp3/node_view.adoc[`resp3::node_view`] -xref:reference:boost/redis/resp3/basic_tree.adoc[`basic_tree`] +xref:reference:boost/redis/resp3/basic_tree.adoc[`resp3::basic_tree`] -xref:reference:boost/redis/resp3/tree.adoc[`tree`] +xref:reference:boost/redis/resp3/tree.adoc[`resp3::tree`] -xref:reference:boost/redis/resp3/view_tree.adoc[`view_tree`] +xref:reference:boost/redis/resp3/view_tree.adoc[`resp3::view_tree`] -xref:reference:boost/redis/resp3/flat_tree.adoc[`flat_tree`] +xref:reference:boost/redis/resp3/flat_tree.adoc[`resp3::flat_tree`] xref:reference:boost/redis/resp3/boost_redis_to_bulk-08.adoc[`boost_redis_to_bulk`] -xref:reference:boost/redis/resp3/type.adoc[`type`] +xref:reference:boost/redis/resp3/type.adoc[`resp3::type`] -xref:reference:boost/redis/resp3/is_aggregate.adoc[`is_aggregate`] +xref:reference:boost/redis/resp3/is_aggregate.adoc[`resp3::is_aggregate`] | xref:reference:boost/redis/adapter/adapt2.adoc[`adapter::adapt2`] -xref:reference:boost/redis/resp3/parser.adoc[`parser`] +xref:reference:boost/redis/resp3/parser.adoc[`resp3::parser`] -xref:reference:boost/redis/resp3/parse.adoc[`parse`] +xref:reference:boost/redis/resp3/parse.adoc[`resp3::parse`] |=== \ No newline at end of file From 807c8a2d311e292308e58252a6189b9f01a96950 Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Thu, 27 Nov 2025 13:52:34 +0100 Subject: [PATCH 35/36] Mention in discussion --- doc/modules/ROOT/pages/requests_responses.adoc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/doc/modules/ROOT/pages/requests_responses.adoc b/doc/modules/ROOT/pages/requests_responses.adoc index 59938dd5c..7f40fed11 100644 --- a/doc/modules/ROOT/pages/requests_responses.adoc +++ b/doc/modules/ROOT/pages/requests_responses.adoc @@ -278,7 +278,8 @@ struct basic_node { ---- Any response to a Redis command can be parsed into a -xref:reference:boost/redis/generic_response.adoc[boost::redis::generic_response]. +xref:reference:boost/redis/generic_response.adoc[boost::redis::generic_response] +and its counterpart xref:reference:boost/redis/generic_flat_response.adoc[boost::redis::generic_flat_response]. The vector can be seen as a pre-order view of the response tree. Using it is not different than using other types: @@ -292,7 +293,7 @@ co_await conn->async_exec(req, resp); For example, suppose we want to retrieve a hash data structure from Redis with `HGETALL`, some of the options are -* `boost::redis::generic_response`: always works. +* `boost::redis::generic_response` and `boost::redis::generic_flat_response`: always works. * `std::vector`: efficient and flat, all elements as string. * `std::map`: efficient if you need the data as a `std::map`. * `std::map`: efficient if you are storing serialized data. Avoids temporaries and requires `boost_redis_from_bulk` for `U` and `V`. From e7a808c4ebe4ee8db091d75d51ff0b97834a30ae Mon Sep 17 00:00:00 2001 From: Ruben Perez Date: Thu, 27 Nov 2025 13:55:36 +0100 Subject: [PATCH 36/36] Make node operator== bool instead of auto and docs --- include/boost/redis/resp3/node.hpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/include/boost/redis/resp3/node.hpp b/include/boost/redis/resp3/node.hpp index 1e284dab3..e7a768222 100644 --- a/include/boost/redis/resp3/node.hpp +++ b/include/boost/redis/resp3/node.hpp @@ -43,7 +43,7 @@ struct basic_node { * @param b Right hand side node object. */ template -auto operator==(basic_node const& a, basic_node const& b) +bool operator==(basic_node const& a, basic_node const& b) { // clang-format off return a.aggregate_size == b.aggregate_size @@ -53,9 +53,14 @@ auto operator==(basic_node const& a, basic_node const& b) // clang-format on }; -/// Inequality operator for RESP3 nodes +/** @brief Inequality operator for RESP3 nodes. + * @relates basic_node + * + * @param a Left hand side node object. + * @param b Right hand side node object. + */ template -auto operator!=(basic_node const& a, basic_node const& b) +bool operator!=(basic_node const& a, basic_node const& b) { return !(a == b); };