diff --git a/CHANGELOG.md b/CHANGELOG.md index 30b3a653..25663218 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ Version 259: * Reduce the number of instantiations of filter_token_list +* Remove the use of `static_string` from `http::fields` -------------------------------------------------------------------------------- diff --git a/include/boost/beast/http/fields.hpp b/include/boost/beast/http/fields.hpp index 927191d7..f891acb1 100644 --- a/include/boost/beast/http/fields.hpp +++ b/include/boost/beast/http/fields.hpp @@ -62,8 +62,6 @@ class basic_fields friend class fields_test; // for `header` - static std::size_t constexpr max_static_buffer = 4096; - struct element; using off_t = std::uint16_t; @@ -72,7 +70,7 @@ public: /// The type of allocator used. using allocator_type = Allocator; - /// The type of element used to represent a field + /// The type of element used to represent a field class value_type { friend class basic_fields; diff --git a/include/boost/beast/http/impl/fields.hpp b/include/boost/beast/http/impl/fields.hpp index 268f523d..28aac84d 100644 --- a/include/boost/beast/http/impl/fields.hpp +++ b/include/boost/beast/http/impl/fields.hpp @@ -12,7 +12,6 @@ #include #include -#include #include #include #include @@ -751,12 +750,99 @@ equal_range(string_view name) const -> namespace detail { +struct temporary_buffer +{ + temporary_buffer() = default; + + temporary_buffer(temporary_buffer const&) = delete; + temporary_buffer(temporary_buffer&&) = delete; + + temporary_buffer& operator=(temporary_buffer const&) = delete; + temporary_buffer& operator=(temporary_buffer&&) = delete; + + ~temporary_buffer() noexcept + { + deallocate(data_); + } + + void append(string_view sv) + { + reserve(sv.size()); + unsafe_append(sv); + } + + void append(string_view sv1, string_view sv2) + { + reserve(sv1.size() + sv2.size()); + unsafe_append(sv1); + unsafe_append(sv2); + } + + string_view view() const noexcept + { + return {data_, size_}; + } + + std::size_t size() const noexcept + { + return size_; + } + + std::size_t capacity() const noexcept + { + return capacity_; + } + + bool empty() const noexcept + { + return size_ == 0; + } + +private: + void unsafe_append(string_view sv) + { + auto n = sv.size(); + std::memcpy(&data_[size_], sv.data(), n); + size_ += n; + } + + void reserve(std::size_t sv_size) + { + if (capacity_ - size_ < sv_size) + { + auto constexpr limit = (std::numeric_limits::max)(); + if (limit/2 < capacity_ || limit - size_ < sv_size) + { + BOOST_THROW_EXCEPTION( + std::length_error{"temporary_buffer::append"}); + } + + auto const new_cap = (std::max)(size_ + sv_size, capacity_*2); + char* const p = new char[new_cap]; + std::memcpy(p, data_, size_); + deallocate(boost::exchange(data_, p)); + capacity_ = new_cap; + } + } + + void deallocate(char* data) noexcept + { + if (data != buffer_) + delete[] data; + } + + char buffer_[4096]; + char* data_ = buffer_; + std::size_t capacity_ = sizeof(buffer_); + std::size_t size_ = 0; +}; + // Filter a token list // -template +template void filter_token_list( - String& s, + detail::temporary_buffer& s, string_view value, Pred&& pred) { @@ -768,22 +854,21 @@ filter_token_list( while(pred(*it)) if(++it == last) return; - s.append(it->data(), it->size()); + s.append(*it); while(++it != last) { if(! pred(*it)) { - s.append(", "); - s.append(it->data(), it->size()); + s.append(", ", *it); } } } // Filter the last item in a token list -template +template void filter_token_list_last( - String& s, + detail::temporary_buffer& s, string_view value, Pred&& pred) { @@ -795,10 +880,10 @@ filter_token_list_last( if(next == te.end()) { if(! pred(*it)) - s.append(it->data(), it->size()); + s.append(*it); return; } - s.append(it->data(), it->size()); + s.append(*it); for(;;) { it = next; @@ -807,34 +892,32 @@ filter_token_list_last( { if(! pred(*it)) { - s.append(", "); - s.append(it->data(), it->size()); + s.append(", ", *it); } return; } - s.append(", "); - s.append(it->data(), it->size()); + s.append(", ", *it); } } } -template +struct iequals_predicate +{ + bool operator()(string_view s) + { + return beast::iequals(s, sv1) || beast::iequals(s, sv2); + } + + string_view sv1; + string_view sv2; +}; + +inline void keep_alive_impl( - String& s, string_view value, + detail::temporary_buffer& s, string_view value, unsigned version, bool keep_alive) { - struct iequals_predicate - { - bool operator()(string_view s) - { - return beast::iequals(s, sv1) || beast::iequals(s, sv2); - } - - string_view sv1; - string_view sv2; - }; - if(version < 11) { if(keep_alive) @@ -990,6 +1073,7 @@ void basic_fields:: set_chunked_impl(bool value) { + detail::temporary_buffer buf; auto it = find(field::transfer_encoding); if(value) { @@ -1011,78 +1095,21 @@ set_chunked_impl(bool value) } itt = next; } - static_string buf; - if(! beast::detail::sum_exceeds( - it->value().size(), 9u, buf.max_size())) - { - buf.append(it->value().data(), it->value().size()); - buf.append(", chunked", 9); - set(field::transfer_encoding, buf); - } - else - { - #ifdef BOOST_BEAST_HTTP_NO_FIELDS_BASIC_STRING_ALLOCATOR - // Workaround for https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56437 - std::string s; - #else - using A = - typename beast::detail::allocator_traits< - Allocator>::template rebind_alloc; - std::basic_string< - char, - std::char_traits, - A> s{A{this->get()}}; - #endif - s.reserve(it->value().size() + 9); - s.append(it->value().data(), it->value().size()); - s.append(", chunked", 9); - set(field::transfer_encoding, s); - } + + buf.append(it->value(), ", chunked"); + set(field::transfer_encoding, buf.view()); return; } // filter "chunked" if(it == end()) return; -#ifndef BOOST_NO_EXCEPTIONS - try - { - static_string buf; - detail::filter_token_list_last(buf, it->value(), - [](string_view s) - { - return iequals(s, "chunked"); - }); - if(! buf.empty()) - set(field::transfer_encoding, buf); - else - erase(field::transfer_encoding); - } - catch(std::length_error const&) -#endif - { - #ifdef BOOST_BEAST_HTTP_NO_FIELDS_BASIC_STRING_ALLOCATOR - // Workaround for https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56437 - std::string s; - #else - using A = - typename beast::detail::allocator_traits< - Allocator>::template rebind_alloc; - std::basic_string< - char, - std::char_traits, - A> s{A{this->get()}}; - #endif - s.reserve(it->value().size()); - detail::filter_token_list_last(s, it->value(), - [](string_view s) - { - return iequals(s, "chunked"); - }); - if(! s.empty()) - set(field::transfer_encoding, s); - else - erase(field::transfer_encoding); - } + + detail::filter_token_list_last(buf, it->value(), + detail::iequals_predicate{"chunked"}); + if(! buf.empty()) + set(field::transfer_encoding, buf.view()); + else + erase(field::transfer_encoding); } template @@ -1105,40 +1132,12 @@ set_keep_alive_impl( { // VFALCO What about Proxy-Connection ? auto const value = (*this)[field::connection]; -#ifndef BOOST_NO_EXCEPTIONS - try - { - static_string buf; - detail::keep_alive_impl( - buf, value, version, keep_alive); - if(buf.empty()) - erase(field::connection); - else - set(field::connection, buf); - } - catch(std::length_error const&) -#endif - { - #ifdef BOOST_BEAST_HTTP_NO_FIELDS_BASIC_STRING_ALLOCATOR - // Workaround for https://gcc.gnu.org/bugzilla/show_bug.cgi?id=56437 - std::string s; - #else - using A = - typename beast::detail::allocator_traits< - Allocator>::template rebind_alloc; - std::basic_string< - char, - std::char_traits, - A> s{A{this->get()}}; - #endif - s.reserve(value.size()); - detail::keep_alive_impl( - s, value, version, keep_alive); - if(s.empty()) - erase(field::connection); - else - set(field::connection, s); - } + detail::temporary_buffer buf; + detail::keep_alive_impl(buf, value, version, keep_alive); + if(buf.empty()) + erase(field::connection); + else + set(field::connection, buf.view()); } //------------------------------------------------------------------------------ diff --git a/test/beast/http/fields.cpp b/test/beast/http/fields.cpp index 4eaaedd3..c6f45468 100644 --- a/test/beast/http/fields.cpp +++ b/test/beast/http/fields.cpp @@ -24,6 +24,9 @@ namespace http { class fields_test : public beast::unit_test::suite { public: + static constexpr std::size_t max_static_buffer = + sizeof(http::detail::temporary_buffer); + template class test_allocator { @@ -685,8 +688,7 @@ public: (! res.keep_alive() && ! v)); }; - BOOST_STATIC_ASSERT(fields::max_static_buffer == 4096); - std::string const big(4096 + 1, 'a'); + std::string const big(max_static_buffer + 1, 'a'); // HTTP/1.0 res.version(10); @@ -846,10 +848,10 @@ public: res.content_length(0); BEAST_EXPECT(res[field::content_length] == "0"); - + res.content_length(100); BEAST_EXPECT(res[field::content_length] == "100"); - + res.content_length(boost::none); BEAST_EXPECT(res.count(field::content_length) == 0); @@ -857,12 +859,12 @@ public: res.content_length(0); BEAST_EXPECT(res[field::content_length] == "0"); BEAST_EXPECT(res.count(field::transfer_encoding) == 0); - + res.set(field::transfer_encoding, "chunked"); res.content_length(100); BEAST_EXPECT(res[field::content_length] == "100"); BEAST_EXPECT(res.count(field::transfer_encoding) == 0); - + res.set(field::transfer_encoding, "chunked"); res.content_length(boost::none); BEAST_EXPECT(res.count(field::content_length) == 0); @@ -874,12 +876,12 @@ public: res.content_length(0); BEAST_EXPECT(res[field::content_length] == "0"); BEAST_EXPECT(res[field::transfer_encoding] == s); - + res.set(field::transfer_encoding, s); res.content_length(100); BEAST_EXPECT(res[field::content_length] == "100"); BEAST_EXPECT(res[field::transfer_encoding] == s); - + res.set(field::transfer_encoding, s); res.content_length(boost::none); BEAST_EXPECT(res.count(field::content_length) == 0); @@ -889,12 +891,12 @@ public: res.content_length(0); BEAST_EXPECT(res[field::content_length] == "0"); BEAST_EXPECT(res[field::transfer_encoding] == s); - + res.set(field::transfer_encoding, s + ", chunked"); res.content_length(100); BEAST_EXPECT(res[field::content_length] == "100"); BEAST_EXPECT(res[field::transfer_encoding] == s); - + res.set(field::transfer_encoding, s + ", chunked"); res.content_length(boost::none); BEAST_EXPECT(res.count(field::content_length) == 0); @@ -904,12 +906,12 @@ public: res.content_length(0); BEAST_EXPECT(res[field::content_length] == "0"); BEAST_EXPECT(res[field::transfer_encoding] == "chunked, " + s); - + res.set(field::transfer_encoding, "chunked, " + s); res.content_length(100); BEAST_EXPECT(res[field::content_length] == "100"); BEAST_EXPECT(res[field::transfer_encoding] == "chunked, " + s); - + res.set(field::transfer_encoding, "chunked, " + s); res.content_length(boost::none); BEAST_EXPECT(res.count(field::content_length) == 0); @@ -918,8 +920,7 @@ public: check("foo"); - BOOST_STATIC_ASSERT(fields::max_static_buffer == 4096); - std::string const big(4096 + 1, 'a'); + std::string const big(max_static_buffer + 1, 'a'); check(big); }