From 3eb1d1025bb19fb91b974dda6d99fe4fdae08365 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Tue, 6 Jun 2017 11:56:06 -0700 Subject: [PATCH] Fix basic_fields allocator awareness --- CHANGELOG.md | 1 + include/beast/http/fields.hpp | 164 ++++--- include/beast/http/impl/fields.ipp | 658 ++++++++++++++++++----------- 3 files changed, 513 insertions(+), 310 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1f8c7572..63134933 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ Version 50 * parser is constructible from other body types * Add field enumeration * Use allocator more in basic_fields +* Fix basic_fields allocator awareness API Changes: diff --git a/include/beast/http/fields.hpp b/include/beast/http/fields.hpp index ee55df27..6c0b339d 100644 --- a/include/beast/http/fields.hpp +++ b/include/beast/http/fields.hpp @@ -81,13 +81,13 @@ public: /// A constant iterator to the field sequence. using iterator = const_iterator; - /// Default constructor. - basic_fields() = default; - /// Destructor ~basic_fields(); - /** Construct the fields. + /// Default constructor. + basic_fields() = default; + + /** Constructor. @param alloc The allocator to use. */ @@ -96,32 +96,60 @@ public: /** Move constructor. - The moved-from object becomes an empty field sequence. - - @param other The object to move from. + The moved-from object behaves as if by call to @ref clear. */ - basic_fields(basic_fields&& other); - - /** Move assignment. - - The moved-from object becomes an empty field sequence. - - @param other The object to move from. - */ - basic_fields& operator=(basic_fields&& other); + basic_fields(basic_fields&&); /// Copy constructor. basic_fields(basic_fields const&); + /** Copy constructor. + + @param alloc The allocator to use. + */ + basic_fields(basic_fields const&, Allocator const& alloc); + + /// Copy constructor. +#if BEAST_DOXYGEN + template +#else + template::value>::type> +#endif + basic_fields(basic_fields const&); + + /** Copy constructor. + + @param alloc The allocator to use. + */ +#if BEAST_DOXYGEN + template +#else + template::value>::type> +#endif + basic_fields(basic_fields const&, + Allocator const& alloc); + + /** Move assignment. + + The moved-from object behaves as if by call to @ref clear. + */ + basic_fields& operator=(basic_fields&&); + /// Copy assignment. basic_fields& operator=(basic_fields const&); - /// Copy constructor. - template - basic_fields(basic_fields const&); - /// Copy assignment. +#if BEAST_DOXYGEN template +#else + template::value>::type> +#endif basic_fields& operator=(basic_fields const&); /// Return a copy of the allocator associated with the container. @@ -269,6 +297,12 @@ public: boost::lexical_cast(value)); } + /// Swap two field containers + template + friend + void + swap(basic_fields& lhs, basic_fields& rhs); + protected: // for header string_view method_impl() const; @@ -284,31 +318,25 @@ protected: void chunked_impl(); private: - struct element - : boost::intrusive::set_base_hook < + class element + : public boost::intrusive::set_base_hook < boost::intrusive::link_mode < boost::intrusive::normal_link>> - , boost::intrusive::list_base_hook < + , public boost::intrusive::list_base_hook < boost::intrusive::link_mode < boost::intrusive::normal_link>> { - element( - string_view name, string_view value) - { - char* p = reinterpret_cast(this + 1); - data.first = p; - data.off = - static_cast(name.size() + 2); - data.len = - static_cast(value.size()); - std::memcpy(p, name.data(), name.size()); - p[data.off-2] = ':'; - p[data.off-1] = ' '; - std::memcpy( - p + data.off, value.data(), value.size()); - p[data.off + data.len] = '\r'; - p[data.off + data.len + 1] = '\n'; - } + off_t off_; + off_t len_; + + public: + element(string_view name, string_view value); + + string_view + name() const; + + string_view + value() const; value_type data; }; @@ -355,37 +383,47 @@ private: element, boost::intrusive::constant_time_size< true>, boost::intrusive::compare>::type; - void - delete_all(); - - void - move_assign(basic_fields&, std::false_type); - - void - move_assign(basic_fields&, std::true_type); - - void - copy_assign(basic_fields const&, std::false_type); - - void - copy_assign(basic_fields const&, std::true_type); - - template - void - copy_from(FieldSequence const& fs) - { - for(auto const& e : fs) - insert(e.name(), e.value()); - } - element& new_element(string_view name, string_view value); void delete_element(element& e); + void + realloc_string(string_view& dest, string_view s); + + template + void + copy_all(basic_fields const&); + + void + clear_all(); + + void + delete_list(); + + void + move_assign(basic_fields&, std::true_type); + + void + move_assign(basic_fields&, std::false_type); + + void + copy_assign(basic_fields const&, std::true_type); + + void + copy_assign(basic_fields const&, std::false_type); + + void + swap(basic_fields& other, std::true_type); + + void + swap(basic_fields& other, std::false_type); + set_t set_; list_t list_; + string_view method_; + string_view target_or_reason_; alloc_type alloc_; }; diff --git a/include/beast/http/impl/fields.ipp b/include/beast/http/impl/fields.ipp index 267773b4..c8625f1d 100644 --- a/include/beast/http/impl/fields.ipp +++ b/include/beast/http/impl/fields.ipp @@ -18,147 +18,6 @@ namespace http { //------------------------------------------------------------------------------ -template -basic_fields:: -~basic_fields() -{ - delete_all(); -} - -//------------------------------------------------------------------------------ - -template -inline -string_view -basic_fields:: -method_impl() const -{ - return (*this)[":method"]; -} - -template -inline -string_view -basic_fields:: -target_impl() const -{ - return (*this)[":target"]; -} - -template -inline -string_view -basic_fields:: -reason_impl() const -{ - return (*this)[":reason"]; -} - -template -inline -void -basic_fields:: -method_impl(string_view s) -{ - if(s.empty()) - this->erase(":method"); - else - this->replace(":method", s); -} - -template -inline -void -basic_fields:: -target_impl(string_view s) -{ - if(s.empty()) - this->erase(":target"); - else - this->replace(":target", s); -} - -template -inline -void -basic_fields:: -reason_impl(string_view s) -{ - if(s.empty()) - this->erase(":reason"); - else - this->replace(":reason", s); -} - -template -inline -void -basic_fields:: -content_length_impl(std::uint64_t n) -{ - this->erase("Content-Length"); - this->insert("Content-Length", - to_static_string(n)); -} - -template -inline -void -basic_fields:: -connection_impl(close_t) -{ - auto it = find("Connection"); - if(it == end()) - this->insert("Connection", "close"); - else - this->replace("Connection", - it->value().to_string() + ", close"); -} - -template -inline -void -basic_fields:: -connection_impl(keep_alive_t) -{ - auto it = find("Connection"); - if(it == end()) - this->insert("Connection", "keep-alive"); - else - this->replace("Connection", - it->value().to_string() + ", keep-alive"); -} - -template -inline -void -basic_fields:: -connection_impl(upgrade_t) -{ - auto it = find("Connection"); - if(it == end()) - this->insert("Connection", "upgrade"); - else - this->replace("Connection", - it->value().to_string() + ", upgrade"); -} - -template -inline -void -basic_fields:: -chunked_impl() -{ - auto it = find("Transfer-Encoding"); - if(it == end()) - this->insert("Transfer-Encoding", "chunked"); - else - this->replace("Transfer-Encoding", - it->value().to_string() + ", chunked"); -} - -//------------------------------------------------------------------------------ - template class basic_fields:: const_iterator @@ -245,103 +104,63 @@ public: } }; +//------------------------------------------------------------------------------ + template -void basic_fields:: -delete_all() +element:: +element(string_view name, string_view value) + : off_(static_cast(name.size() + 2)) + , len_(static_cast(value.size())) { - for(auto it = list_.begin(); it != list_.end();) - delete_element(*it++); + char* p = reinterpret_cast(this + 1); + data.first = p; + data.off = off_; + data.len = len_; + + p[off_-2] = ':'; + p[off_-1] = ' '; + p[off_ + len_] = '\r'; + p[off_ + len_ + 1] = '\n'; + std::memcpy(p, name.data(), name.size()); + std::memcpy(p + off_, value.data(), value.size()); } template inline -void +string_view basic_fields:: -move_assign(basic_fields& other, std::false_type) +element:: +name() const { - if(alloc_ != other.alloc_) - { - copy_from(other); - other.clear(); - } - else - { - set_ = std::move(other.set_); - list_ = std::move(other.list_); - } + return {reinterpret_cast< + char const*>(this + 1), + static_cast(off_ - 2)}; } template inline -void +string_view basic_fields:: -move_assign(basic_fields& other, std::true_type) +element:: +value() const { - alloc_ = std::move(other.alloc_); - set_ = std::move(other.set_); - list_ = std::move(other.list_); -} - -template -inline -void -basic_fields:: -copy_assign(basic_fields const& other, std::false_type) -{ - copy_from(other); -} - -template -inline -void -basic_fields:: -copy_assign(basic_fields const& other, std::true_type) -{ - alloc_ = other.alloc_; - copy_from(other); -} - -template -auto -basic_fields:: -new_element(string_view name, string_view value) -> - element& -{ - if(name.size() + 2 > - (std::numeric_limits::max)()) - BOOST_THROW_EXCEPTION(std::length_error{ - "field name too large"}); - if(value.size() + 2 > - (std::numeric_limits::max)()) - BOOST_THROW_EXCEPTION(std::length_error{ - "field value too large"}); - value = detail::trim(value); - std::uint16_t const off = - static_cast(name.size() + 2); - std::uint16_t const len = - static_cast(value.size()); - auto const p = alloc_traits::allocate(alloc_, - 1 + (off + len + 2 + sizeof(element) - 1) / - sizeof(element)); - alloc_traits::construct(alloc_, p, name, value); - return *p; -} - -template -void -basic_fields:: -delete_element(element& e) -{ - auto const n = 1 + - (e.data.off + e.data.len + 2 + - sizeof(element) - 1) / sizeof(element); - alloc_traits::destroy(alloc_, &e); - alloc_traits::deallocate(alloc_, &e, n); + return {reinterpret_cast< + char const*>(this + 1) + off_, + static_cast(len_)}; } //------------------------------------------------------------------------------ +template +basic_fields:: +~basic_fields() +{ + delete_list(); + realloc_string(method_, {}); + realloc_string(target_or_reason_, {}); +} + template basic_fields:: basic_fields(Allocator const& alloc) @@ -354,8 +173,48 @@ basic_fields:: basic_fields(basic_fields&& other) : set_(std::move(other.set_)) , list_(std::move(other.list_)) + , method_(other.method_) + , target_or_reason_(other.target_or_reason_) , alloc_(std::move(other.alloc_)) { + other.method_.clear(); + other.target_or_reason_.clear(); +} + +template +basic_fields:: +basic_fields(basic_fields const& other) + : basic_fields(alloc_traits:: + select_on_container_copy_construction(other.alloc_)) +{ + copy_all(other); +} + +template +basic_fields:: +basic_fields(basic_fields const& other, + Allocator const& alloc) + : alloc_(alloc) +{ + copy_all(other); +} + +template +template +basic_fields:: +basic_fields(basic_fields const& other) +{ + copy_all(other); +} + +template +template +basic_fields:: +basic_fields(basic_fields const& other, + Allocator const& alloc) + : alloc_(alloc) +{ + copy_all(other); } template @@ -366,53 +225,36 @@ operator=(basic_fields&& other) -> { if(this == &other) return *this; - clear(); - move_assign(other, std::integral_constant{}); + move_assign(other, typename alloc_traits:: + propagate_on_container_move_assignment{}); return *this; } -template -basic_fields:: -basic_fields(basic_fields const& other) - : basic_fields(alloc_traits:: - select_on_container_copy_construction(other.alloc_)) -{ - copy_from(other); -} - template auto basic_fields:: operator=(basic_fields const& other) -> basic_fields& { - clear(); - copy_assign(other, std::integral_constant{}); + copy_assign(other, typename alloc_traits:: + propagate_on_container_copy_assignment{}); return *this; } template -template -basic_fields:: -basic_fields(basic_fields const& other) -{ - copy_from(other); -} - -template -template +template auto basic_fields:: operator=(basic_fields const& other) -> basic_fields& { - clear(); - copy_from(other); + clear_all(); + copy_all(other); return *this; } +//------------------------------------------------------------------------------ + template std::size_t basic_fields:: @@ -453,9 +295,9 @@ void basic_fields:: clear() noexcept { - delete_all(); - list_.clear(); + delete_list(); set_.clear(); + list_.clear(); } template @@ -501,6 +343,328 @@ replace(string_view name, string_view value) insert(name, value); } +//------------------------------------------------------------------------------ + +template +inline +string_view +basic_fields:: +method_impl() const +{ + return method_; +} + +template +inline +string_view +basic_fields:: +target_impl() const +{ + return target_or_reason_; +} + +template +inline +string_view +basic_fields:: +reason_impl() const +{ + return target_or_reason_; +} + +template +inline +void +basic_fields:: +method_impl(string_view s) +{ + realloc_string(method_, s); +} + +template +inline +void +basic_fields:: +target_impl(string_view s) +{ + realloc_string(target_or_reason_, s); +} + +template +inline +void +basic_fields:: +reason_impl(string_view s) +{ + realloc_string(target_or_reason_, s); +} + +template +inline +void +basic_fields:: +content_length_impl(std::uint64_t n) +{ + this->erase("Content-Length"); + this->insert("Content-Length", + to_static_string(n)); +} + +template +inline +void +basic_fields:: +connection_impl(close_t) +{ + auto it = find("Connection"); + if(it == end()) + this->insert("Connection", "close"); + else + this->replace("Connection", + it->value().to_string() + ", close"); +} + +template +inline +void +basic_fields:: +connection_impl(keep_alive_t) +{ + auto it = find("Connection"); + if(it == end()) + this->insert("Connection", "keep-alive"); + else + this->replace("Connection", + it->value().to_string() + ", keep-alive"); +} + +template +inline +void +basic_fields:: +connection_impl(upgrade_t) +{ + auto it = find("Connection"); + if(it == end()) + this->insert("Connection", "upgrade"); + else + this->replace("Connection", + it->value().to_string() + ", upgrade"); +} + +template +inline +void +basic_fields:: +chunked_impl() +{ + auto it = find("Transfer-Encoding"); + if(it == end()) + this->insert("Transfer-Encoding", "chunked"); + else + this->replace("Transfer-Encoding", + it->value().to_string() + ", chunked"); +} + +//------------------------------------------------------------------------------ + +template +auto +basic_fields:: +new_element(string_view name, string_view value) -> + element& +{ + if(name.size() + 2 > + (std::numeric_limits::max)()) + BOOST_THROW_EXCEPTION(std::length_error{ + "field name too large"}); + if(value.size() + 2 > + (std::numeric_limits::max)()) + BOOST_THROW_EXCEPTION(std::length_error{ + "field value too large"}); + value = detail::trim(value); + std::uint16_t const off = + static_cast(name.size() + 2); + std::uint16_t const len = + static_cast(value.size()); + auto const p = alloc_traits::allocate(alloc_, + 1 + (off + len + 2 + sizeof(element) - 1) / + sizeof(element)); + alloc_traits::construct(alloc_, p, name, value); + return *p; +} + +template +void +basic_fields:: +delete_element(element& e) +{ + auto const n = 1 + + (e.data.off + e.data.len + 2 + + sizeof(element) - 1) / sizeof(element); + alloc_traits::destroy(alloc_, &e); + alloc_traits::deallocate(alloc_, &e, n); +} + +template +void +basic_fields:: +realloc_string(string_view& dest, string_view s) +{ + s = detail::trim(s); + if(dest.empty() && s.empty()) + return; + auto a = typename std::allocator_traits< + Allocator>::template rebind_alloc< + char>(alloc_); + if(! dest.empty()) + { + a.deallocate(const_cast( + dest.data()), dest.size()); + dest.clear(); + } + if(! s.empty()) + { + auto const p = a.allocate(s.size()); + std::memcpy(p, s.data(), s.size()); + dest = {p, s.size()}; + } +} + +template +template +void +basic_fields:: +copy_all(basic_fields const& other) +{ + for(auto const& e : other.list_) + insert(e.name(), e.value()); + realloc_string(method_, other.method_); + realloc_string(target_or_reason_, + other.target_or_reason_); +} + +template +void +basic_fields:: +clear_all() +{ + clear(); + realloc_string(method_, {}); + realloc_string(target_or_reason_, {}); +} + +template +void +basic_fields:: +delete_list() +{ + for(auto it = list_.begin(); it != list_.end();) + delete_element(*it++); +} + +//------------------------------------------------------------------------------ + +template +inline +void +basic_fields:: +move_assign(basic_fields& other, std::true_type) +{ + clear_all(); + set_ = std::move(other.set_); + list_ = std::move(other.list_); + method_ = other.method_; + target_or_reason_ = other.target_or_reason_; + other.method_.clear(); + other.target_or_reason_.clear(); + alloc_ = other.alloc_; +} + +template +inline +void +basic_fields:: +move_assign(basic_fields& other, std::false_type) +{ + clear_all(); + if(alloc_ != other.alloc_) + { + copy_all(other); + other.clear_all(); + } + else + { + set_ = std::move(other.set_); + list_ = std::move(other.list_); + method_ = other.method_; + target_or_reason_ = other.target_or_reason_; + other.method_.clear(); + other.target_or_reason_.clear(); + } +} + +template +inline +void +basic_fields:: +copy_assign(basic_fields const& other, std::true_type) +{ + clear_all(); + alloc_ = other.alloc_; + copy_all(other); +} + +template +inline +void +basic_fields:: +copy_assign(basic_fields const& other, std::false_type) +{ + clear_all(); + copy_all(other); +} + +template +void +swap(basic_fields& lhs, + basic_fields& rhs) +{ + using alloc_traits = typename + basic_fields::alloc_traits; + lhs.swap(rhs, typename alloc_traits:: + propagate_on_container_swap{}); +} + +template +inline +void +basic_fields:: +swap(basic_fields& other, std::true_type) +{ + using std::swap; + swap(alloc_, other.alloc_); + swap(set_, other.set_); + swap(list_, other.list_); + swap(method_, other.method_); + swap(target_or_reason_, other.target_or_reason_); +} + +template +inline +void +basic_fields:: +swap(basic_fields& other, std::false_type) +{ + BOOST_ASSERT(alloc_ == other.alloc_); + using std::swap; + swap(set_, other.set_); + swap(list_, other.list_); + swap(method_, other.method_); + swap(target_or_reason_, other.target_or_reason_); +} + + } // http } // beast