From 0e014e86ac045a43b8cf9da6e7a16228f13cef9b Mon Sep 17 00:00:00 2001 From: alandefreitas Date: Wed, 26 Jan 2022 19:56:41 -0300 Subject: [PATCH] const_string small buffer close #118 --- include/boost/url/const_string.hpp | 35 +++- include/boost/url/impl/const_string.hpp | 36 +++-- include/boost/url/impl/const_string.ipp | 74 +++++---- test/unit/const_string.cpp | 207 +++++++++++++++++++----- test/unit/static_pool.cpp | 2 +- 5 files changed, 269 insertions(+), 85 deletions(-) diff --git a/include/boost/url/const_string.hpp b/include/boost/url/const_string.hpp index d4ecbd1c..2cffa8a2 100644 --- a/include/boost/url/const_string.hpp +++ b/include/boost/url/const_string.hpp @@ -42,10 +42,43 @@ namespace urls { */ class const_string : public string_view { +public: + /** Built-in capacity + + Strings whose length is up to + this size will not require + dynamic allocation. + */ + static constexpr + std::size_t + builtin_capacity = 32; + +private: struct base; struct result; - base* p_; + union data_t + { + base* p_; + char buf_[builtin_capacity]; + + data_t() = default; + + explicit + data_t(base* p) + : p_(p) + { + } + }; + + data_t data_; + + static constexpr + bool + is_small(std::size_t n) noexcept + { + return n <= builtin_capacity; + } BOOST_URL_DECL explicit diff --git a/include/boost/url/impl/const_string.hpp b/include/boost/url/impl/const_string.hpp index 95a4a1ba..1d2823c1 100644 --- a/include/boost/url/impl/const_string.hpp +++ b/include/boost/url/impl/const_string.hpp @@ -119,6 +119,7 @@ public: result construct(std::size_t size) const override { + // VFALCO Should we do something special when size==0? detail::over_allocator< string, Allocator> a(size, a_); auto p = ::new(a.allocate(1)) string(a_); @@ -153,6 +154,10 @@ operator()( std::size_t n, InitFn const& init) const { + // VFALCO Should we do something special when n==0? + if (is_small(n)) + return const_string(n, + std::allocator{}, init); auto r = p_->construct(n); const_string s(r); init(n, r.data); @@ -167,14 +172,21 @@ const_string( string_view s, Allocator const& a) { - auto r = factory::impl< - Allocator>(a).construct( - s.size()); + // VFALCO Should we do something special when n==0? + if (is_small(s.size())) + { + std::memcpy(data_.buf_, + s.data(), s.size()); + static_cast(*this + ) = { data_.buf_, s.size()}; + return; + } + auto r = factory::impl( + a).construct(s.size()); static_cast( *this) = { r.data, r.size }; - std::memcpy( - r.data, s.data(), s.size()); - p_ = r.p; + std::memcpy(r.data, s.data(), s.size()); + data_.p_ = r.p; } template< @@ -186,13 +198,19 @@ const_string( Allocator const& a, InitFn const& init) { + if (is_small(size)) + { + init(size, data_.buf_); + static_cast( + *this) = { data_.buf_, size }; + return; + } auto r = factory::impl< - Allocator>(a).construct( - size); + Allocator>(a).construct(size); static_cast( *this) = { r.data, r.size }; init(size, r.data); - p_ = r.p; + data_.p_ = r.p; } } // urls diff --git a/include/boost/url/impl/const_string.ipp b/include/boost/url/impl/const_string.ipp index 6d1982e7..62a01f49 100644 --- a/include/boost/url/impl/const_string.ipp +++ b/include/boost/url/impl/const_string.ipp @@ -127,58 +127,43 @@ operator()(string_view s) const const_string:: const_string( result const& r) noexcept - : string_view( - r.data, r.size) - , p_(r.p) + : string_view(r.data, r.size) + , data_(r.p) { } const_string:: ~const_string() { - p_->release(size()); + if (!is_small(size())) + data_.p_->release(size()); } const_string:: const_string() noexcept : string_view() - , p_([] - { - struct empty_string final : base - { - void - destroy(std::size_t) noexcept override - { - } - }; - - struct deleter - { - empty_string* p; - - ~deleter() - { - p->release(0); - } - }; - - static empty_string cs{}; - static deleter d{&cs}; - - ++cs.refs; - - return &cs; - }()) { } const_string:: const_string( const_string const& other) noexcept - : string_view(other) - , p_(other.p_) + : string_view() { - ++p_->refs; + if (is_small(other.size())) + { + // other is small, + // nothing to release + std::memcpy( data_.buf_, + other.data_.buf_, other.size()); + static_cast(*this) = + string_view(data_.buf_, other.size()); + return; + } + data_.p_ = other.data_.p_; + ++data_.p_->refs; + static_cast( + *this) = string_view(other); } const_string& @@ -186,11 +171,24 @@ const_string:: operator=( const_string const& other) noexcept { - ++other.p_->refs; - p_->release(size()); - p_ = other.p_; + if (is_small(other.size())) + { + if (!is_small(size())) + data_.p_->release(size()); + else if(this == &other) + return *this; + std::memcpy(data_.buf_, + other.data_.buf_, other.size()); + static_cast(*this) = + string_view(data_.buf_, other.size()); + return *this; + } + ++other.data_.p_->refs; + if (!is_small(size())) + data_.p_->release(size()); + data_.p_ = other.data_.p_; static_cast( - *this) = other; + *this) = string_view(other); return *this; } diff --git a/test/unit/const_string.cpp b/test/unit/const_string.cpp index 11147398..fe671410 100644 --- a/test/unit/const_string.cpp +++ b/test/unit/const_string.cpp @@ -21,6 +21,27 @@ struct const_string_test { using A = std::allocator; + string_view big_; + string_view big2_; + string_view small_; + static constexpr std::size_t C = + const_string::builtin_capacity; + + const_string_test() noexcept + { + string_view cs = + "*" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "abcdefghijklmnopqrstuvwxyz"; + BOOST_TEST(cs.size() > C); + big_ = cs.substr(1); + big2_ = cs; + small_ = big_.substr(0, C); + BOOST_TEST(big_.size() > C); + BOOST_TEST(small_.size() <= C); + BOOST_TEST(C > 3); + } + void testConstString() { @@ -32,6 +53,7 @@ struct const_string_test BOOST_TEST(s.empty()); // same buffer + BOOST_TEST(s.size() <= C); BOOST_TEST( const_string().data() == const_string().data()); @@ -40,15 +62,12 @@ struct const_string_test // const_string(string_view, InitFn) // ~const_string() { - const_string s(3, A{}, - [](std::size_t, char* dest) + const_string s(small_.size(), A{}, + [&](std::size_t n, char* dest) { - dest[0] = 'a'; - dest[1] = 'b'; - dest[2] = 'c'; + small_.copy(dest, n, 0); }); - BOOST_TEST(s == "abc"); - BOOST_TEST(! s.empty()); + BOOST_TEST(s == small_); } // const_string(string_view, Allocator) @@ -60,34 +79,121 @@ struct const_string_test BOOST_TEST(! s.empty()); // different buffer + BOOST_TEST(s.size() <= C); BOOST_TEST(s.data() != x); } // const_string(const_string const&) { - const_string s1("abc", A{}); - const_string s2(s1); - BOOST_TEST(s1 == "abc"); - BOOST_TEST(s2 == "abc"); - BOOST_TEST(s2 == s1); + // small buffer + { + auto c = small_; + const_string s1(c, A{}); + const_string s2(s1); + BOOST_TEST(s1 == c); + BOOST_TEST(s2 == c); + BOOST_TEST(s2 == s1); - // same buffer - BOOST_TEST(s1.data() == s2.data()); + // different buffer + BOOST_TEST(s1.size() <= C); + BOOST_TEST(s1.data() != s2.data()); + } + + // big buffer + { + const_string s1(big_, A{}); + const_string s2(s1); + BOOST_TEST(s1 == big_); + BOOST_TEST(s2 == big_); + BOOST_TEST(s2 == s1); + + // same buffer + BOOST_TEST(s1.size() > C); + BOOST_TEST(s1.data() == s2.data()); + } } // operator=(const_string const&) { - const_string s1("abc", A{}); - const_string s2; - BOOST_TEST(s1 == "abc"); - BOOST_TEST(s2.empty()); - s2 = s1; - BOOST_TEST(s1 == "abc"); - BOOST_TEST(s2 == "abc"); - BOOST_TEST(s2 == s1); + // small + { + const_string s1("abc", A{}); + const_string s2; + BOOST_TEST(s1 == "abc"); + BOOST_TEST(s2.empty()); + s2 = s1; + BOOST_TEST(s1 == "abc"); + BOOST_TEST(s2 == "abc"); + BOOST_TEST(s2 == s1); - // same buffer - BOOST_TEST(s1.data() == s2.data()); + // different buffer + BOOST_TEST(s1.size() <= C); + BOOST_TEST(s1.data() != s2.data()); + } + + // big + { + const_string s1(big_, A{}); + const_string s2; + BOOST_TEST(s1 == big_); + BOOST_TEST(s2.empty()); + s2 = s1; + BOOST_TEST(s1 == big_); + BOOST_TEST(s2 == big_); + BOOST_TEST(s2 == s1); + + // same buffer + BOOST_TEST(s1.size() > C); + BOOST_TEST(s1.data() == s2.data()); + } + + // big + { + const_string s1(small_, A{}); + const_string s2(big_, A{}); + BOOST_TEST(s1 == small_); + BOOST_TEST(s2 == big_); + s1 = s2; + BOOST_TEST(s1 == big_); + BOOST_TEST(s2 == big_); + BOOST_TEST(s2 == s1); + + // same buffer + BOOST_TEST(s1.size() > C); + BOOST_TEST(s1.data() == s2.data()); + } + + // big + { + const_string s1(big_, A{}); + const_string s2(big2_, A{}); + BOOST_TEST(s1 == big_); + BOOST_TEST(s2 == big2_); + s1 = s2; + BOOST_TEST(s1 == big2_); + BOOST_TEST(s2 == big2_); + BOOST_TEST(s2 == s1); + + // same buffer + BOOST_TEST(s1.size() > C); + BOOST_TEST(s1.data() == s2.data()); + } + + // operator= (coverage) + { + const_string s1(big_, A{}); + const_string s2(small_, A{}); + s1 = s2; + BOOST_TEST(s1 == s2); + } + + // operator= (self-assignment) + { + const_string s(small_, A{}); + const_string const& cs(s); + s = cs; + BOOST_TEST(s == small_); + } } } @@ -121,15 +227,28 @@ struct const_string_test const_string s; { factory a; - s = a(3, - [](std::size_t, char* dest) + s = a(C, + [this]( + std::size_t n, + char* dest) { - dest[0] = 'a'; - dest[1] = 'b'; - dest[2] = 'c'; + small_.copy(dest, n, 0); }); } - BOOST_TEST(s == "abc"); + BOOST_TEST(s == small_); + BOOST_TEST(! s.empty()); + } + { + const_string s; + { + factory a; + s = a(big_.size(), + [this](std::size_t n, char* dest) + { + big_.copy(dest, n, 0); + }); + } + BOOST_TEST(s == big_); BOOST_TEST(! s.empty()); } { @@ -137,15 +256,31 @@ struct const_string_test const_string s; { factory a(sp.allocator()); - s = a(3, - [](std::size_t, char* dest) + s = a(C, + [this]( + std::size_t n, + char* dest) { - dest[0] = 'a'; - dest[1] = 'b'; - dest[2] = 'c'; + small_.copy(dest, n, 0); }); } - BOOST_TEST(s == "abc"); + BOOST_TEST(s == small_); + BOOST_TEST(! s.empty()); + } + { + static_pool<1000> sp; + const_string s; + { + factory a(sp.allocator()); + s = a(big_.size(), + [this]( + std::size_t n, + char* dest) + { + big_.copy(dest, n, 0); + }); + } + BOOST_TEST(s == big_); BOOST_TEST(! s.empty()); } } diff --git a/test/unit/static_pool.cpp b/test/unit/static_pool.cpp index 0aa7c8a5..2e6a9b47 100644 --- a/test/unit/static_pool.cpp +++ b/test/unit/static_pool.cpp @@ -36,7 +36,7 @@ public: run() { string_view s; - s = "abcdefghijklmnopqrstuvwxyz"; + s = "abcdefghijklmnopqrstuvwxyzabcdefghijk"; { std::string s0; BOOST_TEST(s0.capacity() <