Use allocator more in basic_fields

Fix #392

CHANGELOG.md

@@ -2,6 +2,7 @@ Version 50
 
 * parser is constructible from other body types
 * Add field enumeration
+* Use allocator more in basic_fields
 
 API Changes:
 

include/beast/http/detail/fields.hpp

@@ -1,214 +0,0 @@
-//
-// Copyright (c) 2013-2017 Vinnie Falco (vinnie dot falco at gmail dot com)
-//
-// Distributed under the Boost Software License, Version 1.0. (See accompanying
-// file LICENSE_1_0.txt or copy at http://www.boost.org/LICENSE_1_0.txt)
-//
-
-#ifndef BEAST_HTTP_DETAIL_FIELDS_HPP
-#define BEAST_HTTP_DETAIL_FIELDS_HPP
-
-#include <beast/core/string_view.hpp>
-#include <beast/core/detail/ci_char_traits.hpp>
-#include <boost/intrusive/list.hpp>
-#include <boost/intrusive/set.hpp>
-
-namespace beast {
-namespace http {
-
-template<class Allocator>
-class basic_fields;
-
-namespace detail {
-
-class basic_fields_base
-{
-public:
-    struct value_type
-    {
-        std::string first;
-        std::string second;
-
-        value_type(string_view name_,
-                string_view value_)
-            : first(name_)
-            , second(value_)
-        {
-        }
-
-        string_view
-        name() const
-        {
-            return first;
-        }
-
-        string_view
-        value() const
-        {
-            return second;
-        }
-    };
-
-protected:
-    template<class Allocator>
-    friend class beast::http::basic_fields;
-
-    struct element
-        : boost::intrusive::set_base_hook <
-            boost::intrusive::link_mode <
-                boost::intrusive::normal_link>>
-        , boost::intrusive::list_base_hook <
-            boost::intrusive::link_mode <
-                boost::intrusive::normal_link>>
-    {
-        value_type data;
-
-        element(string_view name,
-                string_view value)
-            : data(name, value)
-        {
-        }
-    };
-
-    struct less : private beast::detail::ci_less
-    {
-        template<class String>
-        bool
-        operator()(String const& lhs, element const& rhs) const
-        {
-            return ci_less::operator()(lhs, rhs.data.first);
-        }
-
-        template<class String>
-        bool
-        operator()(element const& lhs, String const& rhs) const
-        {
-            return ci_less::operator()(lhs.data.first, rhs);
-        }
-
-        bool
-        operator()(element const& lhs, element const& rhs) const
-        {
-            return ci_less::operator()(
-                lhs.data.first, rhs.data.first);
-        }
-    };
-
-    using list_t = boost::intrusive::make_list<element,
-        boost::intrusive::constant_time_size<false>>::type;
-
-    using set_t = boost::intrusive::make_multiset<element,
-        boost::intrusive::constant_time_size<true>,
-            boost::intrusive::compare<less>>::type;
-
-    // data
-    set_t set_;
-    list_t list_;
-
-    basic_fields_base(set_t&& set, list_t&& list)
-        : set_(std::move(set))
-        , list_(std::move(list))
-    {
-    }
-
-public:
-    class const_iterator;
-
-    using iterator = const_iterator;
-
-    basic_fields_base() = default;
-};
-
-//------------------------------------------------------------------------------
-
-class basic_fields_base::const_iterator
-{
-    using iter_type = list_t::const_iterator;
-
-    iter_type it_;
-
-    template<class Allocator>
-    friend class beast::http::basic_fields;
-
-    friend class basic_fields_base;
-
-    const_iterator(iter_type it)
-        : it_(it)
-    {
-    }
-
-public:
-    using value_type =
-        typename basic_fields_base::value_type;
-    using pointer = value_type const*;
-    using reference = value_type const&;
-    using difference_type = std::ptrdiff_t;
-    using iterator_category =
-        std::bidirectional_iterator_tag;
-
-    const_iterator() = default;
-    const_iterator(const_iterator&& other) = default;
-    const_iterator(const_iterator const& other) = default;
-    const_iterator& operator=(const_iterator&& other) = default;
-    const_iterator& operator=(const_iterator const& other) = default;
-
-    bool
-    operator==(const_iterator const& other) const
-    {
-        return it_ == other.it_;
-    }
-
-    bool
-    operator!=(const_iterator const& other) const
-    {
-        return !(*this == other);
-    }
-
-    reference
-    operator*() const
-    {
-        return it_->data;
-    }
-
-    pointer
-    operator->() const
-    {
-        return &**this;
-    }
-
-    const_iterator&
-    operator++()
-    {
-        ++it_;
-        return *this;
-    }
-
-    const_iterator
-    operator++(int)
-    {
-        auto temp = *this;
-        ++(*this);
-        return temp;
-    }
-
-    const_iterator&
-    operator--()
-    {
-        --it_;
-        return *this;
-    }
-
-    const_iterator
-    operator--(int)
-    {
-        auto temp = *this;
-        --(*this);
-        return temp;
-    }
-};
-
-} // detail
-} // http
-} // beast
-
-#endif

include/beast/http/fields.hpp

@@ -10,11 +10,13 @@
 
 #include <beast/config.hpp>
 #include <beast/core/string_view.hpp>
-#include <beast/core/detail/empty_base_optimization.hpp>
+#include <beast/core/detail/ci_char_traits.hpp>
-#include <beast/http/detail/fields.hpp>
 #include <boost/lexical_cast.hpp>
+#include <boost/intrusive/list.hpp>
+#include <boost/intrusive/set.hpp>
 #include <algorithm>
 #include <cctype>
+#include <cstring>
 #include <memory>
 #include <string>
 #include <type_traits>
@@ -39,70 +41,44 @@ namespace http {
     @note Meets the requirements of @b FieldSequence.
 */
 template<class Allocator>
-class basic_fields :
+class basic_fields
-#if ! BEAST_DOXYGEN
-    private beast::detail::empty_base_optimization<
-        typename std::allocator_traits<Allocator>::
-            template rebind_alloc<
-                detail::basic_fields_base::element>>,
-#endif
-    public detail::basic_fields_base
 {
-    using alloc_type = typename
+private:
-        std::allocator_traits<Allocator>::
+    using off_t = std::uint16_t;
-            template rebind_alloc<
-                detail::basic_fields_base::element>;
 
-    using alloc_traits =
+public:
-        std::allocator_traits<alloc_type>;
+    /** The value type of the field sequence.
 
-    using size_type =
+        Meets the requirements of @b Field.
-        typename std::allocator_traits<Allocator>::size_type;
+    */
-
+    struct value_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<class FieldSequence>
-    void
-    copy_from(FieldSequence const& fs)
     {
-        for(auto const& e : fs)
+        string_view
-            insert(e.first, e.second);
+        name() const
+        {
+            return {first, off - 2u};
         }
 
+        string_view
+        value() const
+        {
+            return {first + off, len};
+        }
+
+        char const* first;
+        off_t off;
+        off_t len;
+    };
+
 public:
     /// The type of allocator used.
     using allocator_type = Allocator;
 
-    /** The value type of the field sequence.
+    /// A constant iterator to the field sequence.
+    class const_iterator;
 
-        Meets the requirements of @b Field.
+    /// A constant iterator to the field sequence.
-    */
+    using iterator = const_iterator;
-#if BEAST_DOXYGEN
-    using value_type = implementation_defined;
-#endif
-
-    /// A const iterator to the field sequence
-#if BEAST_DOXYGEN
-    using iterator = implementation_defined;
-#endif
-
-    /// A const iterator to the field sequence
-#if BEAST_DOXYGEN
-    using const_iterator = implementation_defined;
-#endif
 
     /// Default constructor.
     basic_fields() = default;
@@ -147,42 +123,51 @@ public:
     template<class OtherAlloc>
     basic_fields& operator=(basic_fields<OtherAlloc> const&);
 
-    /// Returns a const iterator to the beginning of the field sequence.
+    /// Return a copy of the allocator associated with the container.
+    allocator_type
+    get_allocator() const
+    {
+        return typename std::allocator_traits<
+            Allocator>::template rebind_alloc<
+                element>(alloc_);
+    }
+
+    /// Return a const iterator to the beginning of the field sequence.
     const_iterator
     begin() const
     {
         return list_.cbegin();
     }
 
-    /// Returns a const iterator to the end of the field sequence.
+    /// Return a const iterator to the end of the field sequence.
     const_iterator
     end() const
     {
         return list_.cend();
     }
 
-    /// Returns a const iterator to the beginning of the field sequence.
+    /// Return a const iterator to the beginning of the field sequence.
     const_iterator
     cbegin() const
     {
         return list_.cbegin();
     }
 
-    /// Returns a const iterator to the end of the field sequence.
+    /// Return a const iterator to the end of the field sequence.
     const_iterator
     cend() const
     {
         return list_.cend();
     }
 
-    /// Returns `true` if the specified field exists.
+    /// Return `true` if the specified field exists.
     bool
     exists(string_view name) const
     {
         return set_.find(name, less{}) != set_.end();
     }
 
-    /// Returns the number of values for the specified field.
+    /// Return the number of values for the specified field.
     std::size_t
     count(string_view name) const;
 
@@ -247,6 +232,7 @@ public:
         ! std::is_constructible<string_view, T>::value>::type
     insert(string_view name, T const& value)
     {
+        // VFALCO This should use a static buffer, see lexical_cast doc
         insert(name, boost::lexical_cast<std::string>(value));
     }
 
@@ -277,6 +263,7 @@ public:
         ! std::is_constructible<string_view, T>::value>::type
     replace(string_view name, T const& value)
     {
+        // VFALCO This should use a static buffer, see lexical_cast doc
         replace(name,
             boost::lexical_cast<std::string>(value));
     }
@@ -329,6 +316,111 @@ private:
         else
             this->replace(":reason", s);
     }
+
+private:
+    struct element
+        : boost::intrusive::set_base_hook <
+            boost::intrusive::link_mode <
+                boost::intrusive::normal_link>>
+        , boost::intrusive::list_base_hook <
+            boost::intrusive::link_mode <
+                boost::intrusive::normal_link>>
+    {
+        element(
+            string_view name, string_view value)
+        {
+            char* p = reinterpret_cast<char*>(this + 1);
+            data.first = p;
+            data.off =
+                static_cast<off_t>(name.size() + 2);
+            data.len =
+                static_cast<off_t>(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';
+        }
+
+        value_type data;
+    };
+
+    struct less : private beast::detail::ci_less
+    {
+        template<class String>
+        bool
+        operator()(String const& lhs, element const& rhs) const
+        {
+            return ci_less::operator()(lhs, rhs.data.name());
+        }
+
+        template<class String>
+        bool
+        operator()(element const& lhs, String const& rhs) const
+        {
+            return ci_less::operator()(lhs.data.name(), rhs);
+        }
+
+        bool
+        operator()(element const& lhs, element const& rhs) const
+        {
+            return ci_less::operator()(
+                lhs.data.name(), rhs.data.name());
+        }
+    };
+
+    using alloc_type = typename
+        std::allocator_traits<Allocator>::
+            template rebind_alloc<element>;
+
+    using alloc_traits =
+        std::allocator_traits<alloc_type>;
+
+    using size_type =
+        typename std::allocator_traits<Allocator>::size_type;
+
+    using list_t = typename boost::intrusive::make_list<
+        element, boost::intrusive::constant_time_size<
+            false>>::type;
+
+    using set_t = typename boost::intrusive::make_multiset<
+        element, boost::intrusive::constant_time_size<
+            true>, boost::intrusive::compare<less>>::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<class FieldSequence>
+    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);
+
+    set_t set_;
+    list_t list_;
+    alloc_type alloc_;
 };
 
 /// A typical HTTP header fields container

include/beast/http/impl/fields.ipp

@@ -9,23 +9,116 @@
 #define BEAST_HTTP_IMPL_FIELDS_IPP
 
 #include <beast/http/detail/rfc7230.hpp>
+#include <boost/throw_exception.hpp>
 #include <algorithm>
 
 namespace beast {
 namespace http {
 
+//------------------------------------------------------------------------------
+
+template<class Allocator>
+basic_fields<Allocator>::
+~basic_fields()
+{
+    delete_all();
+}
+
+//------------------------------------------------------------------------------
+
+template<class Allocator>
+class basic_fields<Allocator>::
+    const_iterator
+{
+    using iter_type = typename list_t::const_iterator;
+
+    iter_type it_;
+
+    template<class Alloc>
+    friend class beast::http::basic_fields;
+
+    const_iterator(iter_type it)
+        : it_(it)
+    {
+    }
+
+public:
+    using value_type = typename
+        basic_fields<Allocator>::value_type;
+    using pointer = value_type const*;
+    using reference = value_type const&;
+    using difference_type = std::ptrdiff_t;
+    using iterator_category =
+        std::bidirectional_iterator_tag;
+
+    const_iterator() = default;
+    const_iterator(const_iterator&& other) = default;
+    const_iterator(const_iterator const& other) = default;
+    const_iterator& operator=(const_iterator&& other) = default;
+    const_iterator& operator=(const_iterator const& other) = default;
+
+    bool
+    operator==(const_iterator const& other) const
+    {
+        return it_ == other.it_;
+    }
+
+    bool
+    operator!=(const_iterator const& other) const
+    {
+        return !(*this == other);
+    }
+
+    reference
+    operator*() const
+    {
+        return it_->data;
+    }
+
+    pointer
+    operator->() const
+    {
+        return &**this;
+    }
+
+    const_iterator&
+    operator++()
+    {
+        ++it_;
+        return *this;
+    }
+
+    const_iterator
+    operator++(int)
+    {
+        auto temp = *this;
+        ++(*this);
+        return temp;
+    }
+
+    const_iterator&
+    operator--()
+    {
+        --it_;
+        return *this;
+    }
+
+    const_iterator
+    operator--(int)
+    {
+        auto temp = *this;
+        --(*this);
+        return temp;
+    }
+};
+
 template<class Allocator>
 void
 basic_fields<Allocator>::
 delete_all()
 {
     for(auto it = list_.begin(); it != list_.end();)
-    {
+        delete_element(*it++);
-        auto& e = *it++;
-        alloc_traits::destroy(this->member(), &e);
-        alloc_traits::deallocate(
-            this->member(), &e, 1);
-    }
 }
 
 template<class Allocator>
@@ -34,7 +127,7 @@ void
 basic_fields<Allocator>::
 move_assign(basic_fields& other, std::false_type)
 {
-    if(this->member() != other.member())
+    if(alloc_ != other.alloc_)
     {
         copy_from(other);
         other.clear();
@@ -52,7 +145,7 @@ void
 basic_fields<Allocator>::
 move_assign(basic_fields& other, std::true_type)
 {
-    this->member() = std::move(other.member());
+    alloc_ = std::move(other.alloc_);
     set_ = std::move(other.set_);
     list_ = std::move(other.list_);
 }
@@ -72,34 +165,63 @@ void
 basic_fields<Allocator>::
 copy_assign(basic_fields const& other, std::true_type)
 {
-    this->member() = other.member();
+    alloc_ = other.alloc_;
     copy_from(other);
 }
 
+template<class Allocator>
+auto
+basic_fields<Allocator>::
+new_element(string_view name, string_view value) ->
+    element&
+{
+    if(name.size() + 2 >
+            (std::numeric_limits<off_t>::max)())
+        BOOST_THROW_EXCEPTION(std::length_error{
+            "field name too large"});
+    if(value.size() + 2 >
+            (std::numeric_limits<off_t>::max)())
+        BOOST_THROW_EXCEPTION(std::length_error{
+            "field value too large"});
+    value = detail::trim(value);
+    std::uint16_t const off =
+        static_cast<off_t>(name.size() + 2);
+    std::uint16_t const len =
+        static_cast<off_t>(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<class Allocator>
+void
+basic_fields<Allocator>::
+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<class Allocator>
-basic_fields<Allocator>::
-~basic_fields()
-{
-    delete_all();
-}
-
 template<class Allocator>
 basic_fields<Allocator>::
 basic_fields(Allocator const& alloc)
-    : beast::detail::empty_base_optimization<
+    : alloc_(alloc)
-        alloc_type>(alloc)
 {
 }
 
 template<class Allocator>
 basic_fields<Allocator>::
 basic_fields(basic_fields&& other)
-    : beast::detail::empty_base_optimization<alloc_type>(
+    : set_(std::move(other.set_))
-        std::move(other.member()))
+    , list_(std::move(other.list_))
-    , detail::basic_fields_base(
+    , alloc_(std::move(other.alloc_))
-        std::move(other.set_), std::move(other.list_))
 {
 }
 
@@ -121,7 +243,7 @@ template<class Allocator>
 basic_fields<Allocator>::
 basic_fields(basic_fields const& other)
     : basic_fields(alloc_traits::
-        select_on_container_copy_construction(other.member()))
+        select_on_container_copy_construction(other.alloc_))
 {
     copy_from(other);
 }
@@ -190,7 +312,7 @@ operator[](string_view name) const
     auto const it = find(name);
     if(it == end())
         return {};
-    return it->second;
+    return it->value();
 }
 
 template<class Allocator>
@@ -218,8 +340,7 @@ erase(string_view name)
         auto& e = *it++;
         set_.erase(set_.iterator_to(e));
         list_.erase(list_.iterator_to(e));
-        alloc_traits::destroy(this->member(), &e);
+        delete_element(e);
-        alloc_traits::deallocate(this->member(), &e, 1);
         if(it == last)
             break;
         ++n;
@@ -232,11 +353,9 @@ void
 basic_fields<Allocator>::
 insert(string_view name, string_view value)
 {
-    value = detail::trim(value);
+    auto& e = new_element(name, value);
-    auto const p = alloc_traits::allocate(this->member(), 1);
+    set_.insert_before(set_.upper_bound(name, less{}), e);
-    alloc_traits::construct(this->member(), p, name, value);
+    list_.push_back(e);
-    set_.insert_before(set_.upper_bound(name, less{}), *p);
-    list_.push_back(*p);
 }
 
 template<class Allocator>

test/http/fields.cpp

@@ -14,7 +14,7 @@
 namespace beast {
 namespace http {
 
-class basic_fields_test : public beast::unit_test::suite
+class fields_test : public beast::unit_test::suite
 {
 public:
     template<class Allocator>
@@ -120,7 +120,7 @@ public:
     }
 };
 
-BEAST_DEFINE_TESTSUITE(basic_fields,http,beast);
+BEAST_DEFINE_TESTSUITE(fields,http,beast);
 
 } // http
 } // beast