From be0ef72621a07b49a024e4314dab049bd0dd7cc3 Mon Sep 17 00:00:00 2001 From: Aleksey Loginov Date: Mon, 22 Jan 2024 17:23:07 +0300 Subject: [PATCH 1/5] Make static container without array --- src/rpp/rpp/disposables/details/container.hpp | 63 +++++++++--- src/tests/rpp/test_disposables.cpp | 95 ++++++++++++++++++- 2 files changed, 144 insertions(+), 14 deletions(-) diff --git a/src/rpp/rpp/disposables/details/container.hpp b/src/rpp/rpp/disposables/details/container.hpp index 2a14c5726..15fd0643b 100644 --- a/src/rpp/rpp/disposables/details/container.hpp +++ b/src/rpp/rpp/disposables/details/container.hpp @@ -12,7 +12,6 @@ #include #include -#include #include #include @@ -61,7 +60,7 @@ template class dynamic_disposables_container : public dynamic_disposables_container_base { public: - dynamic_disposables_container() + dynamic_disposables_container() : dynamic_disposables_container_base{Count} {} }; @@ -71,26 +70,61 @@ class static_disposables_container { public: static_disposables_container() = default; + static_disposables_container(const static_disposables_container&) = delete; + static_disposables_container& operator=(const static_disposables_container& other) = delete; + + static_disposables_container& operator=(static_disposables_container&& other) noexcept + { + if (this == &other) + return *this; + + m_size = other.m_size; + for(size_t i =0; i < m_size; ++i) + std::construct_at(get(i), std::move(*other.get(i))); + + other.clear(); + return *this; + } + + static_disposables_container(static_disposables_container&& other) noexcept + { + *this = std::move(other); + } + + ~static_disposables_container() noexcept + { + clear(); + } void push_back(const rpp::disposable_wrapper& d) { if (m_size >= Count) throw rpp::utils::more_disposables_than_expected{"static_disposables_container obtained more disposables than expected"}; - m_data[m_size++] = d; + std::construct_at(get(m_size++), d); } void push_back(rpp::disposable_wrapper&& d) { if (m_size >= Count) throw rpp::utils::more_disposables_than_expected{"static_disposables_container obtained more disposables than expected"}; - m_data[m_size++] = std::move(d); + std::construct_at(get(m_size++), std::move(d)); } void remove(const rpp::disposable_wrapper& d) { - auto itr = std::remove(m_data.begin(), m_data.end(), d); - while(itr != m_data.end()) { - (*itr++) = disposable_wrapper{}; + for (size_t i = 0; i < m_size;) + { + if (*get(i) != d) + { + ++i; + continue; + } + + for(size_t j = i+1; j < m_size; ++j) + { + std::construct_at(get(j-1), std::move(*get(j))); + std::destroy_at(get(j)); + } --m_size; } } @@ -98,19 +132,26 @@ class static_disposables_container void dispose() const { for (size_t i =0; i < m_size; ++i) { - m_data[i].dispose(); + get(i)->dispose(); } } void clear() { - m_data = std::array{}; + for (size_t i =0; i < m_size; ++i) + std::destroy_at(get(i)); m_size = 0; } private: - mutable std::array m_data{}; - size_t m_size{}; + const rpp::disposable_wrapper* get(size_t i) const + { + return std::launder(reinterpret_cast(&m_data[i*sizeof(rpp::disposable_wrapper)])); + } + +private: + alignas(rpp::disposable_wrapper) std::byte m_data[sizeof(rpp::disposable_wrapper) * Count]{}; + size_t m_size{}; }; struct none_disposables_container diff --git a/src/tests/rpp/test_disposables.cpp b/src/tests/rpp/test_disposables.cpp index 293f31a15..e46ce1e35 100644 --- a/src/tests/rpp/test_disposables.cpp +++ b/src/tests/rpp/test_disposables.cpp @@ -68,10 +68,10 @@ TEMPLATE_TEST_CASE("disposable keeps state", "", rpp::details::disposables::dyna CHECK(!other->is_disposed()); d.add(other); CHECK(!other->is_disposed()); - + d.clear(); CHECK(other->is_disposed()); - + CHECK(!d.is_disposed()); } SECTION("calling clear on disposed disposable") @@ -183,7 +183,7 @@ TEST_CASE("refcount disposable dispose underlying in case of reaching zero") { auto d = refcount->add_ref(); CHECK(d.is_disposed()); - + refcounted.dispose(); CHECK(underlying->dispose_count == 1); CHECK(refcounted.is_disposed()); @@ -241,4 +241,93 @@ TEST_CASE("composite_disposable correctly handles exception") d.dispose(); CHECK(d1.is_disposed()); CHECK(!d2.is_disposed()); +} + +TEST_CASE("static_disposable_container works as expected") +{ + rpp::details::disposables::static_disposables_container<2> container{}; + + auto d1 = rpp::composite_disposable_wrapper{std::make_shared()}; + auto d2 = rpp::composite_disposable_wrapper{std::make_shared()}; + + SECTION("dispose empty") + { + container.dispose(); + } + + container.push_back(d1); + container.push_back(d2); + + SECTION("dispose with added disposable") + { + container.dispose(); + CHECK(d1.is_disposed()); + CHECK(d2.is_disposed()); + } + + SECTION("clear with added disposable") + { + container.clear(); + container.dispose(); + CHECK(!d1.is_disposed()); + CHECK(!d2.is_disposed()); + SECTION("add cleared and dispose") + { + container.push_back(d1); + CHECK(!d1.is_disposed()); + container.dispose(); + CHECK(d1.is_disposed()); + CHECK(!d2.is_disposed()); + } + } + + SECTION("remove with added disposable") + { + container.remove(d1); + container.dispose(); + CHECK(!d1.is_disposed()); + CHECK(d2.is_disposed()); + SECTION("add removed and dispose") + { + container.push_back(d1); + CHECK(!d1.is_disposed()); + container.dispose(); + CHECK(d1.is_disposed()); + } + } + + SECTION("move container") + { + auto other = std::move(container); + SECTION("dispose original") + { + container.dispose(); + CHECK(!d1.is_disposed()); + CHECK(!d2.is_disposed()); + } + + SECTION("dispose copied") + { + other.dispose(); + CHECK(d1.is_disposed()); + CHECK(d2.is_disposed()); + } + SECTION("move back") + { + container = std::move(other); + SECTION("dispose copied") + { + other.dispose(); + CHECK(!d1.is_disposed()); + CHECK(!d2.is_disposed()); + } + + SECTION("dispose original") + { + container.dispose(); + CHECK(d1.is_disposed()); + CHECK(d2.is_disposed()); + } + } + } } \ No newline at end of file From c45c39faacf44202c11a47e3f9c89b53b4dd66d4 Mon Sep 17 00:00:00 2001 From: Aleksey Loginov Date: Mon, 22 Jan 2024 17:32:33 +0300 Subject: [PATCH 2/5] fix leakage --- src/rpp/rpp/disposables/details/container.hpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/rpp/rpp/disposables/details/container.hpp b/src/rpp/rpp/disposables/details/container.hpp index 15fd0643b..917988785 100644 --- a/src/rpp/rpp/disposables/details/container.hpp +++ b/src/rpp/rpp/disposables/details/container.hpp @@ -120,6 +120,8 @@ class static_disposables_container continue; } + std::destroy_at(get(i)); + for(size_t j = i+1; j < m_size; ++j) { std::construct_at(get(j-1), std::move(*get(j))); From c016405df3e3d34a7314a12c8f89a45a36521340 Mon Sep 17 00:00:00 2001 From: Aleksey Loginov Date: Mon, 22 Jan 2024 17:37:54 +0300 Subject: [PATCH 3/5] compile fix --- src/rpp/rpp/disposables/details/container.hpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/rpp/rpp/disposables/details/container.hpp b/src/rpp/rpp/disposables/details/container.hpp index 917988785..5bdf76481 100644 --- a/src/rpp/rpp/disposables/details/container.hpp +++ b/src/rpp/rpp/disposables/details/container.hpp @@ -150,6 +150,10 @@ class static_disposables_container { return std::launder(reinterpret_cast(&m_data[i*sizeof(rpp::disposable_wrapper)])); } + rpp::disposable_wrapper* get(size_t i) + { + return std::launder(reinterpret_cast(&m_data[i*sizeof(rpp::disposable_wrapper)])); + } private: alignas(rpp::disposable_wrapper) std::byte m_data[sizeof(rpp::disposable_wrapper) * Count]{}; From ecf5ee209a08eaeb374725a22b053f725dc89952 Mon Sep 17 00:00:00 2001 From: Aleksey Loginov Date: Mon, 22 Jan 2024 17:43:47 +0300 Subject: [PATCH 4/5] silent warnings --- src/tests/rpp/test_disposables.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/tests/rpp/test_disposables.cpp b/src/tests/rpp/test_disposables.cpp index e46ce1e35..5203a6a43 100644 --- a/src/tests/rpp/test_disposables.cpp +++ b/src/tests/rpp/test_disposables.cpp @@ -301,7 +301,7 @@ TEST_CASE("static_disposable_container works as expected") auto other = std::move(container); SECTION("dispose original") { - container.dispose(); + container.dispose(); // NOLINT CHECK(!d1.is_disposed()); CHECK(!d2.is_disposed()); } @@ -317,7 +317,7 @@ TEST_CASE("static_disposable_container works as expected") container = std::move(other); SECTION("dispose copied") { - other.dispose(); + other.dispose(); // NOLINT CHECK(!d1.is_disposed()); CHECK(!d2.is_disposed()); } From 3e2a98d62692f75711c19dc9a9203783c04d50f0 Mon Sep 17 00:00:00 2001 From: Aleksey Loginov Date: Mon, 22 Jan 2024 17:59:40 +0300 Subject: [PATCH 5/5] speedup --- src/rpp/rpp/disposables/details/container.hpp | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/rpp/rpp/disposables/details/container.hpp b/src/rpp/rpp/disposables/details/container.hpp index 5bdf76481..aa3ccc804 100644 --- a/src/rpp/rpp/disposables/details/container.hpp +++ b/src/rpp/rpp/disposables/details/container.hpp @@ -120,14 +120,10 @@ class static_disposables_container continue; } - std::destroy_at(get(i)); - for(size_t j = i+1; j < m_size; ++j) - { - std::construct_at(get(j-1), std::move(*get(j))); - std::destroy_at(get(j)); - } - --m_size; + *get(j-1) = std::move(*get(j)); + + std::destroy_at(get(--m_size)); } }