From 6d9709fe5427c5b2aea95394a3d48852da4c2c08 Mon Sep 17 00:00:00 2001 From: Francisco Date: Tue, 23 Mar 2021 15:47:26 +0000 Subject: [PATCH] adt - make circular buffer work with types without default ctor --- lib/include/srsran/adt/circular_buffer.h | 243 ++++++++++++++----- lib/include/srsran/adt/circular_map.h | 14 +- lib/include/srsran/adt/detail/type_storage.h | 24 +- lib/include/srsran/common/srsran_assert.h | 2 +- lib/test/adt/circular_buffer_test.cc | 227 ++++++++++++++--- 5 files changed, 392 insertions(+), 118 deletions(-) diff --git a/lib/include/srsran/adt/circular_buffer.h b/lib/include/srsran/adt/circular_buffer.h index 28fdf7a81..600273326 100644 --- a/lib/include/srsran/adt/circular_buffer.h +++ b/lib/include/srsran/adt/circular_buffer.h @@ -13,7 +13,9 @@ #ifndef SRSRAN_CIRCULAR_BUFFER_H #define SRSRAN_CIRCULAR_BUFFER_H +#include "srsran/adt/detail/type_storage.h" #include "srsran/adt/expected.h" +#include "srsran/common/srsran_assert.h" #include #include @@ -51,51 +53,97 @@ size_t get_max_size(const std::vector& a) template class base_circular_buffer { - using T = typename Container::value_type; + using storage_t = typename Container::value_type; + using T = typename storage_t::value_type; -public: - using value_type = T; - using difference_type = typename Container::difference_type; + template + class iterator_impl + { + using parent_type = typename std::conditional::value, + base_circular_buffer, + const base_circular_buffer >::type; - struct iterator { - iterator(base_circular_buffer& parent_, size_t i) : parent(&parent_), idx(i) {} - iterator& operator++() + public: + using value_type = DataType; + using reference = DataType&; + using pointer = DataType*; + using difference_type = std::ptrdiff_t; + using iterator_category = std::forward_iterator_tag; + + iterator_impl(parent_type& parent_, size_t i) : parent(&parent_), idx(i) {} + + iterator_impl& operator++() { idx = (idx + 1) % parent->max_size(); return *this; } - iterator operator++(int) + iterator_impl operator++(int) { - iterator tmp(*this); + iterator_impl tmp(*this); ++(*this); return tmp; } - iterator operator+(difference_type n) + iterator_impl operator+(difference_type n) { - iterator tmp(*this); + iterator_impl tmp(*this); tmp += n; return tmp; } - iterator& operator+=(difference_type n) + iterator_impl& operator+=(difference_type n) { idx = (idx + n) % parent->max_size(); return *this; } - value_type* operator->() { return &parent->buffer[idx]; } - const value_type* operator->() const { return &parent->buffer[idx]; } - value_type& operator*() { return parent->buffer[idx]; } - const value_type& operator*() const { return parent->buffer[idx]; } - bool operator==(const iterator& it) const { return it.parent == parent and it.idx == idx; } - bool operator!=(const iterator& it) const { return not(*this == it); } + value_type* operator->() { return &get(); } + const value_type* operator->() const { return &get(); } + value_type& operator*() { return get(); } + const value_type& operator*() const { return get(); } + + bool operator==(const iterator_impl& it) const { return it.parent == parent and it.idx == idx; } + bool operator!=(const iterator_impl& it) const { return not(*this == it); } private: - base_circular_buffer* parent; - size_t idx; + void assert_idx_within_bounds() + { + srsran_assert(idx + (idx >= parent->rpos ? 0 : parent->max_size()) < parent->rpos + parent->count, + "index=%zd is out-of-bounds [%zd, %zd)", + idx, + parent->rpos, + parent->count); + } + value_type& get() + { + assert_idx_within_bounds(); + return parent->buffer[idx].get(); + } + const value_type& get() const + { + assert_idx_within_bounds(); + return parent->buffer[idx].get(); + } + parent_type* parent; + size_t idx; }; - template - explicit base_circular_buffer(Args&&... args) : buffer(std::forward(args)...) - {} +public: + using value_type = T; + using difference_type = typename Container::difference_type; + using size_type = std::size_t; + + using iterator = iterator_impl; + using const_iterator = iterator_impl; + + base_circular_buffer() = default; + ~base_circular_buffer() { clear(); } + + template + typename std::enable_if::value>::type push(U&& t) + { + srsran_assert(not full(), "Circular buffer is full."); + size_t wpos = (rpos + count) % max_size(); + buffer[wpos].emplace(std::forward(t)); + count++; + } bool try_push(T&& t) { @@ -104,13 +152,7 @@ public: } push(std::move(t)); } - void push(T&& t) - { - assert(not full()); - size_t wpos = (rpos + count) % max_size(); - buffer[wpos] = std::move(t); - count++; - } + bool try_push(const T& t) { if (full()) { @@ -118,49 +160,56 @@ public: } push(t); } - void push(const T& t) - { - assert(not full()); - size_t wpos = (rpos + count) % max_size(); - buffer[wpos] = t; - count++; - } void pop() { - assert(not empty()); + srsran_assert(not empty(), "Cannot call pop() in empty circular buffer"); + buffer[rpos].destroy(); rpos = (rpos + 1) % max_size(); count--; } T& top() { - assert(not empty()); - return buffer[rpos]; + srsran_assert(not empty(), "Cannot call top() in empty circular buffer"); + return buffer[rpos].get(); } const T& top() const { - assert(not empty()); - return buffer[rpos]; + srsran_assert(not empty(), "Cannot call top() in empty circular buffer"); + return buffer[rpos].get(); + } + void clear() + { + for (size_t i = 0; i < count; ++i) { + buffer[rpos + i].destroy(); + } + count = 0; } - void clear() { count = 0; } bool full() const { return count == max_size(); } bool empty() const { return count == 0; } size_t size() const { return count; } size_t max_size() const { return detail::get_max_size(buffer); } - iterator begin() { return iterator(*this, rpos); } - iterator end() { return iterator(*this, (rpos + count) % max_size()); } - - template >::value> > - void set_size(size_t size) + T& operator[](size_t i) { - buffer.resize(size); + srsran_assert(i < count, "Out-of-bounds access to circular buffer (%zd >= %zd)", i, count); + return buffer[(rpos + i) % max_size()].get(); } + const T& operator[](size_t i) const + { + srsran_assert(i < count, "Out-of-bounds access to circular buffer (%zd >= %zd)", i, count); + return buffer[(rpos + i) % max_size()].get(); + } + + iterator begin() { return iterator(*this, rpos); } + const_iterator begin() const { return const_iterator(*this, rpos); } + iterator end() { return iterator(*this, (rpos + count) % max_size()); } + const_iterator end() const { return const_iterator(*this, (rpos + count) % max_size()); } template T discard_if(const F& func) { - for (auto it = buffer.begin(); it != buffer.end(); it++) { + for (auto it = begin(); it != end(); it++) { if (*it != nullptr && func(*it)) { T tmp = std::move(*it); *it = nullptr; @@ -170,7 +219,13 @@ public: return nullptr; } -private: +protected: + base_circular_buffer(size_t rpos_, size_t count_) : rpos(rpos_), count(count_) {} + template + base_circular_buffer(size_t rpos_, size_t count_, BufferArgs&&... args) : + rpos(rpos_), count(count_), buffer(std::forward(args)...) + {} + Container buffer; size_t rpos = 0; size_t count = 0; @@ -394,8 +449,53 @@ protected: * @tparam N size of the queue */ template -class static_circular_buffer : public detail::base_circular_buffer > -{}; +class static_circular_buffer : public detail::base_circular_buffer, N> > +{ + using base_t = detail::base_circular_buffer, N> >; + +public: + static_circular_buffer() = default; + static_circular_buffer(const static_circular_buffer& other) : base_t(other.rpos, other.count) + { + static_assert(std::is_copy_constructible::value, "T must be copy-constructible"); + std::uninitialized_copy(other.begin(), other.end(), base_t::begin()); + } + static_circular_buffer(static_circular_buffer&& other) noexcept : base_t(other.rpos, other.count) + { + static_assert(std::is_move_constructible::value, "T must be move-constructible"); + for (size_t i = 0; i < other.count; ++i) { + size_t idx = (other.rpos + i) % other.max_size(); + base_t::buffer[idx].move_ctor(std::move(other.buffer[idx])); + } + other.clear(); + } + static_circular_buffer& operator=(const static_circular_buffer& other) + { + if (this == &other) { + return *this; + } + base_t::clear(); + base_t::rpos = other.rpos; + base_t::count = other.count; + for (size_t i = 0; i < other.count; ++i) { + size_t idx = (other.rpos + i) % other.max_size(); + base_t::buffer[idx].copy_ctor(other.buffer[idx]); + } + return *this; + } + static_circular_buffer& operator=(static_circular_buffer&& other) noexcept + { + base_t::clear(); + base_t::rpos = other.rpos; + base_t::count = other.count; + for (size_t i = 0; i < other.count; ++i) { + size_t idx = (other.rpos + i) % other.max_size(); + base_t::buffer[idx].move_ctor(std::move(other.buffer[idx])); + } + other.clear(); + return *this; + } +}; /** * Circular buffer with buffer storage via a std::vector. @@ -404,19 +504,44 @@ class static_circular_buffer : public detail::base_circular_buffer -class dyn_circular_buffer : public detail::base_circular_buffer > +class dyn_circular_buffer : public detail::base_circular_buffer > > { - using base_t = detail::base_circular_buffer >; + using base_t = detail::base_circular_buffer > >; public: dyn_circular_buffer() = default; - explicit dyn_circular_buffer(size_t size) : base_t(size) {} + explicit dyn_circular_buffer(size_t max_size) : base_t(0, 0, max_size) {} + dyn_circular_buffer(dyn_circular_buffer&& other) noexcept : base_t(other.rpos, other.count, std::move(other.buffer)) + { + other.count = 0; + other.rpos = 0; + } + dyn_circular_buffer(const dyn_circular_buffer& other) : base_t(other.rpos, other.count, other.max_size()) + { + static_assert(std::is_copy_constructible::value, "T must be copy-constructible"); + for (size_t i = 0; i < other.count; ++i) { + size_t idx = (other.rpos + i) % other.max_size(); + base_t::buffer[idx].copy_ctor(other.buffer[idx]); + } + } + dyn_circular_buffer& operator=(dyn_circular_buffer other) noexcept + { + swap(other); + other.clear(); + return *this; + } + + void swap(dyn_circular_buffer& other) noexcept + { + std::swap(base_t::rpos, other.rpos); + std::swap(base_t::count, other.count); + std::swap(base_t::buffer, other.buffer); + } void set_size(size_t size) { - // Note: dynamic resizes not supported. - assert(base_t::empty()); - base_t::set_size(size); + srsran_assert(base_t::empty(), "Dynamic resizes not supported when circular buffer is not empty"); + base_t::buffer.resize(size); } }; diff --git a/lib/include/srsran/adt/circular_map.h b/lib/include/srsran/adt/circular_map.h index 6c3823204..23fc4c252 100644 --- a/lib/include/srsran/adt/circular_map.h +++ b/lib/include/srsran/adt/circular_map.h @@ -105,7 +105,7 @@ public: { for (size_t idx = 0; idx < other.capacity(); ++idx) { if (present[idx]) { - buffer[idx].template construct(other.get_obj_(idx)); + buffer[idx].template emplace(other.get_obj_(idx)); } } } @@ -113,7 +113,7 @@ public: { for (size_t idx = 0; idx < other.capacity(); ++idx) { if (present[idx]) { - buffer[idx].template construct(std::move(other.get_obj_(idx))); + buffer[idx].template emplace(std::move(other.get_obj_(idx))); } } other.clear(); @@ -153,7 +153,7 @@ public: if (present[idx]) { return false; } - buffer[idx].template construct(id, obj); + buffer[idx].template emplace(id, obj); present[idx] = true; count++; return true; @@ -164,7 +164,7 @@ public: if (present[idx]) { return srsran::expected(std::move(obj)); } - buffer[idx].template construct(id, std::move(obj)); + buffer[idx].template emplace(id, std::move(obj)); present[idx] = true; count++; return iterator(this, idx); @@ -240,9 +240,9 @@ private: obj_t& get_obj_(size_t idx) { return buffer[idx].get(); } const obj_t& get_obj_(size_t idx) const { return buffer[idx].get(); } - std::array, N> buffer; - std::array present; - size_t count = 0; + std::array, N> buffer; + std::array present; + size_t count = 0; }; } // namespace srsran diff --git a/lib/include/srsran/adt/detail/type_storage.h b/lib/include/srsran/adt/detail/type_storage.h index 2645eaef1..b97cc522e 100644 --- a/lib/include/srsran/adt/detail/type_storage.h +++ b/lib/include/srsran/adt/detail/type_storage.h @@ -18,23 +18,21 @@ namespace srsran { +namespace detail { + template struct type_storage { + using value_type = T; + template - void construct(Args&&... args) + void emplace(Args&&... args) { new (&buffer) T(std::forward(args)...); } void destroy() { get().~T(); } - void copy_ctor(const type_storage& other) { buffer.get() = other.get(); } - void move_ctor(type_storage&& other) { buffer.get() = std::move(other.get()); } - void copy_assign(const type_storage& other) - { - if (this == &other) { - return; - } - get() = other.get(); - } + void copy_ctor(const type_storage& other) { emplace(other.get()); } + void move_ctor(type_storage&& other) { emplace(std::move(other.get())); } + void copy_assign(const type_storage& other) { get() = other.get(); } void move_assign(type_storage&& other) { get() = std::move(other.get()); } T& get() { return reinterpret_cast(buffer); } @@ -53,7 +51,7 @@ void copy_if_present_helper(type_storage& lhs, const type_storage& rhs, bo lhs.destroy(); } if (rhs_present) { - lhs.template construct(rhs.get()); + lhs.copy_ctor(rhs); } } @@ -67,10 +65,12 @@ void move_if_present_helper(type_storage& lhs, type_storage& rhs, bool lhs lhs.destroy(); } if (rhs_present) { - lhs.template construct(std::move(rhs.get())); + lhs.move_ctor(std::move(rhs)); } } +} // namespace detail + } // namespace srsran #endif // SRSRAN_TYPE_STORAGE_H diff --git a/lib/include/srsran/common/srsran_assert.h b/lib/include/srsran/common/srsran_assert.h index 48b1f4244..cef540f00 100644 --- a/lib/include/srsran/common/srsran_assert.h +++ b/lib/include/srsran/common/srsran_assert.h @@ -28,8 +28,8 @@ do { \ if (srsran_unlikely(not(condition))) { \ srslog::fetch_basic_logger("ALL").error("%s:%d: " fmt, __FILE__, __LINE__, ##__VA_ARGS__); \ - srsran::console_stderr("%s:%d: " fmt "\n", __FILE__, __LINE__, ##__VA_ARGS__); \ srslog::flush(); \ + srsran::console_stderr("%s:%d: " fmt "\n", __FILE__, __LINE__, ##__VA_ARGS__); \ std::abort(); \ } \ } while (0) diff --git a/lib/test/adt/circular_buffer_test.cc b/lib/test/adt/circular_buffer_test.cc index 97db4b267..a1f78a8b1 100644 --- a/lib/test/adt/circular_buffer_test.cc +++ b/lib/test/adt/circular_buffer_test.cc @@ -15,55 +15,198 @@ namespace srsran { +struct C { + C() : val_ptr(new int(5)) { count++; } + ~C() { count--; } + C(C&& other) : val_ptr(move(other.val_ptr)) { count++; } + C& operator=(C&&) = default; + + std::unique_ptr val_ptr; + + static size_t count; +}; +size_t C::count = 0; + +struct D { + D() { count++; } + ~D() { count--; } + D(const D&) { count++; } + D(D&&) = delete; + D& operator=(D&&) = delete; + D& operator=(const D&) = default; + + static size_t count; +}; +size_t D::count = 0; + int test_static_circular_buffer() { - static_circular_buffer circ_buffer; - TESTASSERT(circ_buffer.max_size() == 10); - TESTASSERT(circ_buffer.empty() and not circ_buffer.full() and circ_buffer.size() == 0); + { + static_circular_buffer circ_buffer; + TESTASSERT(circ_buffer.max_size() == 10); + TESTASSERT(circ_buffer.empty() and not circ_buffer.full() and circ_buffer.size() == 0); - // push until full - for (size_t i = 0; i < circ_buffer.max_size(); ++i) { - TESTASSERT(circ_buffer.size() == i and not circ_buffer.full()); - circ_buffer.push(i); - TESTASSERT(not circ_buffer.empty()); - } - TESTASSERT(circ_buffer.size() == 10 and circ_buffer.full()); + // push until full + for (size_t i = 0; i < circ_buffer.max_size(); ++i) { + TESTASSERT(circ_buffer.size() == i and not circ_buffer.full()); + circ_buffer.push(i); + TESTASSERT(not circ_buffer.empty()); + } + TESTASSERT(circ_buffer.size() == 10 and circ_buffer.full()); - // test iterator - int count = 0; - for (int& it : circ_buffer) { - TESTASSERT(it == count); - count++; - } - TESTASSERT(*circ_buffer.begin() == circ_buffer.top()); + // test iterator + int count = 0; + for (int it : circ_buffer) { + TESTASSERT(it == count); + count++; + } + TESTASSERT(*circ_buffer.begin() == circ_buffer.top()); - // pop until empty - for (size_t i = 0; i < circ_buffer.max_size(); ++i) { - TESTASSERT(circ_buffer.size() == circ_buffer.max_size() - i and not circ_buffer.empty()); - TESTASSERT(circ_buffer.top() == (int)i); - circ_buffer.pop(); - } - TESTASSERT(circ_buffer.empty() and circ_buffer.size() == 0); + // pop until empty + for (size_t i = 0; i < circ_buffer.max_size(); ++i) { + TESTASSERT(circ_buffer.size() == circ_buffer.max_size() - i and not circ_buffer.empty()); + TESTASSERT(circ_buffer.top() == (int)i); + circ_buffer.pop(); + } + TESTASSERT(circ_buffer.empty() and circ_buffer.size() == 0); - // test iteration with wrap-around in memory - for (size_t i = 0; i < circ_buffer.max_size(); ++i) { - circ_buffer.push(i); - } - for (size_t i = 0; i < circ_buffer.max_size() / 2; ++i) { - circ_buffer.pop(); - } - circ_buffer.push(circ_buffer.max_size()); - circ_buffer.push(circ_buffer.max_size() + 1); - TESTASSERT(circ_buffer.size() == circ_buffer.max_size() / 2 + 2); - count = circ_buffer.max_size() / 2; - for (int& it : circ_buffer) { - TESTASSERT(it == count); - count++; + // test iteration with wrap-around in memory + for (size_t i = 0; i < circ_buffer.max_size(); ++i) { + circ_buffer.push(i); + } + for (size_t i = 0; i < circ_buffer.max_size() / 2; ++i) { + circ_buffer.pop(); + } + circ_buffer.push(circ_buffer.max_size()); + circ_buffer.push(circ_buffer.max_size() + 1); + TESTASSERT(circ_buffer.size() == circ_buffer.max_size() / 2 + 2); + count = circ_buffer.max_size() / 2; + for (int& it : circ_buffer) { + TESTASSERT(it == count); + count++; + } } + // TEST: move-only types + { + static_circular_buffer circbuffer; + circbuffer.push(C{}); + circbuffer.push(C{}); + circbuffer.push(C{}); + circbuffer.push(C{}); + circbuffer.push(C{}); + TESTASSERT(circbuffer.full() and C::count == 5); + C c = std::move(circbuffer.top()); + TESTASSERT(circbuffer.full() and C::count == 6); + circbuffer.pop(); + TESTASSERT(not circbuffer.full() and C::count == 5); + + static_circular_buffer circbuffer2(std::move(circbuffer)); + TESTASSERT(circbuffer.empty() and circbuffer2.size() == 4); + TESTASSERT(C::count == 5); + circbuffer.push(C{}); + TESTASSERT(C::count == 6); + circbuffer = std::move(circbuffer2); + TESTASSERT(C::count == 5); + } + + TESTASSERT(C::count == 0); return SRSRAN_SUCCESS; } +void test_dyn_circular_buffer() +{ + { + dyn_circular_buffer circ_buffer(10); + TESTASSERT(circ_buffer.max_size() == 10); + TESTASSERT(circ_buffer.empty() and not circ_buffer.full() and circ_buffer.size() == 0); + + // push until full + for (size_t i = 0; i < circ_buffer.max_size(); ++i) { + TESTASSERT(circ_buffer.size() == i and not circ_buffer.full()); + circ_buffer.push(i); + TESTASSERT(not circ_buffer.empty()); + } + TESTASSERT(circ_buffer.size() == 10 and circ_buffer.full()); + + // test iterator + int count = 0; + for (int it : circ_buffer) { + TESTASSERT(it == count); + count++; + } + TESTASSERT(*circ_buffer.begin() == circ_buffer.top()); + + // pop until empty + for (size_t i = 0; i < circ_buffer.max_size(); ++i) { + TESTASSERT(circ_buffer.size() == circ_buffer.max_size() - i and not circ_buffer.empty()); + TESTASSERT(circ_buffer.top() == (int)i); + circ_buffer.pop(); + } + TESTASSERT(circ_buffer.empty() and circ_buffer.size() == 0); + + // test iteration with wrap-around in memory + for (size_t i = 0; i < circ_buffer.max_size(); ++i) { + circ_buffer.push(i); + } + for (size_t i = 0; i < circ_buffer.max_size() / 2; ++i) { + circ_buffer.pop(); + } + circ_buffer.push(circ_buffer.max_size()); + circ_buffer.push(circ_buffer.max_size() + 1); + TESTASSERT(circ_buffer.size() == circ_buffer.max_size() / 2 + 2); + count = circ_buffer.max_size() / 2; + for (int& it : circ_buffer) { + TESTASSERT(it == count); + count++; + } + } + + // TEST: move-only types + { + dyn_circular_buffer circbuffer(5); + circbuffer.push(C{}); + circbuffer.push(C{}); + circbuffer.push(C{}); + circbuffer.push(C{}); + circbuffer.push(C{}); + TESTASSERT(circbuffer.full() and C::count == 5); + C c = std::move(circbuffer.top()); + TESTASSERT(circbuffer.full() and C::count == 6); + circbuffer.pop(); + TESTASSERT(not circbuffer.full() and C::count == 5); + + dyn_circular_buffer circbuffer2(std::move(circbuffer)); + TESTASSERT(circbuffer.empty() and circbuffer2.size() == 4); + TESTASSERT(C::count == 5); + circbuffer.set_size(5); + circbuffer.push(C{}); + TESTASSERT(C::count == 6); + circbuffer = std::move(circbuffer2); + TESTASSERT(C::count == 5); + } + + // TEST: copy-only types + { + dyn_circular_buffer circbuffer(3); + D d{}; + circbuffer.push(d); + circbuffer.push(d); + circbuffer.push(d); + TESTASSERT(circbuffer.full() and D::count == 4); + + dyn_circular_buffer circbuffer2(circbuffer); + TESTASSERT(circbuffer2.full() and circbuffer.full()); + TESTASSERT(D::count == 7); + circbuffer.pop(); + circbuffer.pop(); + TESTASSERT(D::count == 5); + circbuffer = circbuffer2; + TESTASSERT(D::count == 7); + } + TESTASSERT(C::count == 0); +} + int test_queue_block_api() { dyn_blocking_queue queue(100); @@ -115,9 +258,15 @@ int test_queue_block_api_2() } // namespace srsran -int main() +int main(int argc, char** argv) { + auto& test_log = srslog::fetch_basic_logger("TEST"); + test_log.set_level(srslog::basic_levels::info); + + srsran::test_init(argc, argv); + TESTASSERT(srsran::test_static_circular_buffer() == SRSRAN_SUCCESS); + srsran::test_dyn_circular_buffer(); TESTASSERT(srsran::test_queue_block_api() == SRSRAN_SUCCESS); TESTASSERT(srsran::test_queue_block_api_2() == SRSRAN_SUCCESS); srsran::console("Success\n");