Use AtomicEntryRef in single value enumerated attribute for mapping

from local doc id to enumerated value reference.

11 files changed, 73 insertions, 29 deletions

diff --git a/searchlib/src/vespa/searchlib/attribute/load_utils.cpp b/searchlib/src/vespa/searchlib/attribute/load_utils.cpp
index f71222e6faa..2c53003112a 100644
--- a/searchlib/src/vespa/searchlib/attribute/load_utils.cpp
+++ b/searchlib/src/vespa/searchlib/attribute/load_utils.cpp
@@ -12,6 +12,7 @@
 
 using search::multivalue::Value;
 using search::multivalue::WeightedValue;
+using vespalib::datastore::AtomicEntryRef;
 
 namespace search::attribute {
 
@@ -84,15 +85,15 @@ template uint32_t loadFromEnumeratedMultiValue(MultiValueMapping<Value<ValueType
 #define INSTANTIATE_WSET(ValueType, Saver) \
 template uint32_t loadFromEnumeratedMultiValue(MultiValueMapping<WeightedValue<ValueType>> &, ReaderBase &, vespalib::ConstArrayRef<ValueType>, vespalib::ConstArrayRef<uint32_t>, Saver)
 #define INSTANTIATE_SINGLE(ValueType, Saver) \
-template void loadFromEnumeratedSingleValue(vespalib::RcuVectorBase<ValueType> &, vespalib::GenerationHolder &, ReaderBase &, vespalib::ConstArrayRef<ValueType>, vespalib::ConstArrayRef<uint32_t>, Saver)
+template void loadFromEnumeratedSingleValue(vespalib::RcuVectorBase<ValueType> &, vespalib::GenerationHolder &, ReaderBase &, vespalib::ConstArrayRef<load_utils::NonAtomicValue_t<ValueType>>, vespalib::ConstArrayRef<uint32_t>, Saver)
 
 #define INSTANTIATE_SINGLE_ARRAY_WSET(ValueType, Saver) \
 INSTANTIATE_SINGLE(ValueType, Saver); \
-INSTANTIATE_ARRAY(ValueType, Saver); \
-INSTANTIATE_WSET(ValueType, Saver)
+INSTANTIATE_ARRAY(load_utils::NonAtomicValue_t<ValueType>, Saver); \
+INSTANTIATE_WSET(load_utils::NonAtomicValue_t<ValueType>, Saver)
 
 #define INSTANTIATE_ENUM(Saver) \
-INSTANTIATE_SINGLE_ARRAY_WSET(IEnumStore::Index, Saver)
+INSTANTIATE_SINGLE_ARRAY_WSET(AtomicEntryRef, Saver)
 
 #define INSTANTIATE_VALUE(ValueType) \
 INSTANTIATE_SINGLE_ARRAY_WSET(ValueType, NoSaveLoadedEnum)
diff --git a/searchlib/src/vespa/searchlib/attribute/load_utils.h b/searchlib/src/vespa/searchlib/attribute/load_utils.h
index a9a41b8eaeb..fe41811dcfa 100644
--- a/searchlib/src/vespa/searchlib/attribute/load_utils.h
+++ b/searchlib/src/vespa/searchlib/attribute/load_utils.h
@@ -6,8 +6,39 @@
 #include "readerbase.h"
 #include <vespa/vespalib/util/arrayref.h>
 
+namespace vespalib::datastore {
+
+class AtomicEntryRef;
+class EntryRef;
+
+}
+
 namespace search::attribute {
 
+namespace load_utils {
+
+/*
+ * Helper class to map from atomic value to non-atomic value, e.g.
+ * from AtomicEntryRef to EntryRef.
+ */
+template <typename MaybeAtomicValue>
+class NonAtomicValue {
+public:
+    using type = MaybeAtomicValue;
+};
+
+template <>
+class NonAtomicValue<vespalib::datastore::AtomicEntryRef>
+{
+public:
+    using type = vespalib::datastore::EntryRef;
+};
+
+template <class MaybeAtomicValue>
+using NonAtomicValue_t = typename NonAtomicValue<MaybeAtomicValue>::type;
+
+}
+
 /**
  * Helper functions used to open / load attribute vector data files from disk.
  */
@@ -51,7 +82,7 @@ void
 loadFromEnumeratedSingleValue(Vector &vector,
                               vespalib::GenerationHolder &genHolder,
                               ReaderBase &attrReader,
-                              vespalib::ConstArrayRef<typename Vector::ValueType> enumValueToValueMap,
+                              vespalib::ConstArrayRef<load_utils::NonAtomicValue_t<typename Vector::ValueType>> enumValueToValueMap,
                               vespalib::ConstArrayRef<uint32_t> enum_value_remapping,
                               Saver saver) __attribute((noinline));
 
diff --git a/searchlib/src/vespa/searchlib/attribute/load_utils.hpp b/searchlib/src/vespa/searchlib/attribute/load_utils.hpp
index 50566b36e75..92cbc72ae2c 100644
--- a/searchlib/src/vespa/searchlib/attribute/load_utils.hpp
+++ b/searchlib/src/vespa/searchlib/attribute/load_utils.hpp
@@ -54,10 +54,12 @@ void
 loadFromEnumeratedSingleValue(Vector &vector,
                               vespalib::GenerationHolder &genHolder,
                               ReaderBase &attrReader,
-                              vespalib::ConstArrayRef<typename Vector::ValueType> enumValueToValueMap,
+                              vespalib::ConstArrayRef<load_utils::NonAtomicValue_t<typename Vector::ValueType>> enumValueToValueMap,
                               vespalib::ConstArrayRef<uint32_t> enum_value_remapping,
                               Saver saver)
 {
+    using ValueType = typename Vector::ValueType;
+    using NonAtomicValueType = load_utils::NonAtomicValue_t<ValueType>;
     uint32_t numDocs = attrReader.getEnumCount();
     genHolder.clearHoldLists();
     vector.reset();
@@ -68,7 +70,11 @@ loadFromEnumeratedSingleValue(Vector &vector,
         if (!enum_value_remapping.empty()) {
             enumValue = enum_value_remapping[enumValue];
         }
-        vector.push_back(enumValueToValueMap[enumValue]);
+        if constexpr (std::is_same_v<ValueType, NonAtomicValueType>) {
+            vector.push_back(enumValueToValueMap[enumValue]);
+        } else {
+            vector.push_back(ValueType(enumValueToValueMap[enumValue]));
+        }
         saver.save(enumValue, doc, 1);
     }
 }
diff --git a/searchlib/src/vespa/searchlib/attribute/singleenumattribute.cpp b/searchlib/src/vespa/searchlib/attribute/singleenumattribute.cpp
index cffd52f3dc4..b5de652fbe2 100644
--- a/searchlib/src/vespa/searchlib/attribute/singleenumattribute.cpp
+++ b/searchlib/src/vespa/searchlib/attribute/singleenumattribute.cpp
@@ -32,7 +32,7 @@ AttributeVector::DocId
 SingleValueEnumAttributeBase::addDoc(bool &incGeneration)
 {
     incGeneration = _enumIndices.isFull();
-    _enumIndices.push_back(IEnumStore::Index());
+    _enumIndices.push_back(AtomicEntryRef());
     return _enumIndices.size() - 1;
 }
 
@@ -41,7 +41,12 @@ SingleValueEnumAttributeBase::EnumIndexCopyVector
 SingleValueEnumAttributeBase::getIndicesCopy(uint32_t size) const
 {
     assert(size <= _enumIndices.size());
-    return EnumIndexCopyVector(&_enumIndices[0], &_enumIndices[0] + size);
+    EnumIndexCopyVector result;
+    result.reserve(size);
+    for (uint32_t lid = 0; lid < size; ++lid) {
+        result.push_back(_enumIndices[lid].load_relaxed());
+    }
+    return result;
 }
 
 void
@@ -54,11 +59,11 @@ SingleValueEnumAttributeBase::remap_enum_store_refs(const EnumIndexRemapper& rem
     v.logEnumStoreEvent("reenumerate", "start");
     auto& filter = remapper.get_entry_ref_filter();
     for (uint32_t i = 0; i < _enumIndices.size(); ++i) {
-        EnumIndex ref = _enumIndices[i];
+        EnumIndex ref = _enumIndices[i].load_relaxed();
         if (ref.valid() && filter.has(ref)) {
             ref = remapper.remap(ref);
         }
-        new_indexes.push_back_fast(ref);
+        new_indexes.push_back_fast(AtomicEntryRef(ref));
     }
     v.logEnumStoreEvent("compactfixup", "drain");
     {
diff --git a/searchlib/src/vespa/searchlib/attribute/singleenumattribute.h b/searchlib/src/vespa/searchlib/attribute/singleenumattribute.h
index 8fcef670fb0..4ce55902d7f 100644
--- a/searchlib/src/vespa/searchlib/attribute/singleenumattribute.h
+++ b/searchlib/src/vespa/searchlib/attribute/singleenumattribute.h
@@ -17,24 +17,25 @@ class ReaderBase;
  */
 class SingleValueEnumAttributeBase {
 protected:
+    using AtomicEntryRef = vespalib::datastore::AtomicEntryRef;
+    using AtomicEntryRefVector = vespalib::RcuVectorBase<AtomicEntryRef>;
     using DocId = AttributeVector::DocId;
     using EnumHandle = AttributeVector::EnumHandle;
     using EnumIndex = IEnumStore::Index;
-    using EnumIndexVector = vespalib::RcuVectorBase<EnumIndex>;
     using EnumIndexRemapper = IEnumStore::EnumIndexRemapper;
     using GenerationHolder = vespalib::GenerationHolder;
 
 public:
     using EnumIndexCopyVector = vespalib::Array<EnumIndex>;
 
-    IEnumStore::Index getEnumIndex(DocId docId) const { return _enumIndices[docId]; }
-    EnumHandle getE(DocId doc) const { return _enumIndices[doc].ref(); }
+    IEnumStore::Index getEnumIndex(DocId docId) const { return _enumIndices[docId].load_acquire(); }
+    EnumHandle getE(DocId doc) const { return _enumIndices[doc].load_acquire().ref(); }
 protected:
     SingleValueEnumAttributeBase(const attribute::Config & c, GenerationHolder &genHolder, const vespalib::alloc::Alloc& initial_alloc);
     ~SingleValueEnumAttributeBase();
     AttributeVector::DocId addDoc(bool & incGeneration);
 
-    EnumIndexVector _enumIndices;
+    AtomicEntryRefVector _enumIndices;
 
     EnumIndexCopyVector getIndicesCopy(uint32_t size) const;
     void remap_enum_store_refs(const EnumIndexRemapper& remapper, AttributeVector& v);
diff --git a/searchlib/src/vespa/searchlib/attribute/singleenumattribute.hpp b/searchlib/src/vespa/searchlib/attribute/singleenumattribute.hpp
index 56aa6672696..bb454a8b0d4 100644
--- a/searchlib/src/vespa/searchlib/attribute/singleenumattribute.hpp
+++ b/searchlib/src/vespa/searchlib/attribute/singleenumattribute.hpp
@@ -53,9 +53,9 @@ SingleValueEnumAttribute<B>::addDoc(DocId & doc)
     if (doc > 0u) {
         // Make sure that a valid value(magic default) is referenced,
         // even between addDoc and commit().
-        if (_enumIndices[0].valid()) {
+        if (_enumIndices[0].load_relaxed().valid()) {
             _enumIndices[doc] = _enumIndices[0];
-            this->_enumStore.inc_ref_count(_enumIndices[0]);
+            this->_enumStore.inc_ref_count(_enumIndices[0].load_relaxed());
         }
     }
     this->incNumDocs();
@@ -166,7 +166,7 @@ template <typename B>
 void
 SingleValueEnumAttribute<B>::applyUpdateValueChange(const Change& c, EnumStoreBatchUpdater& updater)
 {
-    EnumIndex oldIdx = _enumIndices[c._doc];
+    EnumIndex oldIdx = _enumIndices[c._doc].load_relaxed();
     EnumIndex newIdx;
     if (c.has_entry_ref()) {
         newIdx = EnumIndex(vespalib::datastore::EntryRef(c.get_entry_ref()));
@@ -204,7 +204,7 @@ SingleValueEnumAttribute<B>::updateEnumRefCounts(const Change& c, EnumIndex newI
                                                  EnumStoreBatchUpdater& updater)
 {
     updater.inc_ref_count(newIdx);
-    _enumIndices[c._doc] = newIdx;
+    _enumIndices[c._doc].store_release(newIdx);
     if (oldIdx.valid()) {
         updater.dec_ref_count(oldIdx);
     }
@@ -220,7 +220,7 @@ SingleValueEnumAttribute<B>::fillValues(LoadedVector & loaded)
         _enumIndices.reset();
         _enumIndices.unsafe_reserve(numDocs);
         for (DocId doc = 0; doc < numDocs; ++doc, loaded.next()) {
-            _enumIndices.push_back(loaded.read().getEidx());
+            _enumIndices.push_back(AtomicEntryRef(loaded.read().getEidx()));
         }
     }
 }
@@ -296,7 +296,7 @@ SingleValueEnumAttribute<B>::clearDocs(DocId lidLow, DocId lidLimit)
     assert(lidLow <= lidLimit);
     assert(lidLimit <= this->getNumDocs());
     for (DocId lid = lidLow; lid < lidLimit; ++lid) {
-        if (_enumIndices[lid] != vespalib::datastore::EntryRef(e)) {
+        if (_enumIndices[lid].load_relaxed() != vespalib::datastore::EntryRef(e)) {
             this->clearDoc(lid);
         }
     }
diff --git a/searchlib/src/vespa/searchlib/attribute/singlenumericenumattribute.h b/searchlib/src/vespa/searchlib/attribute/singlenumericenumattribute.h
index 4383527a4a4..9ef2d680f05 100644
--- a/searchlib/src/vespa/searchlib/attribute/singlenumericenumattribute.h
+++ b/searchlib/src/vespa/searchlib/attribute/singlenumericenumattribute.h
@@ -103,7 +103,7 @@ public:
     // Attribute read API
     //-------------------------------------------------------------------------
     T get(DocId doc) const override {
-        return this->_enumStore.get_value(this->_enumIndices[doc]);
+        return this->_enumStore.get_value(this->_enumIndices[doc].load_acquire());
     }
     largeint_t getInt(DocId doc) const override {
         return static_cast<largeint_t>(get(doc));
diff --git a/searchlib/src/vespa/searchlib/attribute/singlenumericenumattribute.hpp b/searchlib/src/vespa/searchlib/attribute/singlenumericenumattribute.hpp
index c6b720ba14e..215123e9b5b 100644
--- a/searchlib/src/vespa/searchlib/attribute/singlenumericenumattribute.hpp
+++ b/searchlib/src/vespa/searchlib/attribute/singlenumericenumattribute.hpp
@@ -50,7 +50,7 @@ template <typename B>
 void
 SingleValueNumericEnumAttribute<B>::applyArithmeticValueChange(const Change& c, EnumStoreBatchUpdater& updater)
 {
-    EnumIndex oldIdx = this->_enumIndices[c._doc];
+    EnumIndex oldIdx = this->_enumIndices[c._doc].load_relaxed();
     EnumIndex newIdx;
     T newValue = this->template applyArithmetic<T, typename Change::DataType>(get(c._doc), c._data.getArithOperand(), c._type);
     this->_enumStore.find_index(newValue, newIdx);
diff --git a/searchlib/src/vespa/searchlib/attribute/singlenumericpostattribute.hpp b/searchlib/src/vespa/searchlib/attribute/singlenumericpostattribute.hpp
index 4d37171e151..b87dadb5429 100644
--- a/searchlib/src/vespa/searchlib/attribute/singlenumericpostattribute.hpp
+++ b/searchlib/src/vespa/searchlib/attribute/singlenumericpostattribute.hpp
@@ -64,7 +64,7 @@ makePostingChange(const vespalib::datastore::EntryComparator &cmpa,
 {
     for (const auto& elem : currEnumIndices) {
         uint32_t docId = elem.first;
-        EnumIndex oldIdx = this->_enumIndices[docId];
+        EnumIndex oldIdx = this->_enumIndices[docId].load_relaxed();
         EnumIndex newIdx = elem.second;
 
         // add new posting
@@ -97,7 +97,7 @@ SingleValueNumericPostingAttribute<B>::applyValueChanges(EnumStoreBatchUpdater&
         if (enumIter != currEnumIndices.end()) {
             oldIdx = enumIter->second;
         } else {
-            oldIdx = this->_enumIndices[change._doc];
+            oldIdx = this->_enumIndices[change._doc].load_relaxed();
         }
 
         if (change._type == ChangeBase::UPDATE) {
diff --git a/searchlib/src/vespa/searchlib/attribute/singlestringattribute.h b/searchlib/src/vespa/searchlib/attribute/singlestringattribute.h
index cd0fe5cfb9a..45bb007e737 100644
--- a/searchlib/src/vespa/searchlib/attribute/singlestringattribute.h
+++ b/searchlib/src/vespa/searchlib/attribute/singlestringattribute.h
@@ -45,7 +45,7 @@ public:
     //-------------------------------------------------------------------------
     bool isUndefined(DocId doc) const override { return get(doc)[0] == '\0'; }
     const char * get(DocId doc) const override {
-        return this->_enumStore.get_value(this->_enumIndices[doc]);
+        return this->_enumStore.get_value(this->_enumIndices[doc].load_acquire());
     }
     std::vector<EnumHandle> findFoldedEnums(const char *value) const override {
         return this->_enumStore.find_folded_enums(value);
@@ -95,7 +95,7 @@ public:
         int32_t onFind(DocId doc, int32_t elemId) const override {
             if ( elemId != 0) return -1;
             const SingleValueStringAttributeT<B> & attr(static_cast<const SingleValueStringAttributeT<B> &>(attribute()));
-            return isMatch(attr._enumStore.get_value(attr._enumIndices[doc])) ? 0 : -1;
+            return isMatch(attr._enumStore.get_value(attr._enumIndices[doc].load_acquire())) ? 0 : -1;
         }
 
     };
diff --git a/searchlib/src/vespa/searchlib/attribute/singlestringpostattribute.hpp b/searchlib/src/vespa/searchlib/attribute/singlestringpostattribute.hpp
index 4670ee075fe..58c62e60efe 100644
--- a/searchlib/src/vespa/searchlib/attribute/singlestringpostattribute.hpp
+++ b/searchlib/src/vespa/searchlib/attribute/singlestringpostattribute.hpp
@@ -64,7 +64,7 @@ makePostingChange(const vespalib::datastore::EntryComparator &cmpa,
 {
     for (const auto& elem : currEnumIndices) {
         uint32_t docId = elem.first;
-        EnumIndex oldIdx = this->_enumIndices[docId];
+        EnumIndex oldIdx = this->_enumIndices[docId].load_relaxed();
         EnumIndex newIdx = elem.second;
 
         // add new posting
@@ -98,7 +98,7 @@ SingleValueStringPostingAttributeT<B>::applyValueChanges(EnumStoreBatchUpdater&
         if (enumIter != currEnumIndices.end()) {
             oldIdx = enumIter->second;
         } else {
-            oldIdx = this->_enumIndices[change._doc];
+            oldIdx = this->_enumIndices[change._doc].load_relaxed();
         }
         if (change._type == ChangeBase::UPDATE) {
             applyUpdateValueChange(change, enumStore, currEnumIndices);