Merge pull request #21743 from vespa-engine/toregge/use-atomic-uint32_t-for-commmitted-doc-id-limitv7.561.60

Use std::atomic<uint32_t> for committed doc id limit.

20 files changed, 48 insertions, 39 deletions

diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp
index 2e6a7087b69..5d2f358e4c8 100644
--- a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp
+++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.cpp
@@ -992,7 +992,7 @@ DocumentMetaStore::populateActiveBuckets(BucketId::List buckets)
 }
 
 void
-DocumentMetaStore::clearDocs(DocId lidLow, DocId lidLimit)
+DocumentMetaStore::clearDocs(DocId lidLow, DocId lidLimit, bool)
 {
     assert(lidLow <= lidLimit);
     assert(lidLimit <= getNumDocs());
diff --git a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h
index 5cb0b1469fd..e393ad16178 100644
--- a/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h
+++ b/searchcore/src/vespa/searchcore/proton/documentmetastore/documentmetastore.h
@@ -246,7 +246,7 @@ public:
 
     const search::GrowableBitVector &getActiveLids() const { return _lidAlloc.getActiveLids(); }
 
-    void clearDocs(DocId lidLow, DocId lidLimit) override;
+    void clearDocs(DocId lidLow, DocId lidLimit, bool in_shrink_lid_space) override;
 
     /*
      * Called by document db executor to unblock shrinking of lid
diff --git a/searchlib/src/vespa/searchlib/attribute/attributevector.cpp b/searchlib/src/vespa/searchlib/attribute/attributevector.cpp
index 4c3134d3235..ea2379eef58 100644
--- a/searchlib/src/vespa/searchlib/attribute/attributevector.cpp
+++ b/searchlib/src/vespa/searchlib/attribute/attributevector.cpp
@@ -577,12 +577,13 @@ uint32_t AttributeVector::getVersion() const { return 0; }
 void
 AttributeVector::compactLidSpace(uint32_t wantedLidLimit) {
     commit();
-    assert(_committedDocIdLimit >= wantedLidLimit);
-    if (wantedLidLimit < _committedDocIdLimit) {
-        clearDocs(wantedLidLimit, _committedDocIdLimit);
+    uint32_t committed_doc_id_limit = _committedDocIdLimit.load(std::memory_order_relaxed);
+    assert(committed_doc_id_limit >= wantedLidLimit);
+    if (wantedLidLimit < committed_doc_id_limit) {
+        clearDocs(wantedLidLimit, committed_doc_id_limit, false);
     }
     commit();
-    _committedDocIdLimit = wantedLidLimit;
+    _committedDocIdLimit.store(wantedLidLimit, std::memory_order_release);
     _compactLidSpaceGeneration = _genHandler.getCurrentGeneration();
     incGeneration();
 }
@@ -603,14 +604,15 @@ AttributeVector::shrinkLidSpace()
     if (!canShrinkLidSpace()) {
         return;
     }
-    uint32_t committedDocIdLimit = _committedDocIdLimit;
-    clearDocs(committedDocIdLimit, getNumDocs());
+    uint32_t committed_doc_id_limit = _committedDocIdLimit.load(std::memory_order_relaxed);
+    clearDocs(committed_doc_id_limit, getNumDocs(), true);
+    clear_uncommitted_doc_id_limit();
     commit();
-    _committedDocIdLimit = committedDocIdLimit;
+    assert(committed_doc_id_limit == _committedDocIdLimit.load(std::memory_order_relaxed));
     onShrinkLidSpace();
     attribute::IPostingListAttributeBase *pab = getIPostingListAttributeBase();
     if (pab != NULL) {
-        pab->forwardedShrinkLidSpace(_committedDocIdLimit);
+        pab->forwardedShrinkLidSpace(committed_doc_id_limit);
     }
     incGeneration();
     updateStat(true);
@@ -619,7 +621,7 @@ AttributeVector::shrinkLidSpace()
 void AttributeVector::onShrinkLidSpace() {}
 
 void
-AttributeVector::clearDocs(DocId lidLow, DocId lidLimit)
+AttributeVector::clearDocs(DocId lidLow, DocId lidLimit, bool in_shrink_lid_space)
 {
     assert(lidLow <= lidLimit);
     assert(lidLimit <= getNumDocs());
@@ -628,6 +630,9 @@ AttributeVector::clearDocs(DocId lidLow, DocId lidLimit)
     for (DocId lid = lidLow; lid < lidLimit; ++lid) {
         clearDoc(lid);
         if ((++count % commit_interval) == 0) {
+            if (in_shrink_lid_space) {
+                clear_uncommitted_doc_id_limit();
+            }
             commit();
         }
     }
diff --git a/searchlib/src/vespa/searchlib/attribute/attributevector.h b/searchlib/src/vespa/searchlib/attribute/attributevector.h
index 5df8a2aa768..8f0ce6327a8 100644
--- a/searchlib/src/vespa/searchlib/attribute/attributevector.h
+++ b/searchlib/src/vespa/searchlib/attribute/attributevector.h
@@ -253,9 +253,8 @@ protected:
 
     void updateCommittedDocIdLimit() {
         if (_uncommittedDocIdLimit != 0) {
-            if (_uncommittedDocIdLimit > _committedDocIdLimit) {
-                std::atomic_thread_fence(std::memory_order_release);
-                _committedDocIdLimit = _uncommittedDocIdLimit;
+            if (_uncommittedDocIdLimit > _committedDocIdLimit.load(std::memory_order_relaxed)) {
+                _committedDocIdLimit.store(_uncommittedDocIdLimit, std::memory_order_release);
             }
             _uncommittedDocIdLimit = 0;
         }
@@ -403,15 +402,16 @@ public:
 
     // Implements IAttributeVector
     uint32_t getNumDocs() const override final { return _status.getNumDocs(); }
-    uint32_t & getCommittedDocIdLimitRef() { return _committedDocIdLimit; }
+    const std::atomic<uint32_t>& getCommittedDocIdLimitRef() noexcept { return _committedDocIdLimit; }
     void setCommittedDocIdLimit(uint32_t committedDocIdLimit) {
-        _committedDocIdLimit = committedDocIdLimit;
+        _committedDocIdLimit.store(committedDocIdLimit, std::memory_order_release);
     }
     void updateUncommittedDocIdLimit(DocId doc) {
         if (_uncommittedDocIdLimit <= doc)  {
             _uncommittedDocIdLimit = doc + 1;
         }
     }
+    void clear_uncommitted_doc_id_limit() noexcept { _uncommittedDocIdLimit = 0; }
 
     const Status & getStatus() const { return _status; }
     Status & getStatus() { return _status; }
@@ -422,7 +422,7 @@ public:
     CollectionType::Type getCollectionType() const override final { return getInternalCollectionType().type(); }
     bool getIsFilter() const override final { return _config.getIsFilter(); }
     bool getIsFastSearch() const override final { return _config.fastSearch(); }
-    uint32_t getCommittedDocIdLimit() const override final { return _committedDocIdLimit; }
+    uint32_t getCommittedDocIdLimit() const override final { return _committedDocIdLimit.load(std::memory_order_acquire); }
     bool isImported() const override;
 
     /**
@@ -578,7 +578,7 @@ private:
     Status                                _status;
     int                                   _highestValueCount;
     uint32_t                              _enumMax;
-    uint32_t                              _committedDocIdLimit; // docid limit for search
+    std::atomic<uint32_t>                 _committedDocIdLimit; // docid limit for search
     uint32_t                              _uncommittedDocIdLimit; // based on queued changes
     uint64_t                              _createSerialNum;
     uint64_t                              _compactLidSpaceGeneration;
@@ -634,8 +634,8 @@ public:
     virtual uint64_t getUniqueValueCount() const;
     virtual uint64_t getTotalValueCount() const;
     void compactLidSpace(uint32_t wantedLidLimit) override;
-    virtual void clearDocs(DocId lidLow, DocId lidLimit);
-    bool wantShrinkLidSpace() const { return _committedDocIdLimit < getNumDocs(); }
+    virtual void clearDocs(DocId lidLow, DocId lidLimit, bool in_shrink_lid_space);
+    bool wantShrinkLidSpace() const { return _committedDocIdLimit.load(std::memory_order_relaxed) < getNumDocs(); }
     bool canShrinkLidSpace() const override;
     void shrinkLidSpace() override;
     virtual void onShrinkLidSpace();
diff --git a/searchlib/src/vespa/searchlib/attribute/multivalueattribute.h b/searchlib/src/vespa/searchlib/attribute/multivalueattribute.h
index 64e49597aff..01cef03e687 100644
--- a/searchlib/src/vespa/searchlib/attribute/multivalueattribute.h
+++ b/searchlib/src/vespa/searchlib/attribute/multivalueattribute.h
@@ -71,7 +71,7 @@ private:
     void apply_attribute_changes_to_wset(DocumentValues& docValues);
 
 public:
-    void clearDocs(DocId lidLow, DocId lidLimit) override;
+    void clearDocs(DocId lidLow, DocId lidLimit, bool in_shrink_lid_space) override;
     void onShrinkLidSpace() override ;
     void onAddDocs(DocId lidLimit) override;
 };
diff --git a/searchlib/src/vespa/searchlib/attribute/multivalueattribute.hpp b/searchlib/src/vespa/searchlib/attribute/multivalueattribute.hpp
index e009a3b69f6..3b270f32f4b 100644
--- a/searchlib/src/vespa/searchlib/attribute/multivalueattribute.hpp
+++ b/searchlib/src/vespa/searchlib/attribute/multivalueattribute.hpp
@@ -274,7 +274,7 @@ MultiValueAttribute<B, M>::getTotalValueCount() const
 
 template <typename B, typename M>
 void
-MultiValueAttribute<B, M>::clearDocs(DocId lidLow, DocId lidLimit)
+MultiValueAttribute<B, M>::clearDocs(DocId lidLow, DocId lidLimit, bool)
 {
     _mvMapping.clearDocs(lidLow, lidLimit, [this](uint32_t docId) { this->clearDoc(docId); });
 }
diff --git a/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp b/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp
index 5b957af74fd..a9743990626 100644
--- a/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp
+++ b/searchlib/src/vespa/searchlib/attribute/reference_attribute.cpp
@@ -430,7 +430,7 @@ ReferenceAttribute::populateTargetLids(const std::vector<GlobalId>& removes)
 }
 
 void
-ReferenceAttribute::clearDocs(DocId lidLow, DocId lidLimit)
+ReferenceAttribute::clearDocs(DocId lidLow, DocId lidLimit, bool)
 {
     assert(lidLow <= lidLimit);
     assert(lidLimit <= getNumDocs());
diff --git a/searchlib/src/vespa/searchlib/attribute/reference_attribute.h b/searchlib/src/vespa/searchlib/attribute/reference_attribute.h
index 96895fe2b35..b445d030b57 100644
--- a/searchlib/src/vespa/searchlib/attribute/reference_attribute.h
+++ b/searchlib/src/vespa/searchlib/attribute/reference_attribute.h
@@ -89,7 +89,7 @@ public:
     bool notifyReferencedRemoveNoCommit(const GlobalId &gid);
     void notifyReferencedRemove(const GlobalId &gid);
     void populateTargetLids(const std::vector<GlobalId>& removes);
-    void clearDocs(DocId lidLow, DocId lidLimit) override;
+    void clearDocs(DocId lidLow, DocId lidLimit, bool in_shrink_lid_space) override;
     void onShrinkLidSpace() override;
 
     template <typename FunctionType>
diff --git a/searchlib/src/vespa/searchlib/attribute/reference_mappings.cpp b/searchlib/src/vespa/searchlib/attribute/reference_mappings.cpp
index f922f7274ed..65bbd9514c7 100644
--- a/searchlib/src/vespa/searchlib/attribute/reference_mappings.cpp
+++ b/searchlib/src/vespa/searchlib/attribute/reference_mappings.cpp
@@ -8,7 +8,7 @@
 
 namespace search::attribute {
 
-ReferenceMappings::ReferenceMappings(GenerationHolder &genHolder, const uint32_t &committedDocIdLimit)
+ReferenceMappings::ReferenceMappings(GenerationHolder &genHolder, const std::atomic<uint32_t>& committedDocIdLimit)
     : _reverseMappingIndices(genHolder),
       _targetLidLimit(0),
       _reverseMapping(),
diff --git a/searchlib/src/vespa/searchlib/attribute/reference_mappings.h b/searchlib/src/vespa/searchlib/attribute/reference_mappings.h
index b42d0b04b82..e6bcceeef18 100644
--- a/searchlib/src/vespa/searchlib/attribute/reference_mappings.h
+++ b/searchlib/src/vespa/searchlib/attribute/reference_mappings.h
@@ -39,7 +39,7 @@ class ReferenceMappings
     ReverseMapping _reverseMapping;
     // vector containing target lid given source lid
     vespalib::RcuVectorBase<AtomicTargetLid> _targetLids;
-    const uint32_t &_committedDocIdLimit;
+    const std::atomic<uint32_t>& _committedDocIdLimit;
 
     void syncForwardMapping(const Reference &entry);
     void syncReverseMappingIndices(const Reference &entry);
@@ -49,7 +49,7 @@ public:
     // Class used to map from target lid to source lids
     using ReverseMappingRefs = vespalib::ConstArrayRef<AtomicEntryRef>;
 
-    ReferenceMappings(GenerationHolder &genHolder, const uint32_t &committedDocIdLimit);
+    ReferenceMappings(GenerationHolder &genHolder, const std::atomic<uint32_t>& committedDocIdLimit);
 
     ~ReferenceMappings();
 
@@ -85,12 +85,13 @@ public:
     foreach_lid(uint32_t targetLid, FunctionType &&func) const;
 
     TargetLids getTargetLids() const {
-        uint32_t committedDocIdLimit = _committedDocIdLimit;
+        uint32_t committedDocIdLimit = _committedDocIdLimit.load(std::memory_order_acquire);
         return TargetLids(&_targetLids.acquire_elem_ref(0), committedDocIdLimit);
     }
     uint32_t getTargetLid(uint32_t doc) const {
         // Check limit to avoid reading memory beyond end of valid mapping array
-        return doc < _committedDocIdLimit ? _targetLids.acquire_elem_ref(doc).load_acquire() : 0u;
+        uint32_t committed_doc_id_limit = _committedDocIdLimit.load(std::memory_order_acquire);
+        return doc < committed_doc_id_limit ? _targetLids.acquire_elem_ref(doc).load_acquire() : 0u;
     }
     ReverseMappingRefs getReverseMappingRefs() const {
         uint32_t targetLidLimit = _targetLidLimit.load(std::memory_order_acquire);
diff --git a/searchlib/src/vespa/searchlib/attribute/singleboolattribute.cpp b/searchlib/src/vespa/searchlib/attribute/singleboolattribute.cpp
index 474265c914b..f57f3336627 100644
--- a/searchlib/src/vespa/searchlib/attribute/singleboolattribute.cpp
+++ b/searchlib/src/vespa/searchlib/attribute/singleboolattribute.cpp
@@ -224,7 +224,7 @@ SingleBoolAttribute::onSave(IAttributeSaveTarget &saveTarget)
 }
 
 void
-SingleBoolAttribute::clearDocs(DocId lidLow, DocId lidLimit)
+SingleBoolAttribute::clearDocs(DocId lidLow, DocId lidLimit, bool)
 {
     assert(lidLow <= lidLimit);
     assert(lidLimit <= getNumDocs());
diff --git a/searchlib/src/vespa/searchlib/attribute/singleboolattribute.h b/searchlib/src/vespa/searchlib/attribute/singleboolattribute.h
index 469bd54fa55..31054f5e80a 100644
--- a/searchlib/src/vespa/searchlib/attribute/singleboolattribute.h
+++ b/searchlib/src/vespa/searchlib/attribute/singleboolattribute.h
@@ -24,7 +24,7 @@ public:
     void onUpdateStat() override;
     bool onLoad(vespalib::Executor *executor) override;
     void onSave(IAttributeSaveTarget &saveTarget) override;
-    void clearDocs(DocId lidLow, DocId lidLimit) override;
+    void clearDocs(DocId lidLow, DocId lidLimit, bool in_shrink_lid_space) override;
     void onShrinkLidSpace() override;
     void removeOldGenerations(generation_t firstUsed) override;
     void onGenerationChange(generation_t generation) override;
diff --git a/searchlib/src/vespa/searchlib/attribute/singleenumattribute.h b/searchlib/src/vespa/searchlib/attribute/singleenumattribute.h
index 1edcec4b9dd..f606b1afdab 100644
--- a/searchlib/src/vespa/searchlib/attribute/singleenumattribute.h
+++ b/searchlib/src/vespa/searchlib/attribute/singleenumattribute.h
@@ -129,7 +129,7 @@ public:
         return 1;
     }
 
-    void clearDocs(DocId lidLow, DocId lidLimit) override;
+    void clearDocs(DocId lidLow, DocId lidLimit, bool in_shrink_lid_space) override;
     void onShrinkLidSpace() override;
     std::unique_ptr<AttributeSaver> onInitSave(vespalib::stringref fileName) override;
     void onAddDocs(DocId lidLimit) override;
diff --git a/searchlib/src/vespa/searchlib/attribute/singleenumattribute.hpp b/searchlib/src/vespa/searchlib/attribute/singleenumattribute.hpp
index 608f37c0cfa..9f11b444c2c 100644
--- a/searchlib/src/vespa/searchlib/attribute/singleenumattribute.hpp
+++ b/searchlib/src/vespa/searchlib/attribute/singleenumattribute.hpp
@@ -287,7 +287,7 @@ SingleValueEnumAttribute<B>::onGenerationChange(generation_t generation)
 
 template <typename B>
 void
-SingleValueEnumAttribute<B>::clearDocs(DocId lidLow, DocId lidLimit)
+SingleValueEnumAttribute<B>::clearDocs(DocId lidLow, DocId lidLimit, bool)
 {
     EnumHandle e(0);
     bool findDefaultEnumRes(this->findEnum(this->getDefaultEnumTypeValue(), e));
diff --git a/searchlib/src/vespa/searchlib/attribute/singlenumericattribute.h b/searchlib/src/vespa/searchlib/attribute/singlenumericattribute.h
index ead825a0e9d..ccf55329634 100644
--- a/searchlib/src/vespa/searchlib/attribute/singlenumericattribute.h
+++ b/searchlib/src/vespa/searchlib/attribute/singlenumericattribute.h
@@ -171,7 +171,7 @@ public:
         return 0;
     }
 
-    void clearDocs(DocId lidLow, DocId lidLimit) override;
+    void clearDocs(DocId lidLow, DocId lidLimit, bool in_shrink_lid_space) override;
     void onShrinkLidSpace() override;
     std::unique_ptr<AttributeSaver> onInitSave(vespalib::stringref fileName) override;
 };
diff --git a/searchlib/src/vespa/searchlib/attribute/singlenumericattribute.hpp b/searchlib/src/vespa/searchlib/attribute/singlenumericattribute.hpp
index ef618262bcd..eeb73f1fc13 100644
--- a/searchlib/src/vespa/searchlib/attribute/singlenumericattribute.hpp
+++ b/searchlib/src/vespa/searchlib/attribute/singlenumericattribute.hpp
@@ -171,7 +171,7 @@ SingleValueNumericAttribute<B>::getSearch(QueryTermSimple::UP qTerm,
 
 template <typename B>
 void
-SingleValueNumericAttribute<B>::clearDocs(DocId lidLow, DocId lidLimit)
+SingleValueNumericAttribute<B>::clearDocs(DocId lidLow, DocId lidLimit, bool in_shrink_lid_space)
 {
     assert(lidLow <= lidLimit);
     assert(lidLimit <= this->getNumDocs());
@@ -182,6 +182,9 @@ SingleValueNumericAttribute<B>::clearDocs(DocId lidLow, DocId lidLimit)
             this->clearDoc(lid);
         }
         if ((++count % commit_interval) == 0) {
+            if (in_shrink_lid_space) {
+                this->clear_uncommitted_doc_id_limit();
+            }
             this->commit();
         }
     }
diff --git a/searchlib/src/vespa/searchlib/attribute/singlesmallnumericattribute.cpp b/searchlib/src/vespa/searchlib/attribute/singlesmallnumericattribute.cpp
index 24dec664547..ce232749514 100644
--- a/searchlib/src/vespa/searchlib/attribute/singlesmallnumericattribute.cpp
+++ b/searchlib/src/vespa/searchlib/attribute/singlesmallnumericattribute.cpp
@@ -176,7 +176,7 @@ SingleValueSmallNumericAttribute::getSearch(std::unique_ptr<QueryTermSimple> qTe
 }
 
 void
-SingleValueSmallNumericAttribute::clearDocs(DocId lidLow, DocId lidLimit)
+SingleValueSmallNumericAttribute::clearDocs(DocId lidLow, DocId lidLimit, bool)
 {
     assert(lidLow <= lidLimit);
     assert(lidLimit <= getNumDocs());
diff --git a/searchlib/src/vespa/searchlib/attribute/singlesmallnumericattribute.h b/searchlib/src/vespa/searchlib/attribute/singlesmallnumericattribute.h
index 9d95610c171..42d51b2a2c7 100644
--- a/searchlib/src/vespa/searchlib/attribute/singlesmallnumericattribute.h
+++ b/searchlib/src/vespa/searchlib/attribute/singlesmallnumericattribute.h
@@ -187,7 +187,7 @@ public:
         return 0;
     }
 
-    void clearDocs(DocId lidLow, DocId lidLimit) override;
+    void clearDocs(DocId lidLow, DocId lidLimit, bool in_shrink_lid_space) override;
     void onShrinkLidSpace() override;
     uint64_t getEstimatedSaveByteSize() const override;
 };
diff --git a/searchlib/src/vespa/searchlib/tensor/tensor_attribute.cpp b/searchlib/src/vespa/searchlib/tensor/tensor_attribute.cpp
index 5bd14d2c234..383f819659c 100644
--- a/searchlib/src/vespa/searchlib/tensor/tensor_attribute.cpp
+++ b/searchlib/src/vespa/searchlib/tensor/tensor_attribute.cpp
@@ -234,7 +234,7 @@ TensorAttribute::get_state(const vespalib::slime::Inserter& inserter) const
 }
 
 void
-TensorAttribute::clearDocs(DocId lidLow, DocId lidLimit)
+TensorAttribute::clearDocs(DocId lidLow, DocId lidLimit, bool)
 {
     assert(lidLow <= lidLimit);
     assert(lidLimit <= this->getNumDocs());
diff --git a/searchlib/src/vespa/searchlib/tensor/tensor_attribute.h b/searchlib/src/vespa/searchlib/tensor/tensor_attribute.h
index af671915b60..9e2f1aabab1 100644
--- a/searchlib/src/vespa/searchlib/tensor/tensor_attribute.h
+++ b/searchlib/src/vespa/searchlib/tensor/tensor_attribute.h
@@ -58,7 +58,7 @@ public:
     bool supports_get_tensor_ref() const override { return false; }
     const vespalib::eval::ValueType & getTensorType() const override;
     void get_state(const vespalib::slime::Inserter& inserter) const override;
-    void clearDocs(DocId lidLow, DocId lidLimit) override;
+    void clearDocs(DocId lidLow, DocId lidLimit, bool in_shrink_lid_space) override;
     void onShrinkLidSpace() override;
     uint32_t getVersion() const override;
     RefCopyVector getRefCopy() const;