Merge pull request #17225 from vespa-engine/toregge/factor-out-unique-store-dictionary-read-snapshot

Factor out ReadSnapshot from UniqueStoreDictionary.

9 files changed, 113 insertions, 75 deletions

diff --git a/searchlib/src/vespa/searchlib/attribute/enumhintsearchcontext.h b/searchlib/src/vespa/searchlib/attribute/enumhintsearchcontext.h
index f649754e0fe..af64f17934d 100644
--- a/searchlib/src/vespa/searchlib/attribute/enumhintsearchcontext.h
+++ b/searchlib/src/vespa/searchlib/attribute/enumhintsearchcontext.h
@@ -6,7 +6,10 @@
 #include "ipostinglistsearchcontext.h"
 #include <vespa/searchlib/queryeval/searchiterator.h>
 
-namespace vespalib::datastore { class EntryComparator; }
+namespace vespalib::datastore {
+class EntryComparator;
+class IUniqueStoreDictionaryReadSnapshot;
+}
 
 namespace search::attribute {
 
@@ -17,7 +20,7 @@ namespace search::attribute {
 
 class EnumHintSearchContext : public IPostingListSearchContext
 {
-    const IEnumStoreDictionary::ReadSnapshot::UP _dict_snapshot;
+    const std::unique_ptr<vespalib::datastore::IUniqueStoreDictionaryReadSnapshot> _dict_snapshot;
     uint32_t                _uniqueValues;
     uint32_t                _docIdLimit;
     uint64_t                _numValues; // attr.getStatus().getNumValues();
diff --git a/vespalib/src/tests/datastore/unique_store_dictionary/unique_store_dictionary_test.cpp b/vespalib/src/tests/datastore/unique_store_dictionary/unique_store_dictionary_test.cpp
index 2c1b4ad3cf1..664d4fe8acd 100644
--- a/vespalib/src/tests/datastore/unique_store_dictionary/unique_store_dictionary_test.cpp
+++ b/vespalib/src/tests/datastore/unique_store_dictionary/unique_store_dictionary_test.cpp
@@ -41,7 +41,7 @@ public:
 template <typename UniqueStoreDictionaryType>
 struct DictionaryReadTest : public ::testing::Test {
     UniqueStoreDictionaryType dict;
-    IUniqueStoreDictionary::ReadSnapshot::UP snapshot;
+    std::unique_ptr<IUniqueStoreDictionaryReadSnapshot> snapshot;
 
     DictionaryReadTest()
         : dict(std::make_unique<Comparator>(0)),
diff --git a/vespalib/src/vespa/vespalib/datastore/i_unique_store_dictionary.h b/vespalib/src/vespa/vespalib/datastore/i_unique_store_dictionary.h
index d31139efc76..3758dea9cfe 100644
--- a/vespalib/src/vespa/vespalib/datastore/i_unique_store_dictionary.h
+++ b/vespalib/src/vespa/vespalib/datastore/i_unique_store_dictionary.h
@@ -12,6 +12,7 @@ namespace vespalib::datastore {
 
 class EntryComparator;
 struct ICompactable;
+class IUniqueStoreDictionaryReadSnapshot;
 class UniqueStoreAddResult;
 
 /**
@@ -19,20 +20,6 @@ class UniqueStoreAddResult;
  */
 class IUniqueStoreDictionary {
 public:
-    /**
-     * Class that provides a read snapshot of the dictionary.
-     *
-     * A generation guard that must be taken and held while the snapshot is considered valid.
-     */
-    class ReadSnapshot {
-    public:
-        using UP = std::unique_ptr<ReadSnapshot>;
-        virtual ~ReadSnapshot() = default;
-        virtual size_t count(const EntryComparator& comp) const = 0;
-        virtual size_t count_in_range(const EntryComparator& low, const EntryComparator& high) const = 0;
-        virtual void foreach_key(std::function<void(EntryRef)> callback) const = 0;
-    };
-
     using generation_t = vespalib::GenerationHandler::generation_t;
     virtual ~IUniqueStoreDictionary() = default;
     virtual void freeze() = 0;
@@ -47,7 +34,7 @@ public:
     virtual void build(vespalib::ConstArrayRef<EntryRef>, vespalib::ConstArrayRef<uint32_t> ref_counts, std::function<void(EntryRef)> hold) = 0;
     virtual void build(vespalib::ConstArrayRef<EntryRef> refs) = 0;
     virtual void build_with_payload(vespalib::ConstArrayRef<EntryRef> refs, vespalib::ConstArrayRef<uint32_t> payloads) = 0;
-    virtual std::unique_ptr<ReadSnapshot> get_read_snapshot() const = 0;
+    virtual std::unique_ptr<IUniqueStoreDictionaryReadSnapshot> get_read_snapshot() const = 0;
     virtual bool get_has_hash_dictionary() const = 0;
 };
 
diff --git a/vespalib/src/vespa/vespalib/datastore/i_unique_store_dictionary_read_snapshot.h b/vespalib/src/vespa/vespalib/datastore/i_unique_store_dictionary_read_snapshot.h
new file mode 100644
index 00000000000..62b2b330a67
--- /dev/null
+++ b/vespalib/src/vespa/vespalib/datastore/i_unique_store_dictionary_read_snapshot.h
@@ -0,0 +1,25 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#pragma once
+
+#include "entryref.h"
+#include <functional>
+
+namespace vespalib::datastore {
+
+class EntryComparator;
+
+/**
+ * Class that provides a read snapshot of the dictionary.
+ *
+ * A generation guard that must be taken and held while the snapshot is considered valid.
+ */
+class IUniqueStoreDictionaryReadSnapshot {
+public:
+    virtual ~IUniqueStoreDictionaryReadSnapshot() = default;
+    virtual size_t count(const EntryComparator& comp) const = 0;
+    virtual size_t count_in_range(const EntryComparator& low, const EntryComparator& high) const = 0;
+    virtual void foreach_key(std::function<void(EntryRef)> callback) const = 0;
+};
+
+}
diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store_btree_dictionary_read_snapshot.h b/vespalib/src/vespa/vespalib/datastore/unique_store_btree_dictionary_read_snapshot.h
new file mode 100644
index 00000000000..05406e5704d
--- /dev/null
+++ b/vespalib/src/vespa/vespalib/datastore/unique_store_btree_dictionary_read_snapshot.h
@@ -0,0 +1,28 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#pragma once
+
+#include "i_unique_store_dictionary_read_snapshot.h"
+
+namespace vespalib::datastore {
+
+/**
+ * Class that provides a read snapshot of a unique store dictionary using a btree.
+ *
+ * A generation guard that must be taken and held while the snapshot is considered valid.
+ */
+template <typename BTreeDictionaryT>
+class UniqueStoreBTreeDictionaryReadSnapshot : public IUniqueStoreDictionaryReadSnapshot {
+private:
+    using BTreeDictionaryType = BTreeDictionaryT;
+    using FrozenView = typename BTreeDictionaryType::FrozenView;
+    FrozenView _frozen_view;
+    
+public:
+    UniqueStoreBTreeDictionaryReadSnapshot(FrozenView frozen_view);
+    size_t count(const EntryComparator& comp) const override;
+    size_t count_in_range(const EntryComparator& low, const EntryComparator& high) const override;
+    void foreach_key(std::function<void(EntryRef)> callback) const override;
+};
+
+}
diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store_btree_dictionary_read_snapshot.hpp b/vespalib/src/vespa/vespalib/datastore/unique_store_btree_dictionary_read_snapshot.hpp
new file mode 100644
index 00000000000..ec1ae63aa4b
--- /dev/null
+++ b/vespalib/src/vespa/vespalib/datastore/unique_store_btree_dictionary_read_snapshot.hpp
@@ -0,0 +1,45 @@
+// Copyright Verizon Media. Licensed under the terms of the Apache 2.0 license. See LICENSE in the project root.
+
+#pragma once
+
+#include "unique_store_btree_dictionary_read_snapshot.h"
+
+namespace vespalib::datastore {
+
+template <typename BTreeDictionaryT>
+UniqueStoreBTreeDictionaryReadSnapshot<BTreeDictionaryT>::UniqueStoreBTreeDictionaryReadSnapshot(FrozenView frozen_view)
+    : _frozen_view(frozen_view)
+{
+}
+
+template <typename BTreeDictionaryT>
+size_t
+UniqueStoreBTreeDictionaryReadSnapshot<BTreeDictionaryT>::count(const EntryComparator& comp) const
+{
+    auto itr = _frozen_view.lowerBound(EntryRef(), comp);
+    if (itr.valid() && !comp.less(EntryRef(), itr.getKey())) {
+        return 1u;
+    }
+    return 0u;
+}
+
+template <typename BTreeDictionaryT>
+size_t
+UniqueStoreBTreeDictionaryReadSnapshot<BTreeDictionaryT>::count_in_range(const EntryComparator& low, const EntryComparator& high) const
+{
+    auto low_itr = _frozen_view.lowerBound(EntryRef(), low);
+    auto high_itr = low_itr;
+    if (high_itr.valid() && !high.less(EntryRef(), high_itr.getKey())) {
+        high_itr.seekPast(EntryRef(), high);
+    }
+    return high_itr - low_itr;
+}
+
+template <typename BTreeDictionaryT>
+void
+UniqueStoreBTreeDictionaryReadSnapshot<BTreeDictionaryT>::foreach_key(std::function<void(EntryRef)> callback) const
+{
+    _frozen_view.foreach_key(callback);
+}
+
+}
diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.h b/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.h
index 8fbff3f9d93..66b82ace1c1 100644
--- a/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.h
+++ b/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.h
@@ -8,6 +8,7 @@
 namespace vespalib::datastore {
 
 class EntryComparatorWrapper;
+class IUniqueStoreDictionaryReadSnapshot;
 
 class NoHashDictionary;
 
@@ -56,20 +57,8 @@ protected:
     using BTreeDictionaryType = BTreeDictionaryT;
     using DataType = typename BTreeDictionaryType::DataType;
     using FrozenView = typename BTreeDictionaryType::FrozenView;
-    using ReadSnapshot = typename ParentT::ReadSnapshot;
     using generation_t = typename ParentT::generation_t;
 
-    class ReadSnapshotImpl : public ReadSnapshot {
-    private:
-        FrozenView _frozen_view;
-
-    public:
-        ReadSnapshotImpl(FrozenView frozen_view);
-        size_t count(const EntryComparator& comp) const override;
-        size_t count_in_range(const EntryComparator& low, const EntryComparator& high) const override;
-        void foreach_key(std::function<void(EntryRef)> callback) const override;
-    };
-
 public:
     using UniqueStoreBTreeDictionaryBase<BTreeDictionaryT>::has_btree_dictionary;
     using UniqueStoreHashDictionaryBase<HashDictionaryT>::has_hash_dictionary;
@@ -87,7 +76,7 @@ public:
     void build(vespalib::ConstArrayRef<EntryRef>, vespalib::ConstArrayRef<uint32_t> ref_counts, std::function<void(EntryRef)> hold) override;
     void build(vespalib::ConstArrayRef<EntryRef> refs) override;
     void build_with_payload(vespalib::ConstArrayRef<EntryRef>, vespalib::ConstArrayRef<uint32_t> payloads) override;
-    std::unique_ptr<ReadSnapshot> get_read_snapshot() const override;
+    std::unique_ptr<IUniqueStoreDictionaryReadSnapshot> get_read_snapshot() const override;
     bool get_has_hash_dictionary() const override;
 };
 
diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.hpp b/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.hpp
index edacb306c6e..9503a54c645 100644
--- a/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.hpp
+++ b/vespalib/src/vespa/vespalib/datastore/unique_store_dictionary.hpp
@@ -7,6 +7,7 @@
 #include "i_compactable.h"
 #include "unique_store_add_result.h"
 #include "unique_store_dictionary.h"
+#include "unique_store_btree_dictionary_read_snapshot.hpp"
 #include <vespa/vespalib/btree/btree.hpp>
 #include <vespa/vespalib/btree/btreebuilder.hpp>
 #include <vespa/vespalib/btree/btreeiterator.hpp>
@@ -17,47 +18,6 @@
 namespace vespalib::datastore {
 
 template <typename BTreeDictionaryT, typename ParentT, typename HashDictionaryT>
-UniqueStoreDictionary<BTreeDictionaryT, ParentT, HashDictionaryT>::
-ReadSnapshotImpl::ReadSnapshotImpl(FrozenView frozen_view)
-    : _frozen_view(frozen_view)
-{
-}
-
-template <typename BTreeDictionaryT, typename ParentT, typename HashDictionaryT>
-size_t
-UniqueStoreDictionary<BTreeDictionaryT, ParentT, HashDictionaryT>::
-ReadSnapshotImpl::count(const EntryComparator& comp) const
-{
-    auto itr = _frozen_view.lowerBound(EntryRef(), comp);
-    if (itr.valid() && !comp.less(EntryRef(), itr.getKey())) {
-        return 1u;
-    }
-    return 0u;
-}
-
-template <typename BTreeDictionaryT, typename ParentT, typename HashDictionaryT>
-size_t
-UniqueStoreDictionary<BTreeDictionaryT, ParentT, HashDictionaryT>::
-ReadSnapshotImpl::count_in_range(const EntryComparator& low,
-                                 const EntryComparator& high) const
-{
-    auto low_itr = _frozen_view.lowerBound(EntryRef(), low);
-    auto high_itr = low_itr;
-    if (high_itr.valid() && !high.less(EntryRef(), high_itr.getKey())) {
-        high_itr.seekPast(EntryRef(), high);
-    }
-    return high_itr - low_itr;
-}
-
-template <typename BTreeDictionaryT, typename ParentT, typename HashDictionaryT>
-void
-UniqueStoreDictionary<BTreeDictionaryT, ParentT, HashDictionaryT>::
-ReadSnapshotImpl::foreach_key(std::function<void(EntryRef)> callback) const
-{
-    _frozen_view.foreach_key(callback);
-}
-
-template <typename BTreeDictionaryT, typename ParentT, typename HashDictionaryT>
 UniqueStoreDictionary<BTreeDictionaryT, ParentT, HashDictionaryT>::UniqueStoreDictionary(std::unique_ptr<EntryComparator> compare)
     : ParentT(),
       UniqueStoreBTreeDictionaryBase<BTreeDictionaryT>(),
@@ -272,10 +232,10 @@ UniqueStoreDictionary<BTreeDictionaryT, ParentT, HashDictionaryT>::build_with_pa
 }
 
 template <typename BTreeDictionaryT, typename ParentT, typename HashDictionaryT>
-std::unique_ptr<typename ParentT::ReadSnapshot>
+std::unique_ptr<IUniqueStoreDictionaryReadSnapshot>
 UniqueStoreDictionary<BTreeDictionaryT, ParentT, HashDictionaryT>::get_read_snapshot() const
 {
-    return std::make_unique<ReadSnapshotImpl>(this->_btree_dict.getFrozenView());
+    return std::make_unique<UniqueStoreBTreeDictionaryReadSnapshot<BTreeDictionaryT>>(this->_btree_dict.getFrozenView());
 }
 
 template <typename BTreeDictionaryT, typename ParentT, typename HashDictionaryT>
diff --git a/vespalib/src/vespa/vespalib/datastore/unique_store_enumerator.h b/vespalib/src/vespa/vespalib/datastore/unique_store_enumerator.h
index 78597a53dc8..be591649310 100644
--- a/vespalib/src/vespa/vespalib/datastore/unique_store_enumerator.h
+++ b/vespalib/src/vespa/vespalib/datastore/unique_store_enumerator.h
@@ -3,6 +3,7 @@
 #pragma once
 
 #include "i_unique_store_dictionary.h"
+#include "i_unique_store_dictionary_read_snapshot.h"
 #include <vespa/vespalib/stllike/allocator.h>
 
 namespace vespalib::datastore {
@@ -23,7 +24,7 @@ public:
 private:
     using UInt32Vector = std::vector<uint32_t, vespalib::allocator_large<uint32_t>>;
     using EnumValues = std::vector<UInt32Vector>;
-    IUniqueStoreDictionary::ReadSnapshot::UP _dict_snapshot;
+    std::unique_ptr<IUniqueStoreDictionaryReadSnapshot> _dict_snapshot;
     const DataStoreBase &_store;
     EnumValues _enumValues;
     uint32_t _next_enum_val;