From 5f0939e771385f6be9467d0e6da4cf51b07d663b Mon Sep 17 00:00:00 2001 From: Damian Jarek Date: Sun, 16 Jun 2019 23:03:26 +0200 Subject: [PATCH] Simplify websocket::detail::prng: - Use a regular function pointer for dynamic dispatch. - Remove `prng::ref` - it did not benefit the default case (TLS avaialable) and actually made the no-TLS case slower, because the time spent in the generator is dominated by mutex locking. Signed-off-by: Damian Jarek --- CHANGELOG.md | 1 + azure-pipelines.yml | 8 + include/boost/beast/websocket/detail/prng.hpp | 78 +---- include/boost/beast/websocket/detail/prng.ipp | 302 ++++-------------- test/beast/websocket/_detail_prng.cpp | 17 +- 5 files changed, 81 insertions(+), 325 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6090dfdd..26ac09ac 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ Version 261: * Deduplicate `websocket::read_size_hint` definition * Fix UB in websocket read tests * Remove redundant includes in websocket +* Simplify websocket::detail::prng -------------------------------------------------------------------------------- diff --git a/azure-pipelines.yml b/azure-pipelines.yml index c8d649d1..d7b95abc 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -53,6 +53,14 @@ jobs: B2_FLAGS: off off CXXSTD: 11 B2_TARGETS: libs/beast/test//run-fat-tests + GCC 9 C++11 NO_DEPRECATED NO_TLS: + TOOLSET: gcc + CXX: g++-9 + PACKAGES: g++-9 + VARIANT: release + B2_FLAGS: off BOOST_NO_CXX11_THREAD_LOCAL + CXXSTD: 11 + B2_TARGETS: libs/beast/test//run-fat-tests GCC 9 C++11 UBASAN: TOOLSET: gcc CXX: g++-9 diff --git a/include/boost/beast/websocket/detail/prng.hpp b/include/boost/beast/websocket/detail/prng.hpp index e0ce7cd2..6eb474f2 100644 --- a/include/boost/beast/websocket/detail/prng.hpp +++ b/include/boost/beast/websocket/detail/prng.hpp @@ -13,7 +13,6 @@ #include #include #include -#include #include namespace boost { @@ -21,66 +20,7 @@ namespace beast { namespace websocket { namespace detail { -// Type-erased UniformRandomBitGenerator -// with 32-bit unsigned integer value_type -// -class prng -{ -protected: - virtual ~prng() = default; - virtual prng& acquire() noexcept = 0; - virtual void release() noexcept = 0; - virtual std::uint32_t operator()() noexcept = 0; - -public: - using value_type = std::uint32_t; - - class ref - { - prng& p_; - - public: - ref& operator=(ref const&) = delete; - - ~ref() - { - p_.release(); - } - - explicit - ref(prng& p) noexcept - : p_(p.acquire()) - { - } - - ref(ref const& other) noexcept - : p_(other.p_.acquire()) - { - } - - value_type - operator()() noexcept - { - return p_(); - } - - static - value_type constexpr - min BOOST_PREVENT_MACRO_SUBSTITUTION () noexcept - { - return (std::numeric_limits< - value_type>::min)(); - } - - static - value_type constexpr - max BOOST_PREVENT_MACRO_SUBSTITUTION () noexcept - { - return (std::numeric_limits< - value_type>::max)(); - } - }; -}; +using generator = std::uint32_t(*)(); //------------------------------------------------------------------------------ @@ -91,25 +31,11 @@ BOOST_BEAST_DECL std::uint32_t const* prng_seed(std::seed_seq* ss = nullptr); -// Acquire a PRNG using the no-TLS implementation -// -BOOST_BEAST_DECL -prng::ref -make_prng_no_tls(bool secure); - -// Acquire a PRNG using the TLS implementation -// -#ifndef BOOST_NO_CXX11_THREAD_LOCAL -BOOST_BEAST_DECL -prng::ref -make_prng_tls(bool secure); -#endif - // Acquire a PRNG using the TLS implementation if it // is available, otherwise using the no-TLS implementation. // BOOST_BEAST_DECL -prng::ref +generator make_prng(bool secure); } // detail diff --git a/include/boost/beast/websocket/detail/prng.ipp b/include/boost/beast/websocket/detail/prng.ipp index 2807ca96..2d4e7f27 100644 --- a/include/boost/beast/websocket/detail/prng.ipp +++ b/include/boost/beast/websocket/detail/prng.ipp @@ -13,14 +13,10 @@ #include #include #include -#include -#include #include #include #include -#include #include -#include namespace boost { namespace beast { @@ -59,264 +55,92 @@ prng_seed(std::seed_seq* ss) //------------------------------------------------------------------------------ -template -class prng_pool +inline +std::uint32_t +make_nonce() { - std::mutex m_; - T* head_ = nullptr; - -public: - static - prng_pool& - instance() - { - static prng_pool p; - return p; - } - - ~prng_pool() - { - for(auto p = head_; p;) - { - auto next = p->next; - p->~T(); - boost::alignment::aligned_free(p); - p = next; - } - } - - prng::ref - acquire() - { - { - std::lock_guard< - std::mutex> g(m_); - if(head_) - { - auto p = head_; - head_ = head_->next; - return prng::ref(*p); - } - } - auto p = boost::alignment::aligned_alloc( - 16, sizeof(T)); - if(! p) - BOOST_THROW_EXCEPTION(std::bad_alloc{}); - return prng::ref(*(::new(p) T())); - } - - void - release(T& t) - { - std::lock_guard< - std::mutex> g(m_); - t.next = head_; - head_ = &t; - } -}; - -prng::ref -make_prng_no_tls(bool secure) -{ - class fast_prng final : public prng - { - int refs_ = 0; - beast::detail::pcg r_; - - public: - fast_prng* next = nullptr; - - fast_prng() - : r_( - []{ - auto const pv = prng_seed(); - return - ((static_cast(pv[0])<<32)+pv[1]) ^ - ((static_cast(pv[2])<<32)+pv[3]) ^ - ((static_cast(pv[4])<<32)+pv[5]) ^ - ((static_cast(pv[6])<<32)+pv[7]); - }(), - []{ - static std::atomic< - std::uint32_t> nonce{0}; - return ++nonce; - }()) - { - } - - protected: - prng& - acquire() noexcept override - { - ++refs_; - return *this; - } - - void - release() noexcept override - { - if(--refs_ == 0) - prng_pool::instance().release(*this); - } - - value_type - operator()() noexcept override - { - return r_(); - } - }; - - class secure_prng final : public prng - { - int refs_ = 0; - beast::detail::chacha<20> r_; - - public: - secure_prng* next = nullptr; - - secure_prng() - : r_(prng_seed(), [] - { - static std::atomic< - std::uint64_t> nonce{0}; - return ++nonce; - }()) - { - } - - protected: - prng& - acquire() noexcept override - { - ++refs_; - return *this; - } - - void - release() noexcept override - { - if(--refs_ == 0) - prng_pool::instance().release(*this); - } - - value_type - operator()() noexcept override - { - return r_(); - } - }; - - if(secure) - return prng_pool::instance().acquire(); - - return prng_pool::instance().acquire(); + static std::atomic nonce{0}; + return ++nonce; } -//------------------------------------------------------------------------------ - -#ifndef BOOST_NO_CXX11_THREAD_LOCAL - -prng::ref -make_prng_tls(bool secure) +inline +beast::detail::pcg make_pcg() { - class fast_prng final : public prng + auto const pv = prng_seed(); + return beast::detail::pcg{ + ((static_cast(pv[0])<<32)+pv[1]) ^ + ((static_cast(pv[2])<<32)+pv[3]) ^ + ((static_cast(pv[4])<<32)+pv[5]) ^ + ((static_cast(pv[6])<<32)+pv[7]), make_nonce()}; +} + +#ifdef BOOST_NO_CXX11_THREAD_LOCAL + +inline +std::uint32_t +secure_generate() +{ + struct generator { - beast::detail::pcg r_; - - public: - fast_prng() - : r_( - []{ - auto const pv = prng_seed(); - return - ((static_cast(pv[0])<<32)+pv[1]) ^ - ((static_cast(pv[2])<<32)+pv[3]) ^ - ((static_cast(pv[4])<<32)+pv[5]) ^ - ((static_cast(pv[6])<<32)+pv[7]); - }(), - []{ - static std::atomic< - std::uint32_t> nonce{0}; - return ++nonce; - }()) + std::uint32_t operator()() { + std::lock_guard guard{mtx}; + return gen(); } - protected: - prng& - acquire() noexcept override - { - return *this; - } - - void - release() noexcept override - { - } - - value_type - operator()() noexcept override - { - return r_(); - } + beast::detail::chacha<20> gen; + std::mutex mtx; }; + static generator gen{beast::detail::chacha<20>{prng_seed(), make_nonce()}}; + return gen(); +} - class secure_prng final : public prng +inline +std::uint32_t +fast_generate() +{ + struct generator { - beast::detail::chacha<20> r_; - - public: - secure_prng() - : r_(prng_seed(), [] - { - static std::atomic< - std::uint64_t> nonce{0}; - return ++nonce; - }()) + std::uint32_t operator()() { + std::lock_guard guard{mtx}; + return gen(); } - protected: - prng& - acquire() noexcept override - { - return *this; - } - - void - release() noexcept override - { - } - - value_type - operator()() noexcept override - { - return r_(); - } + beast::detail::pcg gen; + std::mutex mtx; }; + static generator gen{make_pcg()}; + return gen(); +} - if(secure) - { - thread_local secure_prng sp; - return prng::ref(sp); - } +#else - thread_local fast_prng fp; - return prng::ref(fp); +inline +std::uint32_t +secure_generate() +{ + thread_local static beast::detail::chacha<20> gen{prng_seed(), make_nonce()}; + return gen(); +} + +inline +std::uint32_t +fast_generate() +{ + thread_local static beast::detail::pcg gen{make_pcg()}; + return gen(); } #endif -//------------------------------------------------------------------------------ - -prng::ref +generator make_prng(bool secure) { -#ifdef BOOST_NO_CXX11_THREAD_LOCAL - return make_prng_no_tls(secure); -#else - return make_prng_tls(secure); -#endif + if (secure) + return &secure_generate; + else + return &fast_generate; } } // detail diff --git a/test/beast/websocket/_detail_prng.cpp b/test/beast/websocket/_detail_prng.cpp index 3eb6e002..882940e0 100644 --- a/test/beast/websocket/_detail_prng.cpp +++ b/test/beast/websocket/_detail_prng.cpp @@ -42,17 +42,20 @@ public: void testPrng(F const& f) { + auto const min = std::numeric_limits::min(); + auto const max = std::numeric_limits::max(); + { auto v = f()(); BEAST_EXPECT( - v >= (prng::ref::min)() && - v <= (prng::ref::max)()); + v >= min && + v <= max); } { auto v = f()(); BEAST_EXPECT( - v >= (prng::ref::min)() && - v <= (prng::ref::max)()); + v >= min && + v <= max); } } @@ -61,12 +64,6 @@ public: { testPrng([]{ return make_prng(true); }); testPrng([]{ return make_prng(false); }); - testPrng([]{ return make_prng_no_tls(true); }); - testPrng([]{ return make_prng_no_tls(false); }); - #ifndef BOOST_NO_CXX11_THREAD_LOCAL - testPrng([]{ return make_prng_tls(true); }); - testPrng([]{ return make_prng_tls(false); }); - #endif } };