diff options
author | Tor Egge <Tor.Egge@online.no> | 2023-06-07 16:00:45 +0200 |
---|---|---|
committer | Tor Egge <Tor.Egge@online.no> | 2023-06-07 16:00:45 +0200 |
commit | 51cb20b7f0775a348be5da63516c0deee28d684d (patch) | |
tree | f1a9a943aaafef05ff409921ba05c313cf7fc046 /vespalib | |
parent | 7dd44d8bd4d0d6620151e4a4b80f96107f803c04 (diff) |
Improve synchronization between writer and readers in vespalib::GenerationHandler.
Diffstat (limited to 'vespalib')
-rw-r--r-- | vespalib/src/vespa/vespalib/util/generationhandler.cpp | 27 | ||||
-rw-r--r-- | vespalib/src/vespa/vespalib/util/generationhandler.h | 5 |
2 files changed, 17 insertions, 15 deletions
diff --git a/vespalib/src/vespa/vespalib/util/generationhandler.cpp b/vespalib/src/vespa/vespalib/util/generationhandler.cpp index 8edf0d6fae4..dda298bf4a2 100644 --- a/vespalib/src/vespa/vespalib/util/generationhandler.cpp +++ b/vespalib/src/vespa/vespalib/util/generationhandler.cpp @@ -17,23 +17,27 @@ GenerationHandler::GenerationHold::~GenerationHold() { void GenerationHandler::GenerationHold::setValid() noexcept { - assert(!valid(_refCount)); - _refCount.fetch_sub(1); + auto old = _refCount.fetch_sub(1, std::memory_order_release); + assert(!valid(old)); } bool GenerationHandler::GenerationHold::setInvalid() noexcept { - uint32_t refs = _refCount; - assert(valid(refs)); - if (refs != 0) { + uint32_t refs = 0; + if (_refCount.compare_exchange_strong(refs, 1, + std::memory_order_acq_rel, + std::memory_order_relaxed)) + { + return true; + } else { + assert(valid(refs)); return false; } - return _refCount.compare_exchange_strong(refs, 1, std::memory_order_seq_cst); } GenerationHandler::GenerationHold * GenerationHandler::GenerationHold::acquire() noexcept { - if (valid(_refCount.fetch_add(2))) { + if (valid(_refCount.fetch_add(2, std::memory_order_acq_rel))) { return this; } else { release(); @@ -46,7 +50,7 @@ GenerationHandler::GenerationHold::copy(GenerationHold *self) noexcept { if (self == nullptr) { return nullptr; } else { - uint32_t oldRefCount = self->_refCount.fetch_add(2); + uint32_t oldRefCount = self->_refCount.fetch_add(2, std::memory_order_relaxed); (void) oldRefCount; assert(valid(oldRefCount)); return self; @@ -143,15 +147,10 @@ GenerationHandler::incGeneration() { generation_t ngen = getNextGeneration(); - // Make pending writes visible to other threads before checking for readers - // present in last generation. - std::atomic_thread_fence(std::memory_order_seq_cst); auto last = _last.load(std::memory_order_relaxed); - if (last->getRefCount() == 0) { + if (last->getRefCountAcqRel() == 0) { // Last generation is unused, morph it to new generation. This is // the typical case when no readers are present. - // Note: atomic thread fence above is needed to avoid stale data in - // reader set_generation(ngen); last->_generation.store(ngen, std::memory_order_relaxed); update_oldest_used_generation(); diff --git a/vespalib/src/vespa/vespalib/util/generationhandler.h b/vespalib/src/vespa/vespalib/util/generationhandler.h index b346b1fa4e2..69dc17cacb1 100644 --- a/vespalib/src/vespa/vespalib/util/generationhandler.h +++ b/vespalib/src/vespa/vespalib/util/generationhandler.h @@ -39,13 +39,16 @@ public: void setValid() noexcept; bool setInvalid() noexcept; void release() noexcept { - _refCount.fetch_sub(2); + _refCount.fetch_sub(2, std::memory_order_acq_rel); } GenerationHold *acquire() noexcept; static GenerationHold *copy(GenerationHold *self) noexcept; uint32_t getRefCount() const noexcept { return _refCount.load(std::memory_order_relaxed) / 2; } + uint32_t getRefCountAcqRel() noexcept { + return _refCount.fetch_add(0, std::memory_order_acq_rel) / 2; + } }; /** |