Merge pull request #27203 from vespa-engine/balder/enough-with-generation-guard

Balder/enough with generation guard

9 files changed, 44 insertions, 53 deletions

diff --git a/searchlib/src/vespa/searchlib/attribute/attributevector.h b/searchlib/src/vespa/searchlib/attribute/attributevector.h
index 6bab0f278ca..e3a7fdeb2c3 100644
--- a/searchlib/src/vespa/searchlib/attribute/attributevector.h
+++ b/searchlib/src/vespa/searchlib/attribute/attributevector.h
@@ -440,7 +440,13 @@ private:
     vespalib::steady_time                 _nextStatUpdateTime;
     std::shared_ptr<vespalib::alloc::MemoryAllocator> _memory_allocator;
 
-////// Locking strategy interface. only available from the Guards.
+    /// Clean up [0, firstUsed>
+    virtual void reclaim_memory(generation_t oldest_used_gen);
+    virtual void before_inc_generation(generation_t current_gen);
+    virtual void onUpdateStat() = 0;
+    friend class AttributeTest;
+public:
+    ////// Locking strategy interface.
     /**
      * Used to guard that a value you reference will always reference
      * a value. It might not be the same value, but at least it will
@@ -448,22 +454,6 @@ private:
      * the guard is alive.
     */
     GenerationHandler::Guard takeGenerationGuard() { return _genHandler.takeGuard(); }
-
-    /// Clean up [0, firstUsed>
-    virtual void reclaim_memory(generation_t oldest_used_gen);
-    virtual void before_inc_generation(generation_t current_gen);
-    virtual void onUpdateStat() = 0;
-    /**
-     * Used to regulate access to critical resources. Apply the
-     * reader/writer guards.
-     */
-    std::shared_mutex & getEnumLock() { return _enumLock; }
-
-    friend class ComponentGuard<AttributeVector>;
-    friend class AttributeValueGuard;
-    friend class AttributeTest;
-    friend class AttributeManagerTest;
-public:
     bool headerTypeOK(const vespalib::GenericHeader &header) const;
     bool hasMultiValue() const override final;
     bool hasWeightedSetType() const override final;
diff --git a/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector_read_guard.cpp b/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector_read_guard.cpp
index b50a3720ff8..0a5e2e91446 100644
--- a/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector_read_guard.cpp
+++ b/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector_read_guard.cpp
@@ -18,7 +18,7 @@ ImportedAttributeVectorReadGuard::ImportedAttributeVectorReadGuard(std::shared_p
       _imported_attribute(imported_attribute),
       _targetLids(),
       _target_docid_limit(0u),
-      _reference_attribute_guard(imported_attribute.getReferenceAttribute()),
+      _reference_attribute_guard(imported_attribute.getReferenceAttribute()->takeGenerationGuard()),
       _target_attribute_guard(imported_attribute.getTargetAttribute()->makeReadGuard(stableEnumGuard)),
       _reference_attribute(*imported_attribute.getReferenceAttribute()),
       _target_attribute(*_target_attribute_guard->attribute())
diff --git a/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector_read_guard.h b/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector_read_guard.h
index 5889934fa23..c984725c6e4 100644
--- a/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector_read_guard.h
+++ b/searchlib/src/vespa/searchlib/attribute/imported_attribute_vector_read_guard.h
@@ -96,7 +96,7 @@ private:
     const ImportedAttributeVector       &_imported_attribute;
     TargetLids                           _targetLids;
     uint32_t                             _target_docid_limit;
-    AttributeGuard                       _reference_attribute_guard;
+    vespalib::GenerationHandler::Guard   _reference_attribute_guard;
     std::unique_ptr<attribute::AttributeReadGuard> _target_attribute_guard;
     const ReferenceAttribute            &_reference_attribute;
 protected:
diff --git a/vespalib/src/vespa/vespalib/btree/btreebuilder.hpp b/vespalib/src/vespa/vespalib/btree/btreebuilder.hpp
index c15190a895e..a941689dda2 100644
--- a/vespalib/src/vespa/vespalib/btree/btreebuilder.hpp
+++ b/vespalib/src/vespa/vespalib/btree/btreebuilder.hpp
@@ -3,6 +3,7 @@
 #pragma once
 
 #include "btreebuilder.h"
+#include <cassert>
 
 namespace vespalib::btree {
 
diff --git a/vespalib/src/vespa/vespalib/btree/btreeiterator.h b/vespalib/src/vespa/vespalib/btree/btreeiterator.h
index 01f0ad3c2d3..2418da18c23 100644
--- a/vespalib/src/vespa/vespalib/btree/btreeiterator.h
+++ b/vespalib/src/vespa/vespalib/btree/btreeiterator.h
@@ -5,6 +5,7 @@
 #include "btreenode.h"
 #include "btreenodeallocator.h"
 #include "btreetraits.h"
+#include <cassert>
 
 namespace vespalib::btree {
 
diff --git a/vespalib/src/vespa/vespalib/btree/btreenode.hpp b/vespalib/src/vespa/vespalib/btree/btreenode.hpp
index 90f4c9a9cc5..7de65261e93 100644
--- a/vespalib/src/vespa/vespalib/btree/btreenode.hpp
+++ b/vespalib/src/vespa/vespalib/btree/btreenode.hpp
@@ -4,6 +4,7 @@
 
 #include "btreenode.h"
 #include <algorithm>
+#include <cassert>
 
 namespace vespalib::btree {
 
@@ -74,9 +75,7 @@ upper_bound(uint32_t sidx, const KeyT & key, CompareT comp) const
 
 template <typename KeyT, typename DataT, typename AggrT, uint32_t NumSlots>
 void
-BTreeNodeTT<KeyT, DataT, AggrT, NumSlots>::insert(uint32_t idx,
-                                                  const KeyT &key,
-                                                  const DataT &data)
+BTreeNodeTT<KeyT, DataT, AggrT, NumSlots>::insert(uint32_t idx, const KeyT &key, const DataT &data)
 {
     assert(validSlots() < NodeType::maxSlots());
     assert(!getFrozen());
@@ -208,8 +207,7 @@ stealSomeFromRightNode(NodeType *victim)
 
 template <typename KeyT, typename DataT, typename AggrT, uint32_t NumSlots>
 void
-BTreeNodeTT<KeyT, DataT, AggrT, NumSlots>::cleanRange(uint32_t from,
-                                                      uint32_t to)
+BTreeNodeTT<KeyT, DataT, AggrT, NumSlots>::cleanRange(uint32_t from, uint32_t to)
 {
     assert(from < to);
     assert(to <= validSlots());
diff --git a/vespalib/src/vespa/vespalib/btree/btreerootbase.hpp b/vespalib/src/vespa/vespalib/btree/btreerootbase.hpp
index 025fed3853a..dea383676f2 100644
--- a/vespalib/src/vespa/vespalib/btree/btreerootbase.hpp
+++ b/vespalib/src/vespa/vespalib/btree/btreerootbase.hpp
@@ -3,6 +3,7 @@
 #pragma once
 
 #include "btreerootbase.h"
+#include <cassert>
 
 namespace vespalib::btree {
 
diff --git a/vespalib/src/vespa/vespalib/util/generationhandler.cpp b/vespalib/src/vespa/vespalib/util/generationhandler.cpp
index 3562926d88d..d64779e5f0b 100644
--- a/vespalib/src/vespa/vespalib/util/generationhandler.cpp
+++ b/vespalib/src/vespa/vespalib/util/generationhandler.cpp
@@ -5,7 +5,7 @@
 
 namespace vespalib {
 
-GenerationHandler::GenerationHold::GenerationHold(void)
+GenerationHandler::GenerationHold::GenerationHold() noexcept
     : _refCount(1),
       _generation(0),
       _next(0)
@@ -16,13 +16,13 @@ GenerationHandler::GenerationHold::~GenerationHold() {
 }
 
 void
-GenerationHandler::GenerationHold::setValid() {
+GenerationHandler::GenerationHold::setValid() noexcept {
     assert(!valid(_refCount));
     _refCount.fetch_sub(1);
 }
 
 bool
-GenerationHandler::GenerationHold::setInvalid() {
+GenerationHandler::GenerationHold::setInvalid() noexcept {
     uint32_t refs = _refCount;
     assert(valid(refs));
     if (refs != 0) {
@@ -32,12 +32,12 @@ GenerationHandler::GenerationHold::setInvalid() {
 }
 
 void
-GenerationHandler::GenerationHold::release() {
+GenerationHandler::GenerationHold::release() noexcept {
     _refCount.fetch_sub(2);
 }
 
 GenerationHandler::GenerationHold *
-GenerationHandler::GenerationHold::acquire() {
+GenerationHandler::GenerationHold::acquire() noexcept {
     if (valid(_refCount.fetch_add(2))) {
         return this;
     } else {
@@ -47,7 +47,7 @@ GenerationHandler::GenerationHold::acquire() {
 }
 
 GenerationHandler::GenerationHold *
-GenerationHandler::GenerationHold::copy(GenerationHold *self) {
+GenerationHandler::GenerationHold::copy(GenerationHold *self) noexcept {
     if (self == nullptr) {
         return nullptr;
     } else {
@@ -59,16 +59,16 @@ GenerationHandler::GenerationHold::copy(GenerationHold *self) {
 }
 
 uint32_t
-GenerationHandler::GenerationHold::getRefCount() const {
+GenerationHandler::GenerationHold::getRefCount() const noexcept {
     return _refCount / 2;
 }
 
-GenerationHandler::Guard::Guard()
+GenerationHandler::Guard::Guard() noexcept
     : _hold(nullptr)
 {
 }
 
-GenerationHandler::Guard::Guard(GenerationHold *hold)
+GenerationHandler::Guard::Guard(GenerationHold *hold) noexcept
     : _hold(hold->acquire())
 {
 }
@@ -78,19 +78,19 @@ GenerationHandler::Guard::~Guard()
     cleanup();
 }
 
-GenerationHandler::Guard::Guard(const Guard & rhs)
+GenerationHandler::Guard::Guard(const Guard & rhs) noexcept
     : _hold(GenerationHold::copy(rhs._hold))
 {
 }
 
-GenerationHandler::Guard::Guard(Guard &&rhs)
+GenerationHandler::Guard::Guard(Guard &&rhs) noexcept
     : _hold(rhs._hold)
 {
     rhs._hold = nullptr;
 }
 
 GenerationHandler::Guard &
-GenerationHandler::Guard::operator=(const Guard & rhs)
+GenerationHandler::Guard::operator=(const Guard & rhs) noexcept
 {
     if (&rhs != this) {
         cleanup();
@@ -100,7 +100,7 @@ GenerationHandler::Guard::operator=(const Guard & rhs)
 }
 
 GenerationHandler::Guard &
-GenerationHandler::Guard::operator=(Guard &&rhs)
+GenerationHandler::Guard::operator=(Guard &&rhs) noexcept
 {
     if (&rhs != this) {
         cleanup();
diff --git a/vespalib/src/vespa/vespalib/util/generationhandler.h b/vespalib/src/vespa/vespalib/util/generationhandler.h
index 6ba71b7f5fb..5aaf342564c 100644
--- a/vespalib/src/vespa/vespalib/util/generationhandler.h
+++ b/vespalib/src/vespa/vespalib/util/generationhandler.h
@@ -28,20 +28,20 @@ public:
         // least significant bit is invalid flag
         std::atomic<uint32_t> _refCount;
 
-        static bool valid(uint32_t refCount) { return (refCount & 1) == 0u; }
+        static bool valid(uint32_t refCount) noexcept { return (refCount & 1) == 0u; }
     public:
         std::atomic<generation_t> _generation;
         GenerationHold *_next;	// next free element or next newer element.
 
-        GenerationHold();
+        GenerationHold() noexcept;
         ~GenerationHold();
 
-        void setValid();
-        bool setInvalid();
-        void release();
-        GenerationHold *acquire();
-        static GenerationHold *copy(GenerationHold *self);
-        uint32_t getRefCount() const;
+        void setValid() noexcept;
+        bool setInvalid() noexcept;
+        void release() noexcept;
+        GenerationHold *acquire() noexcept;
+        static GenerationHold *copy(GenerationHold *self) noexcept;
+        uint32_t getRefCount() const noexcept;
     };
 
     /**
@@ -50,22 +50,22 @@ public:
     class Guard {
     private:
         GenerationHold *_hold;
-        void cleanup() {
+        void cleanup() noexcept {
             if (_hold != nullptr) {
                 _hold->release();
                 _hold = nullptr;
             }
         }
     public:
-        Guard();
-        Guard(GenerationHold *hold); // hold is never nullptr
+        Guard() noexcept;
+        Guard(GenerationHold *hold) noexcept; // hold is never nullptr
         ~Guard();
-        Guard(const Guard & rhs);
-        Guard(Guard &&rhs);
-        Guard & operator=(const Guard & rhs);
-        Guard & operator=(Guard &&rhs);
+        Guard(const Guard & rhs) noexcept;
+        Guard(Guard &&rhs) noexcept;
+        Guard & operator=(const Guard & rhs) noexcept;
+        Guard & operator=(Guard &&rhs) noexcept;
 
-        bool valid() const {
+        bool valid() const noexcept {
             return _hold != nullptr;
         }
         generation_t getGeneration() const { return _hold->_generation.load(std::memory_order_relaxed); }