- Signal that we are using the default comaparator with no additional or mutable state.

- Separate refering the stateless comparator, and creating new statefull comparators.
- Keep the Comparator creation close.

12 files changed, 69 insertions, 53 deletions

diff --git a/searchlib/src/tests/attribute/enum_comparator/enum_comparator_test.cpp b/searchlib/src/tests/attribute/enum_comparator/enum_comparator_test.cpp
index a9efc22dc5a..3ab29797120 100644
--- a/searchlib/src/tests/attribute/enum_comparator/enum_comparator_test.cpp
+++ b/searchlib/src/tests/attribute/enum_comparator/enum_comparator_test.cpp
@@ -30,7 +30,7 @@ TEST("requireThatNumericLessIsWorking")
     NumericEnumStore es(false, DictionaryConfig::Type::BTREE);
     EnumIndex e1 = es.insert(10);
     EnumIndex e2 = es.insert(30);
-    auto cmp1 = es.make_comparator();
+    const auto & cmp1 = es.get_comparator();
     EXPECT_TRUE(cmp1.less(e1, e2));
     EXPECT_FALSE(cmp1.less(e2, e1));
     EXPECT_FALSE(cmp1.less(e1, e1));
@@ -44,7 +44,7 @@ TEST("requireThatNumericEqualIsWorking")
     NumericEnumStore es(false, DictionaryConfig::Type::BTREE);
     EnumIndex e1 = es.insert(10);
     EnumIndex e2 = es.insert(30);
-    auto cmp1 = es.make_comparator();
+    const auto & cmp1 = es.get_comparator();
     EXPECT_FALSE(cmp1.equal(e1, e2));
     EXPECT_FALSE(cmp1.equal(e2, e1));
     EXPECT_TRUE(cmp1.equal(e1, e1));
@@ -60,7 +60,7 @@ TEST("requireThatFloatLessIsWorking")
     EnumIndex e1 = es.insert(10.5);
     EnumIndex e2 = es.insert(30.5);
     EnumIndex e3 = es.insert(std::numeric_limits<float>::quiet_NaN());
-    auto cmp1 = es.make_comparator();
+    const auto & cmp1 = es.get_comparator();
     EXPECT_TRUE(cmp1.less(e1, e2));
     EXPECT_FALSE(cmp1.less(e2, e1));
     EXPECT_FALSE(cmp1.less(e1, e1));
@@ -78,7 +78,7 @@ TEST("requireThatFloatEqualIsWorking")
     EnumIndex e1 = es.insert(10.5);
     EnumIndex e2 = es.insert(30.5);
     EnumIndex e3 = es.insert(std::numeric_limits<float>::quiet_NaN());
-    auto cmp1 = es.make_comparator();
+    const auto & cmp1 = es.get_comparator();
     EXPECT_FALSE(cmp1.equal(e1, e2));
     EXPECT_FALSE(cmp1.equal(e2, e1));
     EXPECT_TRUE(cmp1.equal(e1, e1));
@@ -97,7 +97,7 @@ TEST("requireThatStringLessIsWorking")
     EnumIndex e1 = es.insert("Aa");
     EnumIndex e2 = es.insert("aa");
     EnumIndex e3 = es.insert("aB");
-    auto cmp1 = es.make_comparator();
+    const auto & cmp1 = es.get_comparator();
     EXPECT_TRUE(cmp1.less(e1, e2)); // similar folded, fallback to regular
     EXPECT_FALSE(cmp1.less(e2, e1));
     EXPECT_FALSE(cmp1.less(e1, e1));
@@ -114,7 +114,7 @@ TEST("requireThatStringEqualIsWorking")
     EnumIndex e1 = es.insert("Aa");
     EnumIndex e2 = es.insert("aa");
     EnumIndex e3 = es.insert("aB");
-    auto cmp1 = es.make_comparator();
+    const auto & cmp1 = es.get_comparator();
     EXPECT_FALSE(cmp1.equal(e1, e2)); // similar folded, fallback to regular
     EXPECT_FALSE(cmp1.equal(e2, e1));
     EXPECT_TRUE(cmp1.equal(e1, e1));
@@ -157,7 +157,7 @@ TEST("requireThatFoldedLessIsWorking")
     EnumIndex e2 = es.insert("aa");
     EnumIndex e3 = es.insert("aB");
     EnumIndex e4 = es.insert("Folded");
-    auto cmp1 = es.make_folded_comparator();
+    const auto & cmp1 = es.get_folded_comparator();
     EXPECT_FALSE(cmp1.less(e1, e2)); // similar folded
     EXPECT_FALSE(cmp1.less(e2, e1)); // similar folded
     EXPECT_TRUE(cmp1.less(e2, e3)); // folded compare
@@ -177,7 +177,7 @@ TEST("requireThatFoldedEqualIsWorking")
     EnumIndex e2 = es.insert("aa");
     EnumIndex e3 = es.insert("aB");
     EnumIndex e4 = es.insert("Folded");
-    auto cmp1 = es.make_folded_comparator();
+    const auto & cmp1 = es.get_folded_comparator();
     EXPECT_TRUE(cmp1.equal(e1, e1)); // similar folded
     EXPECT_TRUE(cmp1.equal(e2, e1)); // similar folded
     EXPECT_TRUE(cmp1.equal(e2, e1));
@@ -191,7 +191,6 @@ TEST("requireThatFoldedEqualIsWorking")
     EXPECT_FALSE(cmp3.equal(EnumIndex(), e4)); // similar when prefix
     EXPECT_FALSE(cmp3.equal(e4, EnumIndex())); // similar when prefix
     EXPECT_TRUE(cmp3.equal(EnumIndex(), EnumIndex())); // similar when prefix
-
 }
 
 }
diff --git a/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp b/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp
index 65d7fc4dcc1..ada39f9d2de 100644
--- a/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp
+++ b/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp
@@ -606,7 +606,7 @@ EnumStoreDictionaryTest<EnumStoreTypeAndDictionaryType>::update_posting_idx(Enum
 {
     auto& dict = store.get_dictionary();
     EntryRef old_posting_idx_check;
-    dict.update_posting_list(enum_idx, store.make_comparator(), [&old_posting_idx_check, new_posting_idx](EntryRef posting_idx) noexcept -> EntryRef { old_posting_idx_check = posting_idx; return new_posting_idx; });
+    dict.update_posting_list(enum_idx, store.get_comparator(), [&old_posting_idx_check, new_posting_idx](EntryRef posting_idx) noexcept -> EntryRef { old_posting_idx_check = posting_idx; return new_posting_idx; });
     EXPECT_EQ(old_posting_idx, old_posting_idx_check);
 }
 
diff --git a/searchlib/src/tests/attribute/posting_store/posting_store_test.cpp b/searchlib/src/tests/attribute/posting_store/posting_store_test.cpp
index ebaab3b4085..a502dbcaf79 100644
--- a/searchlib/src/tests/attribute/posting_store/posting_store_test.cpp
+++ b/searchlib/src/tests/attribute/posting_store/posting_store_test.cpp
@@ -132,8 +132,8 @@ PostingStoreTest::populate(uint32_t sequence_length)
     for (int i = 0; i < 9000; ++i) {
         refs.emplace_back(add_sequence(i + 6, i + 6 + sequence_length));
     }
-    dictionary.update_posting_list(_value_store.insert(1), _value_store.make_comparator(), [this, sequence_length](EntryRef) { return add_sequence(4, 4 + sequence_length); });
-    dictionary.update_posting_list(_value_store.insert(2), _value_store.make_comparator(), [this, sequence_length](EntryRef) { return add_sequence(5, 5 + sequence_length); });
+    dictionary.update_posting_list(_value_store.insert(1), _value_store.get_comparator(), [this, sequence_length](EntryRef) { return add_sequence(4, 4 + sequence_length); });
+    dictionary.update_posting_list(_value_store.insert(2), _value_store.get_comparator(), [this, sequence_length](EntryRef) { return add_sequence(5, 5 + sequence_length); });
     for (int i = 9000; i < 11000; ++i) {
         refs.emplace_back(add_sequence(i + 6, i + 6 + sequence_length));
     }
diff --git a/searchlib/src/vespa/searchlib/attribute/enumstore.cpp b/searchlib/src/vespa/searchlib/attribute/enumstore.cpp
index 3a008557676..9da3906ac85 100644
--- a/searchlib/src/vespa/searchlib/attribute/enumstore.cpp
+++ b/searchlib/src/vespa/searchlib/attribute/enumstore.cpp
@@ -21,9 +21,7 @@ EnumStoreT<const char*>::write_value(BufferWriter& writer, Index idx) const
 
 template <>
 ssize_t
-EnumStoreT<const char*>::load_unique_value(const void* src,
-                                           size_t available,
-                                           Index& idx)
+EnumStoreT<const char*>::load_unique_value(const void* src, size_t available, Index& idx)
 {
     const char* value = static_cast<const char*>(src);
     size_t slen = strlen(value);
@@ -43,8 +41,8 @@ EnumStoreT<const char*>::load_unique_value(const void* src,
 
 std::unique_ptr<vespalib::datastore::IUniqueStoreDictionary>
 make_enum_store_dictionary(IEnumStore &store, bool has_postings, const search::DictionaryConfig & dict_cfg,
-                           std::unique_ptr<vespalib::datastore::EntryComparator> compare,
-                           std::unique_ptr<vespalib::datastore::EntryComparator> folded_compare)
+                           std::unique_ptr<EntryComparator> compare,
+                           std::unique_ptr<EntryComparator> folded_compare)
 {
     using NoBTreeDictionary = vespalib::datastore::NoBTreeDictionary;
     using ShardedHashMap = vespalib::datastore::ShardedHashMap;
diff --git a/searchlib/src/vespa/searchlib/attribute/enumstore.h b/searchlib/src/vespa/searchlib/attribute/enumstore.h
index 3d4f6d3e888..618faf84d9b 100644
--- a/searchlib/src/vespa/searchlib/attribute/enumstore.h
+++ b/searchlib/src/vespa/searchlib/attribute/enumstore.h
@@ -47,11 +47,14 @@ public:
     using EntryType = EntryT;
     using EnumStoreType = EnumStoreT<EntryT>;
     using EntryRef = vespalib::datastore::EntryRef;
+    using EntryComparator = vespalib::datastore::EntryComparator;
     using generation_t = vespalib::GenerationHandler::generation_t;
 
 private:
     UniqueStoreType        _store;
     IEnumStoreDictionary*  _dict;
+    ComparatorType         _comparator;
+    ComparatorType         _foldedComparator;
     vespalib::MemoryUsage  _cached_values_memory_usage;
     vespalib::AddressSpace _cached_values_address_space_usage;
     vespalib::MemoryUsage  _cached_dictionary_btree_usage;
@@ -73,6 +76,8 @@ private:
     ssize_t load_unique_values_internal(const void* src, size_t available, IndexVector& idx);
     ssize_t load_unique_value(const void* src, size_t available, Index& idx);
 
+    std::unique_ptr<EntryComparator> allocate_optionally_folded_comparator(bool folded) const;
+    ComparatorType make_optionally_folded_comparator(bool folded) const;
 public:
     EnumStoreT(bool has_postings, const search::DictionaryConfig & dict_cfg);
     ~EnumStoreT() override;
@@ -174,16 +179,16 @@ public:
         return BatchUpdater(*this);
     }
 
-    ComparatorType make_comparator() const {
-        return ComparatorType(_store.get_data_store());
+    const EntryComparator & get_comparator() const {
+        return _comparator;
     }
 
     ComparatorType make_comparator(const EntryType& fallback_value) const {
         return ComparatorType(_store.get_data_store(), fallback_value);
     }
 
-    ComparatorType make_folded_comparator() const {
-        return ComparatorType(_store.get_data_store(), true);
+    const EntryComparator & get_folded_comparator() const {
+        return _foldedComparator;
     }
 
     void write_value(BufferWriter& writer, Index idx) const override;
@@ -204,7 +209,7 @@ public:
         _store.get_allocator().get_data_store().inc_compaction_count();
     }
     std::unique_ptr<Enumerator> make_enumerator() const override;
-    std::unique_ptr<vespalib::datastore::EntryComparator> allocate_comparator() const override;
+    std::unique_ptr<EntryComparator> allocate_comparator() const override;
 
     // Methods below are only relevant for strings, and are templated to only be instantiated on demand.
     template <typename Type>
@@ -225,21 +230,13 @@ public:
     }
 };
 
-std::unique_ptr<vespalib::datastore::IUniqueStoreDictionary>
-make_enum_store_dictionary(IEnumStore &store, bool has_postings, const search::DictionaryConfig & dict_cfg,
-                           std::unique_ptr<vespalib::datastore::EntryComparator> compare,
-                           std::unique_ptr<vespalib::datastore::EntryComparator> folded_compare);
-
-
 template <>
 void
 EnumStoreT<const char*>::write_value(BufferWriter& writer, Index idx) const;
 
 template <>
 ssize_t
-EnumStoreT<const char*>::load_unique_value(const void* src,
-                                           size_t available,
-                                           Index& idx);
+EnumStoreT<const char*>::load_unique_value(const void* src, size_t available, Index& idx);
 
 }
 
diff --git a/searchlib/src/vespa/searchlib/attribute/enumstore.hpp b/searchlib/src/vespa/searchlib/attribute/enumstore.hpp
index de5973dd4a1..ef9362174de 100644
--- a/searchlib/src/vespa/searchlib/attribute/enumstore.hpp
+++ b/searchlib/src/vespa/searchlib/attribute/enumstore.hpp
@@ -22,6 +22,13 @@
 
 namespace search {
 
+using vespalib::datastore::EntryComparator;
+
+std::unique_ptr<vespalib::datastore::IUniqueStoreDictionary>
+make_enum_store_dictionary(IEnumStore &store, bool has_postings, const search::DictionaryConfig & dict_cfg,
+                           std::unique_ptr<EntryComparator> compare,
+                           std::unique_ptr<EntryComparator> folded_compare);
+
 template <typename EntryT>
 void EnumStoreT<EntryT>::free_value_if_unused(Index idx, IndexSet& unused)
 {
@@ -34,9 +41,7 @@ void EnumStoreT<EntryT>::free_value_if_unused(Index idx, IndexSet& unused)
 
 template <typename EntryT>
 ssize_t
-EnumStoreT<EntryT>::load_unique_values_internal(const void* src,
-                                                size_t available,
-                                                IndexVector& idx)
+EnumStoreT<EntryT>::load_unique_values_internal(const void* src, size_t available, IndexVector& idx)
 {
     size_t left = available;
     const char* p = static_cast<const char*>(src);
@@ -69,14 +74,14 @@ template <typename EntryT>
 EnumStoreT<EntryT>::EnumStoreT(bool has_postings, const search::DictionaryConfig & dict_cfg)
     : _store(),
       _dict(),
+      _comparator(_store.get_data_store()),
+      _foldedComparator(make_optionally_folded_comparator(true)),
       _cached_values_memory_usage(),
       _cached_values_address_space_usage(0, 0, (1ull << 32))
 {
     _store.set_dictionary(make_enum_store_dictionary(*this, has_postings, dict_cfg,
-                                                     std::make_unique<ComparatorType>(_store.get_data_store()),
-                                                     (has_string_type() ?
-                                                      std::make_unique<ComparatorType>(_store.get_data_store(), true) :
-                                                      std::unique_ptr<vespalib::datastore::EntryComparator>())));
+                                                     allocate_comparator(),
+                                                     allocate_optionally_folded_comparator(true)));
     _dict = static_cast<IEnumStoreDictionary*>(&_store.get_dictionary());
 }
 
@@ -150,7 +155,7 @@ template <class EntryT>
 bool
 EnumStoreT<EntryT>::is_folded_change(Index idx1, Index idx2) const
 {
-    auto cmp = make_folded_comparator();
+    const auto & cmp = get_folded_comparator();
     assert(!cmp.less(idx2, idx1));
     return cmp.less(idx1, idx2);
 }
@@ -180,14 +185,14 @@ template <typename EntryT>
 void
 EnumStoreT<EntryT>::free_unused_values()
 {
-    _dict->free_unused_values(make_comparator());
+    _dict->free_unused_values(get_comparator());
 }
 
 template <typename EntryT>
 void
 EnumStoreT<EntryT>::free_unused_values(const IndexSet& to_remove)
 {
-    _dict->free_unused_values(to_remove, make_comparator());
+    _dict->free_unused_values(to_remove, get_comparator());
 }
 
 template <typename EntryT>
@@ -265,10 +270,28 @@ EnumStoreT<EntryT>::make_enumerator() const
 }
 
 template <typename EntryT>
-std::unique_ptr<vespalib::datastore::EntryComparator>
+std::unique_ptr<EntryComparator>
 EnumStoreT<EntryT>::allocate_comparator() const
 {
     return std::make_unique<ComparatorType>(_store.get_data_store());
 }
 
+template <typename EntryT>
+std::unique_ptr<EntryComparator>
+EnumStoreT<EntryT>::allocate_optionally_folded_comparator(bool folded) const
+{
+    return (has_string_type() && folded)
+            ? std::make_unique<ComparatorType>(_store.get_data_store(), true)
+            : std::unique_ptr<EntryComparator>();
+}
+
+template <typename EntryT>
+EnumStoreT<EntryT>::ComparatorType
+EnumStoreT<EntryT>::make_optionally_folded_comparator(bool folded) const
+{
+    return (has_string_type() && folded)
+           ? ComparatorType(_store.get_data_store(), true)
+           : ComparatorType(_store.get_data_store());
+}
+
 }
diff --git a/searchlib/src/vespa/searchlib/attribute/multinumericpostattribute.hpp b/searchlib/src/vespa/searchlib/attribute/multinumericpostattribute.hpp
index 5b695c5751b..e3fbed1ecc2 100644
--- a/searchlib/src/vespa/searchlib/attribute/multinumericpostattribute.hpp
+++ b/searchlib/src/vespa/searchlib/attribute/multinumericpostattribute.hpp
@@ -30,7 +30,7 @@ MultiValueNumericPostingAttribute<B, M>::applyValueChanges(const DocIndices& doc
 
     EnumIndexMapper mapper;
     PostingMap changePost(PostingChangeComputer::compute(this->getMultiValueMapping(), docIndices,
-                                                         this->getEnumStore().make_comparator(), mapper));
+                                                         this->getEnumStore().get_comparator(), mapper));
     this->updatePostings(changePost);
     MultiValueNumericEnumAttribute<B, M>::applyValueChanges(docIndices, updater);
 }
diff --git a/searchlib/src/vespa/searchlib/attribute/multistringpostattribute.hpp b/searchlib/src/vespa/searchlib/attribute/multistringpostattribute.hpp
index d5064746cc2..381d5b6339b 100644
--- a/searchlib/src/vespa/searchlib/attribute/multistringpostattribute.hpp
+++ b/searchlib/src/vespa/searchlib/attribute/multistringpostattribute.hpp
@@ -47,7 +47,7 @@ applyValueChanges(const DocIndices& docIndices, EnumStoreBatchUpdater &updater)
 
     StringEnumIndexMapper mapper(dictionary);
     PostingMap changePost(PostingChangeComputer::compute(this->getMultiValueMapping(), docIndices,
-                                                         enumStore.make_folded_comparator(), mapper));
+                                                         enumStore.get_folded_comparator(), mapper));
     this->updatePostings(changePost);
     MultiValueStringAttributeT<B, T>::applyValueChanges(docIndices, updater);
 }
diff --git a/searchlib/src/vespa/searchlib/attribute/postinglistattribute.cpp b/searchlib/src/vespa/searchlib/attribute/postinglistattribute.cpp
index 9a431be5a02..e988091db45 100644
--- a/searchlib/src/vespa/searchlib/attribute/postinglistattribute.cpp
+++ b/searchlib/src/vespa/searchlib/attribute/postinglistattribute.cpp
@@ -281,7 +281,7 @@ void
 PostingListAttributeSubBase<P, LoadedVector, LoadedValueType, EnumStoreType>::
 updatePostings(PostingMap &changePost)
 {
-    updatePostings(changePost, _es.make_folded_comparator());
+    updatePostings(changePost, _es.get_folded_comparator());
 }
 
 
@@ -291,7 +291,7 @@ void
 PostingListAttributeSubBase<P, LoadedVector, LoadedValueType, EnumStoreType>::
 clearPostings(attribute::IAttributeVector::EnumHandle eidx, uint32_t fromLid, uint32_t toLid)
 {
-    clearPostings(eidx, fromLid, toLid, _es.make_folded_comparator());
+    clearPostings(eidx, fromLid, toLid, _es.get_folded_comparator());
 }
 
 
diff --git a/searchlib/src/vespa/searchlib/attribute/singlenumericpostattribute.h b/searchlib/src/vespa/searchlib/attribute/singlenumericpostattribute.h
index 634ac7ba024..4e7f8040f7f 100644
--- a/searchlib/src/vespa/searchlib/attribute/singlenumericpostattribute.h
+++ b/searchlib/src/vespa/searchlib/attribute/singlenumericpostattribute.h
@@ -63,7 +63,7 @@ private:
     void mergeMemoryStats(vespalib::MemoryUsage & total) override;
     void applyUpdateValueChange(const Change & c, EnumStore & enumStore,
                                 std::map<DocId, EnumIndex> & currEnumIndices);
-    void makePostingChange(const vespalib::datastore::EntryComparator *cmp,
+    void makePostingChange(const vespalib::datastore::EntryComparator &cmp,
                            const std::map<DocId, EnumIndex> &currEnumIndices,
                            PostingMap &changePost);
 
diff --git a/searchlib/src/vespa/searchlib/attribute/singlenumericpostattribute.hpp b/searchlib/src/vespa/searchlib/attribute/singlenumericpostattribute.hpp
index 29c4927efd4..f5ab855565c 100644
--- a/searchlib/src/vespa/searchlib/attribute/singlenumericpostattribute.hpp
+++ b/searchlib/src/vespa/searchlib/attribute/singlenumericpostattribute.hpp
@@ -53,7 +53,7 @@ SingleValueNumericPostingAttribute<B>::applyUpdateValueChange(const Change & c,
 template <typename B>
 void
 SingleValueNumericPostingAttribute<B>::
-makePostingChange(const vespalib::datastore::EntryComparator *cmpa,
+makePostingChange(const vespalib::datastore::EntryComparator &cmpa,
                   const std::map<DocId, EnumIndex> &currEnumIndices,
                   PostingMap &changePost)
 {
@@ -63,11 +63,11 @@ makePostingChange(const vespalib::datastore::EntryComparator *cmpa,
         EnumIndex newIdx = elem.second;
 
         // add new posting
-        changePost[EnumPostingPair(newIdx, cmpa)].add(docId, 1);
+        changePost[EnumPostingPair(newIdx, &cmpa)].add(docId, 1);
 
         // remove old posting
         if ( oldIdx.valid()) {
-            changePost[EnumPostingPair(oldIdx, cmpa)].remove(docId);
+            changePost[EnumPostingPair(oldIdx, &cmpa)].remove(docId);
         }
     }
 }
@@ -79,7 +79,6 @@ SingleValueNumericPostingAttribute<B>::applyValueChanges(EnumStoreBatchUpdater&
 {
     EnumStore & enumStore = this->getEnumStore();
     IEnumStoreDictionary& dictionary = enumStore.get_dictionary();
-    auto cmp = enumStore.make_comparator();
     PostingMap changePost;
 
     // used to make sure several arithmetic operations on the same document in a single commit works
@@ -110,7 +109,7 @@ SingleValueNumericPostingAttribute<B>::applyValueChanges(EnumStoreBatchUpdater&
         }
     }
 
-    makePostingChange(&cmp, currEnumIndices, changePost);
+    makePostingChange(enumStore.get_comparator(), currEnumIndices, changePost);
 
     this->updatePostings(changePost);
     SingleValueNumericEnumAttribute<B>::applyValueChanges(updater);
diff --git a/searchlib/src/vespa/searchlib/attribute/singlestringpostattribute.hpp b/searchlib/src/vespa/searchlib/attribute/singlestringpostattribute.hpp
index df6982660f8..39ad8d71021 100644
--- a/searchlib/src/vespa/searchlib/attribute/singlestringpostattribute.hpp
+++ b/searchlib/src/vespa/searchlib/attribute/singlestringpostattribute.hpp
@@ -101,7 +101,7 @@ SingleValueStringPostingAttributeT<B>::applyValueChanges(EnumStoreBatchUpdater&
         }
     }
 
-    makePostingChange(enumStore.make_folded_comparator(), dictionary, currEnumIndices, changePost);
+    makePostingChange(enumStore.get_folded_comparator(), dictionary, currEnumIndices, changePost);
 
     this->updatePostings(changePost);