Tidy up read_size_helper and dynamic buffers

fix #376

CHANGELOG.md

@@ -1,6 +1,7 @@
 Version 44
 
 * Use BOOST_THROW_EXCEPTION
+* Tidy up read_size_helper and dynamic buffers
 
 --------------------------------------------------------------------------------
 

include/beast/core/detail/ostream.hpp

@@ -9,6 +9,7 @@
 #define BEAST_DETAIL_OSTREAM_HPP
 
 #include <boost/asio/buffer.hpp>
+#include <beast/core/detail/read_size_helper.hpp>
 #include <memory>
 #include <ostream>
 #include <streambuf>
@@ -127,6 +128,7 @@ private:
     {
         using boost::asio::buffer_cast;
         using boost::asio::buffer_size;
+        using beast::detail::read_size_helper;
         auto mbs = buf_.prepare(
             read_size_helper(buf_, max_size));
         auto const mb = *mbs.begin();
@@ -207,6 +209,7 @@ private:
     {
         using boost::asio::buffer_cast;
         using boost::asio::buffer_size;
+        using beast::detail::read_size_helper;
         auto mbs = buf_.prepare(
             read_size_helper(buf_, max_size));
         auto const mb = *mbs.begin();

include/beast/core/detail/read_size_helper.hpp

@@ -0,0 +1,35 @@
+//
+// 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_DETAIL_READ_SIZE_HELPER_HPP
+#define BEAST_DETAIL_READ_SIZE_HELPER_HPP
+
+#include <beast/core/type_traits.hpp>
+#include <boost/assert.hpp>
+#include <algorithm>
+#include <cstddef>
+
+namespace beast {
+namespace detail {
+
+template<class DynamicBuffer>
+std::size_t
+read_size_helper(DynamicBuffer const& buffer, std::size_t max_size)
+{
+    static_assert(beast::is_dynamic_buffer<DynamicBuffer>::value,
+        "DynamicBuffer requirements not met");
+    BOOST_ASSERT(max_size >= 1);
+    auto const size = buffer.size();
+    return std::min<std::size_t>(
+        std::max<std::size_t>(512, buffer.capacity() - size),
+        std::min<std::size_t>(max_size, buffer.max_size() - size));
+}
+
+} // detail
+} // beast
+
+#endif

include/beast/core/flat_buffer.hpp

@@ -283,13 +283,6 @@ public:
     void
     shrink_to_fit();
 
-    // Helper for boost::asio::read_until
-    template<class OtherAlloc>
-    friend
-    std::size_t
-    read_size_helper(basic_flat_buffer<
-        OtherAlloc> const&, std::size_t);
-
 private:
     void
     move_from(basic_flat_buffer& other);

include/beast/core/impl/flat_buffer.ipp

@@ -311,23 +311,6 @@ shrink_to_fit()
     end_ = out_;
 }
 
-template<class Allocator>
-std::size_t
-read_size_helper(basic_flat_buffer<
-    Allocator> const& fb, std::size_t max_size)
-{
-    BOOST_ASSERT(max_size >= 1);
-    auto const len = fb.size();
-    auto const avail = fb.capacity() - len;
-    if (avail > 0)
-        return (std::min)(avail, max_size);
-    auto size = (std::min)(
-        fb.capacity() * 2, fb.max_size()) - len;
-    if(size == 0)
-        size = 1;
-    return (std::min)(size, max_size);
-}
-
 } // beast
 
 #endif

include/beast/core/impl/multi_buffer.ipp

@@ -854,21 +854,19 @@ basic_multi_buffer<Allocator>::debug_check() const
 
 template<class Allocator>
 std::size_t
-read_size_helper(basic_multi_buffer<
-    Allocator> const& multi_buffer, std::size_t max_size)
+read_size_helper(
+    basic_multi_buffer<Allocator> const& buffer,
+        std::size_t max_size)
 {
     BOOST_ASSERT(max_size >= 1);
-    // If we already have an allocated
-    // buffer, try to fill that up first
-    auto const avail = multi_buffer.capacity() - multi_buffer.size();
-    if (avail > 0)
-        return (std::min)(avail, max_size);
-    // Try to have just one new block allocated
-    constexpr std::size_t low = 512;
-    if (multi_buffer.alloc_size_ > low)
-        return (std::min)(max_size, multi_buffer.alloc_size_);
-    // ...but enforce a 512 byte minimum.
-    return (std::min)(max_size, low);
+    auto const size = buffer.size();
+    auto const avail = std::min<std::size_t>(
+        buffer.capacity() - size, max_size);
+    if(avail > 0)
+        return avail; // avoid allocation
+    return std::min<std::size_t>(
+        std::min<std::size_t>(max_size, buffer.max_size() - size),
+        avail + buffer.alloc_size());
 }
 
 } // beast

include/beast/core/multi_buffer.hpp

@@ -285,13 +285,6 @@ public:
     void
     consume(size_type n);
 
-    // Helper for boost::asio::read_until
-    template<class OtherAllocator>
-    friend
-    std::size_t
-    read_size_helper(basic_multi_buffer<
-        OtherAllocator> const& multi_buffer, std::size_t max_size);
-
 private:
     void
     clear();
@@ -315,6 +308,13 @@ private:
     debug_check() const;
 };
 
+/// Helper for boost::asio::read_until
+template<class Allocator>
+std::size_t
+read_size_helper(
+    basic_multi_buffer<Allocator> const& buffer,
+        std::size_t max_size);
+
 /// A typical multi buffer
 using multi_buffer = basic_multi_buffer<std::allocator<char>>;
 

include/beast/http/impl/async_read.ipp

@@ -15,6 +15,7 @@
 #include <beast/core/bind_handler.hpp>
 #include <beast/core/handler_ptr.hpp>
 #include <beast/core/type_traits.hpp>
+#include <beast/core/detail/read_size_helper.hpp>
 #include <boost/asio/handler_alloc_hook.hpp>
 #include <boost/asio/handler_continuation_hook.hpp>
 #include <boost/asio/handler_invoke_hook.hpp>
@@ -160,6 +161,7 @@ operator()(error_code ec,
         case 2:
         case 3:
         {
+            using beast::detail::read_size_helper;
             auto const size =
                 read_size_helper(d.db, 65536);
             BOOST_ASSERT(size > 0);

include/beast/http/impl/read.ipp

@@ -15,6 +15,7 @@
 #include <beast/core/bind_handler.hpp>
 #include <beast/core/handler_ptr.hpp>
 #include <beast/core/type_traits.hpp>
+#include <beast/core/detail/read_size_helper.hpp>
 #include <boost/asio/handler_alloc_hook.hpp>
 #include <boost/asio/handler_continuation_hook.hpp>
 #include <boost/asio/handler_invoke_hook.hpp>
@@ -53,6 +54,7 @@ read_some_buffer(
     do_read:
         boost::optional<typename
             DynamicBuffer::mutable_buffers_type> mb;
+        using beast::detail::read_size_helper;
         auto const size =
             read_size_helper(buffer, 65536);
         BOOST_ASSERT(size > 0);

include/beast/websocket/detail/pmd_extension.hpp

@@ -11,6 +11,7 @@
 #include <beast/core/error.hpp>
 #include <beast/core/consuming_buffers.hpp>
 #include <beast/core/detail/ci_char_traits.hpp>
+#include <beast/core/detail/read_size_helper.hpp>
 #include <beast/zlib/deflate_stream.hpp>
 #include <beast/zlib/inflate_stream.hpp>
 #include <beast/websocket/option.hpp>
@@ -368,6 +369,7 @@ inflate(
     for(;;)
     {
         // VFALCO we could be smarter about the size
+        using beast::detail::read_size_helper;
         auto const bs = buffer.prepare(
             read_size_helper(buffer, 65536));
         auto const out = *bs.begin();

test/core/buffer_test.hpp

@@ -9,6 +9,8 @@
 #define BEAST_TEST_BUFFER_TEST_HPP
 
 #include <beast/core/type_traits.hpp>
+#include <beast/core/detail/read_size_helper.hpp>
+#include <beast/core/detail/type_traits.hpp>
 #include <boost/asio/buffer.hpp>
 #include <algorithm>
 #include <type_traits>
@@ -83,6 +85,32 @@ size_rev_post(ConstBufferSequence const& buffers)
     return n;
 }
 
+namespace has_read_size_helper
+{
+    using beast::detail::read_size_helper;
+
+    template<class T, class = void>
+    struct trait : std::false_type {};
+
+    template<class T>
+    struct trait<T, decltype(
+        std::declval<std::size_t&>() =
+            read_size_helper(std::declval<T>(), 0),
+        (void)0)> : std::true_type
+    {
+    };
+}
+
+// Make sure read_size_helper works
+template<class DynamicBuffer>
+inline
+void
+check_read_size_helper()
+{
+    static_assert(has_read_size_helper::trait<DynamicBuffer>::value,
+            "Missing read_size_helper for dynamic buffer");
+}
+
 } // test
 } // beast
 

test/core/buffers_adapter.cpp

@@ -8,6 +8,7 @@
 // Test that header file is self-contained.
 #include <beast/core/buffers_adapter.hpp>
 
+#include "buffer_test.hpp"
 #include <beast/core/ostream.hpp>
 #include <beast/core/multi_buffer.hpp>
 #include <beast/unit_test/suite.hpp>
@@ -176,6 +177,9 @@ public:
     }
     void run() override
     {
+        test::check_read_size_helper<buffers_adapter<
+            boost::asio::mutable_buffers_1>>();
+
         testBuffersAdapter();
         testCommit();
     }

test/core/flat_buffer.cpp

@@ -286,6 +286,8 @@ public:
     void
     run() override
     {
+        test::check_read_size_helper<flat_buffer>();
+
         testCtors();
         testOperations();
 

test/core/multi_buffer.cpp

@@ -281,14 +281,15 @@ public:
 
     void testCapacity()
     {
+        using beast::detail::read_size_helper;
         using boost::asio::buffer_size;
         {
             multi_buffer b{10};
             BEAST_EXPECT(b.alloc_size() == 10);
             BEAST_EXPECT(read_size_helper(b, 1) == 1);
             BEAST_EXPECT(read_size_helper(b, 10) == 10);
-            BEAST_EXPECT(read_size_helper(b, 20) == 20);
-            BEAST_EXPECT(read_size_helper(b, 1000) == 512);
+            BEAST_EXPECT(read_size_helper(b, 20) == 10);
+            BEAST_EXPECT(read_size_helper(b, 1000) == 10);
             b.prepare(3);
             b.commit(3);
             BEAST_EXPECT(read_size_helper(b, 10) == 7);
@@ -345,6 +346,8 @@ public:
 
     void run() override
     {
+        test::check_read_size_helper<multi_buffer>();
+
         testSpecialMembers();
         testAllocator();
         testPrepare();

test/core/static_buffer.cpp

@@ -8,6 +8,7 @@
 // Test that header file is self-contained.
 #include <beast/core/static_buffer.hpp>
 
+#include "buffer_test.hpp"
 #include <beast/unit_test/suite.hpp>
 #include <boost/asio/buffer.hpp>
 #include <string>
@@ -197,6 +198,8 @@ public:
 
     void run() override
     {
+        test::check_read_size_helper<static_buffer_n<32>>();
+
         testStaticBuffer();
         testIterators();
     }