Allow using the total capacity of the RingBuffer

common/ringbuffer.cpp

@@ -36,7 +36,7 @@ auto RingBuffer::Create(std::size_t sz, std::size_t elem_sz, bool limit_writes)
     std::size_t power_of_two{0u};
     if(sz > 0)
     {
-        power_of_two = sz;
+        power_of_two = sz - 1;
         power_of_two |= power_of_two>>1;
         power_of_two |= power_of_two>>2;
         power_of_two |= power_of_two>>4;
@@ -46,11 +46,12 @@ auto RingBuffer::Create(std::size_t sz, std::size_t elem_sz, bool limit_writes)
             power_of_two |= power_of_two>>32;
     }
     ++power_of_two;
-    if(power_of_two <= sz || power_of_two > std::numeric_limits<std::size_t>::max()/elem_sz)
+    if(power_of_two <= sz || power_of_two > std::numeric_limits<std::size_t>::max()>>1
+        || power_of_two > std::numeric_limits<std::size_t>::max()/elem_sz)
         throw std::overflow_error{"Ring buffer size overflow"};
 
     const std::size_t bufbytes{power_of_two * elem_sz};
-    RingBufferPtr rb{new(FamCount(bufbytes)) RingBuffer{limit_writes ? sz : (power_of_two-1),
+    RingBufferPtr rb{new(FamCount(bufbytes)) RingBuffer{limit_writes ? sz : power_of_two,
         power_of_two-1, elem_sz, bufbytes}};
 
     return rb;
@@ -66,35 +67,35 @@ void RingBuffer::reset() noexcept
 
 auto RingBuffer::read(void *dest, std::size_t cnt) noexcept -> std::size_t
 {
-    const std::size_t free_cnt{readSpace()};
+    const std::size_t w{mWritePtr.load(std::memory_order_acquire)};
+    const std::size_t r{mReadPtr.load(std::memory_order_relaxed)};
+    const std::size_t free_cnt{w - r};
     if(free_cnt == 0) return 0;
 
     const std::size_t to_read{std::min(cnt, free_cnt)};
-    std::size_t read_ptr{mReadPtr.load(std::memory_order_relaxed) & mSizeMask};
+    const std::size_t read_ptr{r & mSizeMask};
 
     const std::size_t cnt2{read_ptr + to_read};
     const auto [n1, n2] = (cnt2 <= mSizeMask+1) ? std::make_tuple(to_read, 0_uz)
         : std::make_tuple(mSizeMask+1 - read_ptr, cnt2&mSizeMask);
 
     auto outiter = std::copy_n(mBuffer.begin() + read_ptr*mElemSize, n1*mElemSize,
         static_cast<std::byte*>(dest));
-    read_ptr += n1;
     if(n2 > 0)
-    {
         std::copy_n(mBuffer.begin(), n2*mElemSize, outiter);
-        read_ptr += n2;
-    }
-    mReadPtr.store(read_ptr, std::memory_order_release);
+    mReadPtr.store(r+n1+n2, std::memory_order_release);
     return to_read;
 }
 
 auto RingBuffer::peek(void *dest, std::size_t cnt) const noexcept -> std::size_t
 {
-    const std::size_t free_cnt{readSpace()};
+    const std::size_t w{mWritePtr.load(std::memory_order_acquire)};
+    const std::size_t r{mReadPtr.load(std::memory_order_relaxed)};
+    const std::size_t free_cnt{w - r};
     if(free_cnt == 0) return 0;
 
     const std::size_t to_read{std::min(cnt, free_cnt)};
-    std::size_t read_ptr{mReadPtr.load(std::memory_order_relaxed) & mSizeMask};
+    const std::size_t read_ptr{r & mSizeMask};
 
     const std::size_t cnt2{read_ptr + to_read};
     const auto [n1, n2] = (cnt2 <= mSizeMask+1) ? std::make_tuple(to_read, 0_uz)
@@ -109,25 +110,23 @@ auto RingBuffer::peek(void *dest, std::size_t cnt) const noexcept -> std::size_t
 
 auto RingBuffer::write(const void *src, std::size_t cnt) noexcept -> std::size_t
 {
-    const std::size_t free_cnt{writeSpace()};
+    const std::size_t w{mWritePtr.load(std::memory_order_relaxed)};
+    const std::size_t r{mReadPtr.load(std::memory_order_acquire)};
+    const std::size_t free_cnt{mWriteSize - (w - r)};
     if(free_cnt == 0) return 0;
 
     const std::size_t to_write{std::min(cnt, free_cnt)};
-    std::size_t write_ptr{mWritePtr.load(std::memory_order_relaxed) & mSizeMask};
+    const std::size_t write_ptr{w & mSizeMask};
 
     const std::size_t cnt2{write_ptr + to_write};
     const auto [n1, n2] = (cnt2 <= mSizeMask+1) ? std::make_tuple(to_write, 0_uz)
         : std::make_tuple(mSizeMask+1 - write_ptr, cnt2&mSizeMask);
 
     auto srcbytes = static_cast<const std::byte*>(src);
     std::copy_n(srcbytes, n1*mElemSize, mBuffer.begin() + write_ptr*mElemSize);
-    write_ptr += n1;
     if(n2 > 0)
-    {
         std::copy_n(srcbytes + n1*mElemSize, n2*mElemSize, mBuffer.begin());
-        write_ptr += n2;
-    }
-    mWritePtr.store(write_ptr, std::memory_order_release);
+    mWritePtr.store(w+n1+n2, std::memory_order_release);
     return to_write;
 }
 
@@ -136,11 +135,10 @@ auto RingBuffer::getReadVector() noexcept -> DataPair
 {
     DataPair ret;
 
-    std::size_t w{mWritePtr.load(std::memory_order_acquire)};
-    std::size_t r{mReadPtr.load(std::memory_order_acquire)};
-    w &= mSizeMask;
+    const std::size_t w{mWritePtr.load(std::memory_order_acquire)};
+    std::size_t r{mReadPtr.load(std::memory_order_relaxed)};
+    const std::size_t free_cnt{w - r};
     r &= mSizeMask;
-    const std::size_t free_cnt{(w-r) & mSizeMask};
 
     const std::size_t cnt2{r + free_cnt};
     if(cnt2 > mSizeMask+1)
@@ -169,10 +167,9 @@ auto RingBuffer::getWriteVector() noexcept -> DataPair
     DataPair ret;
 
     std::size_t w{mWritePtr.load(std::memory_order_acquire)};
-    std::size_t r{mReadPtr.load(std::memory_order_acquire) + mWriteSize - mSizeMask};
+    const std::size_t r{mReadPtr.load(std::memory_order_acquire)};
+    const std::size_t free_cnt{mWriteSize - (w - r)};
     w &= mSizeMask;
-    r &= mSizeMask;
-    const std::size_t free_cnt{(r-w-1) & mSizeMask};
 
     const std::size_t cnt2{w + free_cnt};
     if(cnt2 > mSizeMask+1)

common/ringbuffer.h

@@ -49,19 +49,6 @@ struct RingBuffer {
     /** Reset the read and write pointers to zero. This is not thread safe. */
     auto reset() noexcept -> void;
 
-    /**
-     * The non-copying data reader. Returns two ringbuffer data pointers that
-     * hold the current readable data. If the readable data is in one segment
-     * the second segment has zero length.
-     */
-    [[nodiscard]] auto getReadVector() noexcept -> DataPair;
-    /**
-     * The non-copying data writer. Returns two ringbuffer data pointers that
-     * hold the current writeable data. If the writeable data is in one segment
-     * the second segment has zero length.
-     */
-    [[nodiscard]] auto getWriteVector() noexcept -> DataPair;
-
     /**
      * Return the number of elements available for reading. This is the number
      * of elements in front of the read pointer and behind the write pointer.
@@ -70,7 +57,8 @@ struct RingBuffer {
     {
         const std::size_t w{mWritePtr.load(std::memory_order_acquire)};
         const std::size_t r{mReadPtr.load(std::memory_order_acquire)};
-        return (w-r) & mSizeMask;
+        /* mWritePtr is never more than mWriteSize greater than mReadPtr. */
+        return w - r;
     }
 
     /**
@@ -83,14 +71,21 @@ struct RingBuffer {
      * elements into `dest'. Returns the actual number of elements copied.
      */
     [[nodiscard]] auto peek(void *dest, std::size_t cnt) const noexcept -> std::size_t;
+
+    /**
+     * The non-copying data reader. Returns two ringbuffer data pointers that
+     * hold the current readable data. If the readable data is in one segment
+     * the second segment has zero length.
+     */
+    [[nodiscard]] auto getReadVector() noexcept -> DataPair;
     /** Advance the read pointer `cnt' places. */
     auto readAdvance(std::size_t cnt) noexcept -> void
     { mReadPtr.store(mReadPtr.load(std::memory_order_relaxed)+cnt, std::memory_order_release); }
 
 
     /**
-     * Return the number of elements available for writing. This is the number
-     * of elements in front of the write pointer and behind the read pointer.
+     * Return the number of elements available for writing. This is the total
+     * number of writable elements excluding what's readable (already written).
      */
     [[nodiscard]] auto writeSpace() const noexcept -> std::size_t
     { return mWriteSize - readSpace(); }
@@ -100,6 +95,13 @@ struct RingBuffer {
      * the actual number of elements copied.
      */
     [[nodiscard]] auto write(const void *src, std::size_t cnt) noexcept -> std::size_t;
+
+    /**
+     * The non-copying data writer. Returns two ringbuffer data pointers that
+     * hold the current writeable data. If the writeable data is in one segment
+     * the second segment has zero length.
+     */
+    [[nodiscard]] auto getWriteVector() noexcept -> DataPair;
     /** Advance the write pointer `cnt' places. */
     auto writeAdvance(std::size_t cnt) noexcept -> void
     { mWritePtr.store(mWritePtr.load(std::memory_order_relaxed)+cnt, std::memory_order_release); }
@@ -108,9 +110,9 @@ struct RingBuffer {
 
     /**
      * Create a new ringbuffer to hold at least `sz' elements of `elem_sz'
-     * bytes. The number of elements is rounded up to the next power of two
-     * (even if it is already a power of two, to ensure the requested amount
-     * can be written).
+     * bytes. The number of elements is rounded up to a power of two. If
+     * `limit_writes' is true, the writable space will be limited to `sz'
+     * elements regardless of the rounded size.
      */
     [[nodiscard]] static
     auto Create(std::size_t sz, std::size_t elem_sz, bool limit_writes) -> std::unique_ptr<RingBuffer>;