From 669aebf8fdd2664fe92997e3f5daeda75588d441 Mon Sep 17 00:00:00 2001 From: Laurynas Biveinis Date: Tue, 7 Jan 2025 07:12:08 +0200 Subject: [PATCH] Clarify how to free the last released memory by QSBR threads --- examples/example_olc_art.cpp | 9 +++++++++ qsbr.hpp | 20 ++++++++++++-------- 2 files changed, 21 insertions(+), 8 deletions(-) diff --git a/examples/example_olc_art.cpp b/examples/example_olc_art.cpp index 5c1f6209..59dc4925 100644 --- a/examples/example_olc_art.cpp +++ b/examples/example_olc_art.cpp @@ -126,6 +126,15 @@ int main() { threads[1].join(); threads[2].join(); + // Quitting threads may race with epoch changes by design, resulting in + // previous epoch orphaned requests not being executed until epoch + // changes one more time. If that does not happen, some memory might be + // held too long. Thus users are advised to pass through Q state in the + // last thread a couple more times at the end. + unodb::this_thread().qsbr_resume(); + unodb::this_thread().quiescent(); + unodb::this_thread().quiescent(); + std::cerr << "Final tree memory use: " << tree.get_current_memory_use() << '\n'; std::cerr << "QSBR epochs changed: " diff --git a/qsbr.hpp b/qsbr.hpp index e24b3e97..e1facaca 100644 --- a/qsbr.hpp +++ b/qsbr.hpp @@ -619,7 +619,7 @@ class [[nodiscard]] qsbr_per_thread final { std::size_t current_interval_total_dealloc_size{0}; - bool paused{true}; + bool paused{false}; void advance_last_seen_epoch( bool single_thread_mode, qsbr_epoch new_seen_epoch, @@ -755,6 +755,13 @@ class qsbr final { const auto current_state = get_state(); qsbr_state::assert_invariants(current_state); UNODB_DETAIL_ASSERT(qsbr_state::get_thread_count(current_state) <= 1); + // Quitting threads may race with epoch changes by design, resulting in + // previous epoch orphaned requests not being executed until epoch + // changes one more time. If that does not happen, this assert may fire at + // the process exit time. While it is possible to handle this silently, by + // executing the requests then, this may also result in some memory being + // held too long. Thus users are advised to pass through Q state in the + // last thread a couple more times at the end. UNODB_DETAIL_ASSERT(previous_interval_orphaned_requests_empty()); UNODB_DETAIL_ASSERT(current_interval_orphaned_requests_empty()); detail::deallocation_request::assert_zero_instances(); @@ -853,10 +860,7 @@ static_assert(std::atomic::is_always_lock_free); UNODB_DETAIL_DISABLE_MSVC_WARNING(26455) inline qsbr_per_thread::qsbr_per_thread() : last_seen_quiescent_state_epoch{qsbr::instance().register_thread()}, - last_seen_epoch{last_seen_quiescent_state_epoch} { - UNODB_DETAIL_ASSERT(paused); - paused = false; -} + last_seen_epoch{last_seen_quiescent_state_epoch} {} UNODB_DETAIL_RESTORE_MSVC_WARNINGS() inline void qsbr_per_thread::on_next_epoch_deallocate( @@ -919,10 +923,10 @@ inline void qsbr_per_thread::advance_last_seen_epoch( // NOLINTNEXTLINE(readability-simplify-boolean-expr) UNODB_DETAIL_ASSERT( new_seen_epoch == last_seen_epoch.advance() - // The current thread is 1) quitting; 2) not having seen - // the current epoch yet; 3) it quitting will cause an - // epoch advance + // The current thread is 1) quitting; 2) not having seen the current epoch + // yet; 3) it quitting will cause an epoch advance || (!single_thread_mode && new_seen_epoch == last_seen_epoch.advance(2))); + update_requests(single_thread_mode, new_seen_epoch, std::move(new_current_requests)); }