From 7952578e1d2b0bf867eb6c7fcda7a955d34219b7 Mon Sep 17 00:00:00 2001 From: Vinnie Falco Date: Wed, 7 Jun 2017 17:41:27 -0700 Subject: [PATCH] Tune up static_buffer (API Change): The static buffer is updated: * reset() is no longer a member. Use b.consume(b.size()) instead. * Simplified implementaton, uses asio instead of custom types * Better stream performance: consuming the input makes room available in the output. This class is now suitable for HTTP reads. These changes permit the static_buffer wrapper to adapt a user memory buffer if desired, including a stack based array. The static_buffer_n class may also be used for this purpose, it comes with its own storage. --- CHANGELOG.md | 4 + include/beast/core/flat_buffer.hpp | 4 +- include/beast/core/impl/flat_buffer.ipp | 64 ++--- include/beast/core/impl/static_buffer.ipp | 331 +++++----------------- include/beast/core/static_buffer.hpp | 144 +++++----- include/beast/websocket/impl/read.ipp | 16 +- include/beast/websocket/impl/write.ipp | 4 +- test/core/static_buffer.cpp | 57 +--- test/websocket/stream.cpp | 4 +- 9 files changed, 187 insertions(+), 441 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1770c27f..849ac0c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ Version 51 * Fix operator<< for header +API Changes: + +* Tune up static_buffer + -------------------------------------------------------------------------------- Version 50 diff --git a/include/beast/core/flat_buffer.hpp b/include/beast/core/flat_buffer.hpp index a592ea1f..f67d0b51 100644 --- a/include/beast/core/flat_buffer.hpp +++ b/include/beast/core/flat_buffer.hpp @@ -75,7 +75,7 @@ private: return static_cast(last - first); } - char* p_; + char* begin_; char* in_; char* out_; char* last_; @@ -218,7 +218,7 @@ public: std::size_t capacity() const { - return dist(p_, end_); + return dist(begin_, end_); } /// Get a list of buffers that represent the input sequence. diff --git a/include/beast/core/impl/flat_buffer.ipp b/include/beast/core/impl/flat_buffer.ipp index 8ae96b5d..99668754 100644 --- a/include/beast/core/impl/flat_buffer.ipp +++ b/include/beast/core/impl/flat_buffer.ipp @@ -16,7 +16,7 @@ namespace beast { /* Memory is laid out thusly: - p_ ..|.. in_ ..|.. out_ ..|.. last_ ..|.. end_ + begin_ ..|.. in_ ..|.. out_ ..|.. last_ ..|.. end_ */ namespace detail { @@ -41,13 +41,13 @@ void basic_flat_buffer:: move_from(basic_flat_buffer& other) { - p_ = other.p_; + begin_ = other.begin_; in_ = other.in_; out_ = other.out_; last_ = out_; end_ = other.end_; max_ = other.max_; - other.p_ = nullptr; + other.begin_ = nullptr; other.in_ = nullptr; other.out_ = nullptr; other.last_ = nullptr; @@ -65,16 +65,16 @@ copy_from(basic_flat_buffer< auto const n = other.size(); if(n > 0) { - p_ = alloc_traits::allocate( + begin_ = alloc_traits::allocate( this->member(), n); - in_ = p_; - out_ = p_ + n; + in_ = begin_; + out_ = begin_ + n; last_ = out_; end_ = out_; std::memcpy(in_, other.in_, n); return; } - p_ = nullptr; + begin_ = nullptr; in_ = nullptr; out_ = nullptr; last_ = nullptr; @@ -85,9 +85,9 @@ template basic_flat_buffer:: ~basic_flat_buffer() { - if(p_) + if(begin_) alloc_traits::deallocate( - this->member(), p_, dist(p_, end_)); + this->member(), begin_, dist(begin_, end_)); } template @@ -160,7 +160,7 @@ basic_flat_buffer( template basic_flat_buffer:: basic_flat_buffer(std::size_t limit) - : p_(nullptr) + : begin_(nullptr) , in_(nullptr) , out_(nullptr) , last_(nullptr) @@ -176,7 +176,7 @@ basic_flat_buffer(Allocator const& alloc, std::size_t limit) : detail::empty_base_optimization< allocator_type>(alloc) - , p_(nullptr) + , begin_(nullptr) , in_(nullptr) , out_(nullptr) , last_(nullptr) @@ -204,8 +204,8 @@ prepare(std::size_t n) -> // after a memmove, // existing capacity is sufficient if(len > 0) - std::memmove(p_, in_, len); - in_ = p_; + std::memmove(begin_, in_, len); + in_ = begin_; out_ = in_ + len; last_ = out_ + n; return {out_, n}; @@ -220,19 +220,19 @@ prepare(std::size_t n) -> detail::next_pow2(len + n), min_size)); auto const p = alloc_traits::allocate( this->member(), new_size); - if(p_) + if(begin_) { BOOST_ASSERT(p); BOOST_ASSERT(in_); std::memcpy(p, in_, len); alloc_traits::deallocate( - this->member(), p_, capacity()); + this->member(), begin_, capacity()); } - p_ = p; - in_ = p_; + begin_ = p; + in_ = begin_; out_ = in_ + len; last_ = out_ + n; - end_ = p_ + new_size; + end_ = begin_ + new_size; return {out_, n}; } @@ -243,8 +243,8 @@ consume(std::size_t n) { if(n >= dist(in_, out_)) { - in_ = p_; - out_ = p_; + in_ = begin_; + out_ = begin_; return; } in_ += n; @@ -266,19 +266,19 @@ reserve(std::size_t n) auto const p = alloc_traits::allocate( this->member(), new_size); auto const len = size(); - if(p_) + if(begin_) { - BOOST_ASSERT(p_); + BOOST_ASSERT(begin_); BOOST_ASSERT(in_); std::memcpy(p, in_, len); alloc_traits::deallocate( - this->member(), p_, capacity()); + this->member(), begin_, capacity()); } - p_ = p; - in_ = p_; - out_ = p_ + len; + begin_ = p; + in_ = begin_; + out_ = begin_ + len; last_ = out_; - end_ = p_ + new_size; + end_ = begin_ + new_size; } template @@ -292,7 +292,7 @@ shrink_to_fit() char* p; if(len > 0) { - BOOST_ASSERT(p_); + BOOST_ASSERT(begin_); BOOST_ASSERT(in_); p = alloc_traits::allocate( this->member(), len); @@ -303,10 +303,10 @@ shrink_to_fit() p = nullptr; } alloc_traits::deallocate( - this->member(), p_, dist(p_, end_)); - p_ = p; - in_ = p_; - out_ = p_ + len; + this->member(), begin_, dist(begin_, end_)); + begin_ = p; + in_ = begin_; + out_ = begin_ + len; last_ = out_; end_ = out_; } diff --git a/include/beast/core/impl/static_buffer.ipp b/include/beast/core/impl/static_buffer.ipp index 92906d86..9f7b7265 100644 --- a/include/beast/core/impl/static_buffer.ipp +++ b/include/beast/core/impl/static_buffer.ipp @@ -18,288 +18,85 @@ namespace beast { -class static_buffer::const_buffers_type -{ - std::size_t n_; - std::uint8_t const* p_; +/* Memory is laid out thusly: -public: - using value_type = boost::asio::const_buffer; - - class const_iterator; - - const_buffers_type() = delete; - const_buffers_type( - const_buffers_type const&) = default; - const_buffers_type& operator=( - const_buffers_type const&) = default; - - const_iterator - begin() const; - - const_iterator - end() const; - -private: - friend class static_buffer; - - const_buffers_type( - std::uint8_t const* p, std::size_t n) - : n_(n) - , p_(p) - { - } -}; - -class static_buffer::const_buffers_type::const_iterator -{ - std::size_t n_ = 0; - std::uint8_t const* p_ = nullptr; - -public: - using value_type = boost::asio::const_buffer; - using pointer = value_type const*; - using reference = value_type; - 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 p_ == other.p_; - } - - bool - operator!=(const_iterator const& other) const - { - return !(*this == other); - } - - reference - operator*() const - { - return value_type{p_, n_}; - } - - pointer - operator->() const = delete; - - const_iterator& - operator++() - { - p_ += n_; - return *this; - } - - const_iterator - operator++(int) - { - auto temp = *this; - ++(*this); - return temp; - } - - const_iterator& - operator--() - { - p_ -= n_; - return *this; - } - - const_iterator - operator--(int) - { - auto temp = *this; - --(*this); - return temp; - } - -private: - friend class const_buffers_type; - - const_iterator( - std::uint8_t const* p, std::size_t n) - : n_(n) - , p_(p) - { - } -}; + begin_ ..|.. in_ ..|.. out_ ..|.. last_ ..|.. end_ +*/ inline auto -static_buffer::const_buffers_type::begin() const -> - const_iterator -{ - return const_iterator{p_, n_}; -} - -inline -auto -static_buffer::const_buffers_type::end() const -> - const_iterator -{ - return const_iterator{p_ + n_, n_}; -} - -//------------------------------------------------------------------------------ - -class static_buffer::mutable_buffers_type -{ - std::size_t n_; - std::uint8_t* p_; - -public: - using value_type = boost::asio::mutable_buffer; - - class const_iterator; - - mutable_buffers_type() = delete; - mutable_buffers_type( - mutable_buffers_type const&) = default; - mutable_buffers_type& operator=( - mutable_buffers_type const&) = default; - - const_iterator - begin() const; - - const_iterator - end() const; - -private: - friend class static_buffer; - - mutable_buffers_type( - std::uint8_t* p, std::size_t n) - : n_(n) - , p_(p) - { - } -}; - -class static_buffer::mutable_buffers_type::const_iterator -{ - std::size_t n_ = 0; - std::uint8_t* p_ = nullptr; - -public: - using value_type = boost::asio::mutable_buffer; - using pointer = value_type const*; - using reference = value_type; - 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 p_ == other.p_; - } - - bool - operator!=(const_iterator const& other) const - { - return !(*this == other); - } - - reference - operator*() const - { - return value_type{p_, n_}; - } - - pointer - operator->() const = delete; - - const_iterator& - operator++() - { - p_ += n_; - return *this; - } - - const_iterator - operator++(int) - { - auto temp = *this; - ++(*this); - return temp; - } - - const_iterator& - operator--() - { - p_ -= n_; - return *this; - } - - const_iterator - operator--(int) - { - auto temp = *this; - --(*this); - return temp; - } - -private: - friend class mutable_buffers_type; - - const_iterator(std::uint8_t* p, std::size_t n) - : n_(n) - , p_(p) - { - } -}; - -inline -auto -static_buffer::mutable_buffers_type::begin() const -> - const_iterator -{ - return const_iterator{p_, n_}; -} - -inline -auto -static_buffer::mutable_buffers_type::end() const -> - const_iterator -{ - return const_iterator{p_ + n_, n_}; -} - -//------------------------------------------------------------------------------ - -inline -auto -static_buffer::data() const -> +static_buffer:: +data() const -> const_buffers_type { - return const_buffers_type{in_, - static_cast(out_ - in_)}; + return {in_, dist(in_, out_)}; } inline auto -static_buffer::prepare(std::size_t n) -> +static_buffer:: +prepare(std::size_t n) -> mutable_buffers_type { - if(n > static_cast(end_ - out_)) + return prepare_impl(n); +} + +inline +void +static_buffer:: +reset(void* p, std::size_t n) +{ + reset_impl(p, n); +} + +template +void +static_buffer:: +reset_impl(void* p, std::size_t n) +{ + begin_ = + reinterpret_cast(p); + in_ = begin_; + out_ = begin_; + last_ = begin_; + end_ = begin_ + n; +} + +template +auto +static_buffer:: +prepare_impl(std::size_t n) -> + mutable_buffers_type +{ + if(n <= dist(out_, end_)) + { + last_ = out_ + n; + return {out_, n}; + } + auto const len = size(); + if(n > capacity() - len) BOOST_THROW_EXCEPTION(std::length_error{ "static_buffer overflow"}); + if(len > 0) + std::memmove(begin_, in_, len); + in_ = begin_; + out_ = in_ + len; last_ = out_ + n; - return mutable_buffers_type{out_, n}; + return {out_, n}; +} + +template +void +static_buffer:: +consume_impl(std::size_t n) +{ + if(n >= size()) + { + in_ = begin_; + out_ = in_; + return; + } + in_ += n; } } // beast diff --git a/include/beast/core/static_buffer.hpp b/include/beast/core/static_buffer.hpp index 51fc458d..dd9f5958 100644 --- a/include/beast/core/static_buffer.hpp +++ b/include/beast/core/static_buffer.hpp @@ -9,9 +9,7 @@ #define BEAST_STATIC_BUFFER_HPP #include -#include -#include -#include +#include #include namespace beast { @@ -33,28 +31,18 @@ namespace beast { */ class static_buffer { -#if BEAST_DOXYGEN -private: -#else -protected: -#endif - std::uint8_t* begin_; - std::uint8_t* in_; - std::uint8_t* out_; - std::uint8_t* last_; - std::uint8_t* end_; + char* begin_; + char* in_; + char* out_; + char* last_; + char* end_; public: -#if BEAST_DOXYGEN /// The type used to represent the input sequence as a list of buffers. - using const_buffers_type = implementation_defined; + using const_buffers_type = boost::asio::const_buffers_1; /// The type used to represent the output sequence as a list of buffers. - using mutable_buffers_type = implementation_defined; - -#else - class const_buffers_type; - class mutable_buffers_type; + using mutable_buffers_type = boost::asio::mutable_buffers_1; static_buffer( static_buffer const& other) noexcept = delete; @@ -62,7 +50,18 @@ public: static_buffer& operator=( static_buffer const&) noexcept = delete; -#endif + /** Constructor. + + This creates a dynamic buffer using the provided storage area. + + @param p A pointer to valid storage of at least `n` bytes. + + @param n The number of valid bytes pointed to by `p`. + */ + static_buffer(void* p, std::size_t n) + { + reset_impl(p, n); + } /// Return the size of the input sequence. std::size_t @@ -75,14 +74,14 @@ public: std::size_t max_size() const { - return end_ - begin_; + return dist(begin_, end_); } /// Return the maximum sum of input and output sizes that can be held without an allocation. std::size_t capacity() const { - return end_ - in_; + return max_size(); } /** Get a list of buffers that represent the input sequence. @@ -118,28 +117,52 @@ public: void consume(std::size_t n) { - in_ += std::min(n, out_ - in_); + consume_impl(n); } -#if BEAST_DOXYGEN -private: -#else protected: -#endif - static_buffer(std::uint8_t* p, std::size_t n) + /** Default constructor. + + The buffer will be in an undefined state. It is necessary + for the derived class to call @ref reset in order to + initialize the object. + */ + static_buffer(); + + /** Reset the pointed-to buffer. + + This function resets the internal state to the buffer provided. + All input and output sequences are invalidated. This function + allows the derived class to construct its members before + initializing the static buffer. + + @param p A pointer to valid storage of at least `n` bytes. + + @param n The number of valid bytes pointed to by `p`. + */ + void + reset(void* p, std::size_t n); + +private: + static + inline + std::size_t + dist(char const* first, char const* last) { - reset(p, n); + return static_cast(last - first); } + template void - reset(std::uint8_t* p, std::size_t n) - { - begin_ = p; - in_ = p; - out_ = p; - last_ = p; - end_ = p + n; - } + reset_impl(void* p, std::size_t n); + + template + mutable_buffers_type + prepare_impl(std::size_t n); + + template + void + consume_impl(std::size_t n); }; //------------------------------------------------------------------------------ @@ -157,49 +180,22 @@ protected: @see @ref static_buffer */ template -class static_buffer_n - : public static_buffer -#if ! BEAST_DOXYGEN - , private boost::base_from_member< - std::array> -#endif +class static_buffer_n : public static_buffer { - using member_type = boost::base_from_member< - std::array>; + char buf_[N]; + public: -#if BEAST_DOXYGEN -private: -#endif - static_buffer_n( - static_buffer_n const&) = delete; - static_buffer_n& operator=( - static_buffer_n const&) = delete; -#if BEAST_DOXYGEN -public: -#endif + /// Copy constructor (disallowed). + static_buffer_n(static_buffer_n const&) = delete; + + /// Copy assignment (disallowed). + static_buffer_n& operator=(static_buffer_n const&) = delete; /// Construct a static buffer. static_buffer_n() - : static_buffer( - member_type::member.data(), - member_type::member.size()) + : static_buffer(buf_, N) { } - - /** Reset the static buffer. - - @par Effects - - The input sequence and output sequence are empty, - @ref max_size returns `N`. - */ - void - reset() - { - static_buffer::reset( - member_type::member.data(), - member_type::member.size()); - } }; } // beast diff --git a/include/beast/websocket/impl/read.ipp b/include/beast/websocket/impl/read.ipp index 1aab6931..ed1f157d 100644 --- a/include/beast/websocket/impl/read.ipp +++ b/include/beast/websocket/impl/read.ipp @@ -440,7 +440,7 @@ operator()(error_code ec, { ping_data payload; detail::read(payload, d.fb.data()); - d.fb.reset(); + d.fb.consume(d.fb.size()); if(d.ws.ping_cb_) d.ws.ping_cb_(false, payload); if(d.ws.wr_close_) @@ -469,7 +469,7 @@ operator()(error_code ec, detail::read(payload, d.fb.data()); if(d.ws.ping_cb_) d.ws.ping_cb_(true, payload); - d.fb.reset(); + d.fb.consume(d.fb.size()); d.state = do_read_fh; break; } @@ -488,7 +488,7 @@ operator()(error_code ec, if(cr.code == close_code::none) cr.code = close_code::normal; cr.reason = ""; - d.fb.reset(); + d.fb.consume(d.fb.size()); d.ws.template write_close< static_buffer>(d.fb, cr); if(d.ws.wr_block_) @@ -535,7 +535,7 @@ operator()(error_code ec, BOOST_ASSERT(d.ws.wr_block_ == &d); d.ws.wr_block_ = nullptr; } - d.fb.reset(); + d.fb.consume(d.fb.size()); d.state = do_read_fh; break; } @@ -550,7 +550,7 @@ operator()(error_code ec, return; case do_pong + 1: - d.fb.reset(); + d.fb.consume(d.fb.size()); d.state = do_read_fh; d.ws.wr_block_ = nullptr; break; @@ -621,7 +621,7 @@ operator()(error_code ec, d.state = do_fail + 4; break; } - d.fb.reset(); + d.fb.consume(d.fb.size()); d.ws.template write_close< static_buffer>(d.fb, code); if(d.ws.wr_block_) @@ -793,7 +793,7 @@ read_frame(frame_info& fi, DynamicBuffer& dynabuf, error_code& ec) { ping_data payload; detail::read(payload, fb.data()); - fb.reset(); + fb.consume(fb.size()); if(ping_cb_) ping_cb_(false, payload); write_ping( @@ -823,7 +823,7 @@ read_frame(frame_info& fi, DynamicBuffer& dynabuf, error_code& ec) if(cr.code == close_code::none) cr.code = close_code::normal; cr.reason = ""; - fb.reset(); + fb.consume(fb.size()); wr_close_ = true; write_close(fb, cr); boost::asio::write(stream_, fb.data(), ec); diff --git a/include/beast/websocket/impl/write.ipp b/include/beast/websocket/impl/write.ipp index 2172eee8..a3dbd242 100644 --- a/include/beast/websocket/impl/write.ipp +++ b/include/beast/websocket/impl/write.ipp @@ -279,7 +279,7 @@ operator()(error_code ec, case do_nomask_frag + 2: d.cb.consume( bytes_transferred - d.fh_buf.size()); - d.fh_buf.reset(); + d.fh_buf.consume(d.fh_buf.size()); d.fh.op = opcode::cont; if(d.ws.wr_block_ == &d) d.ws.wr_block_ = nullptr; @@ -384,7 +384,7 @@ operator()(error_code ec, case do_mask_frag + 2: d.cb.consume( bytes_transferred - d.fh_buf.size()); - d.fh_buf.reset(); + d.fh_buf.consume(d.fh_buf.size()); d.fh.op = opcode::cont; BOOST_ASSERT(d.ws.wr_block_ == &d); d.ws.wr_block_ = nullptr; diff --git a/test/core/static_buffer.cpp b/test/core/static_buffer.cpp index f7e276db..8e54f1c0 100644 --- a/test/core/static_buffer.cpp +++ b/test/core/static_buffer.cpp @@ -130,7 +130,7 @@ public: } try { - ba.prepare(1); + ba.prepare(ba.capacity() - ba.size() + 1); fail(); } catch(...) @@ -141,59 +141,9 @@ public: }}}}}} } - void testIterators() + void + testReadSizeHelper() { - static_buffer_n<2> ba; - { - auto mb = ba.prepare(2); - std::size_t n; - n = 0; - for(auto it = mb.begin(); - it != mb.end(); it++) - ++n; - BEAST_EXPECT(n == 1); - mb = ba.prepare(2); - n = 0; - for(auto it = mb.begin(); - it != mb.end(); ++it) - ++n; - BEAST_EXPECT(n == 1); - mb = ba.prepare(2); - n = 0; - for(auto it = mb.end(); - it != mb.begin(); it--) - ++n; - BEAST_EXPECT(n == 1); - mb = ba.prepare(2); - n = 0; - for(auto it = mb.end(); - it != mb.begin(); --it) - ++n; - BEAST_EXPECT(n == 1); - } - ba.prepare(2); - ba.commit(1); - std::size_t n; - n = 0; - for(auto it = ba.data().begin(); - it != ba.data().end(); it++) - ++n; - BEAST_EXPECT(n == 1); - n = 0; - for(auto it = ba.data().begin(); - it != ba.data().end(); ++it) - ++n; - BEAST_EXPECT(n == 1); - n = 0; - for(auto it = ba.data().end(); - it != ba.data().begin(); it--) - ++n; - BEAST_EXPECT(n == 1); - n = 0; - for(auto it = ba.data().end(); - it != ba.data().begin(); --it) - ++n; - BEAST_EXPECT(n == 1); } void run() override @@ -201,7 +151,6 @@ public: test::check_read_size_helper>(); testStaticBuffer(); - testIterators(); } }; diff --git a/test/websocket/stream.cpp b/test/websocket/stream.cpp index a98e08cb..69f7cde3 100644 --- a/test/websocket/stream.cpp +++ b/test/websocket/stream.cpp @@ -1839,9 +1839,9 @@ public: BEAST_EXPECT(n < limit); } - void run() override + void + run() override { -testHandshake(); BOOST_STATIC_ASSERT(std::is_constructible< stream, boost::asio::io_service&>::value);