Merge pull request #28560 from vespa-engine/toregge/use-make-for-lookup-method-on-existing-comparator

Use make_for_lookup() member function on existing comparator

3 files changed, 39 insertions, 23 deletions

diff --git a/searchlib/src/vespa/searchlib/attribute/enumcomparator.h b/searchlib/src/vespa/searchlib/attribute/enumcomparator.h
index 52d9ebad902..255719ad1c1 100644
--- a/searchlib/src/vespa/searchlib/attribute/enumcomparator.h
+++ b/searchlib/src/vespa/searchlib/attribute/enumcomparator.h
@@ -18,14 +18,24 @@ public:
     using ParentType = vespalib::datastore::UniqueStoreComparator<EntryT, IEnumStore::InternalIndex>;
     using DataStoreType = typename ParentType::DataStoreType;
 
-    EnumStoreComparator(const DataStoreType& data_store, const EntryT& lookup_value)
-        : ParentType(data_store, lookup_value)
-    {}
     EnumStoreComparator(const DataStoreType& data_store)
         : ParentType(data_store)
     {}
 
+private:
+    EnumStoreComparator(const DataStoreType& data_store, const EntryT& lookup_value)
+        : ParentType(data_store, lookup_value)
+    {}
+
+public:
     static bool equal_helper(const EntryT& lhs, const EntryT& rhs);
+
+    EnumStoreComparator<EntryT> make_folded() const {
+        return *this;
+    }
+    EnumStoreComparator<EntryT> make_for_lookup(const EntryT& lookup_value) const {
+        return EnumStoreComparator<EntryT>(this->_store, lookup_value);
+    }
 };
 
 /**
@@ -45,22 +55,31 @@ public:
     EnumStoreStringComparator(const DataStoreType& data_store)
         : EnumStoreStringComparator(data_store, false)
     {}
+
+private:
     EnumStoreStringComparator(const DataStoreType& data_store, bool fold);
 
     /**
      * Creates a comparator using the given low-level data store and that uses the
      * given value during compare if the enum index is invalid.
      */
-    EnumStoreStringComparator(const DataStoreType& data_store, const char* lookup_value)
-        : EnumStoreStringComparator(data_store, false, lookup_value)
-    {}
     EnumStoreStringComparator(const DataStoreType& data_store, bool fold, const char* lookup_value);
     EnumStoreStringComparator(const DataStoreType& data_store, bool fold, const char* lookup_value, bool prefix);
 
+public:
     bool less(const vespalib::datastore::EntryRef lhs, const vespalib::datastore::EntryRef rhs) const override;
     bool equal(const vespalib::datastore::EntryRef lhs, const vespalib::datastore::EntryRef rhs) const override;
+    EnumStoreStringComparator make_folded() const {
+        return EnumStoreStringComparator(_store, true);
+    }
+    EnumStoreStringComparator make_for_lookup(const char* lookup_value) const {
+        return EnumStoreStringComparator(_store, _fold, lookup_value);
+    }
+    EnumStoreStringComparator make_for_prefix_lookup(const char* lookup_value) const {
+        return EnumStoreStringComparator(_store, _fold, lookup_value, true);
+    }
 private:
-    inline bool use_prefix() const { return _prefix; }
+    inline bool use_prefix() const noexcept { return _prefix; }
     const bool _fold;
     const bool _prefix;
     uint32_t   _prefix_len;
diff --git a/searchlib/src/vespa/searchlib/attribute/enumstore.h b/searchlib/src/vespa/searchlib/attribute/enumstore.h
index 8cafd04586e..6560decf42f 100644
--- a/searchlib/src/vespa/searchlib/attribute/enumstore.h
+++ b/searchlib/src/vespa/searchlib/attribute/enumstore.h
@@ -37,15 +37,16 @@ namespace search {
 template <class EntryT>
 class EnumStoreT : public IEnumStore {
 public:
-    using ComparatorType = std::conditional_t<std::is_same_v<EntryT, const char *>,
+    using EntryType = EntryT;
+    static constexpr bool has_string_type = std::is_same_v<EntryType, const char *>;
+    using ComparatorType = std::conditional_t<has_string_type,
                                               EnumStoreStringComparator,
                                               EnumStoreComparator<EntryT>>;
-    using AllocatorType = std::conditional_t<std::is_same_v<EntryT, const char *>,
+    using AllocatorType = std::conditional_t<has_string_type,
                                              vespalib::datastore::UniqueStoreStringAllocator<InternalIndex>,
                                              vespalib::datastore::UniqueStoreAllocator<EntryT, InternalIndex>>;
     using UniqueStoreType = vespalib::datastore::UniqueStore<EntryT, InternalIndex, ComparatorType, AllocatorType>;
 
-    using EntryType = EntryT;
     using EnumStoreType = EnumStoreT<EntryT>;
     using EntryRef = vespalib::datastore::EntryRef;
     using EntryComparator = vespalib::datastore::EntryComparator;
@@ -69,10 +70,6 @@ private:
         return _store.get_allocator().get_wrapped(idx);
     }
 
-    static bool has_string_type() {
-        return std::is_same_v<EntryType, const char *>;
-    }
-
     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);
 
@@ -191,7 +188,7 @@ public:
     }
 
     ComparatorType make_comparator(const EntryType& lookup_value) const {
-        return ComparatorType(_store.get_data_store(), lookup_value);
+        return _store.get_comparator().make_for_lookup(lookup_value);
     }
 
     const EntryComparator & get_folded_comparator() const {
@@ -225,12 +222,12 @@ public:
     template <typename Type>
     ComparatorType
     make_folded_comparator(const Type& lookup_value) const {
-        return ComparatorType(_store.get_data_store(), is_folded(), lookup_value);
+        return _foldedComparator.make_for_lookup(lookup_value);
     }
     template<typename Type>
     ComparatorType
     make_folded_comparator_prefix(const Type& lookup_value) const {
-        return ComparatorType(_store.get_data_store(), is_folded(), lookup_value, true);
+        return _foldedComparator.make_for_prefix_lookup(lookup_value);
     }
     template<typename Type>
     std::vector<IEnumStore::EnumHandle>
diff --git a/searchlib/src/vespa/searchlib/attribute/enumstore.hpp b/searchlib/src/vespa/searchlib/attribute/enumstore.hpp
index 85cf676182e..140ac207d32 100644
--- a/searchlib/src/vespa/searchlib/attribute/enumstore.hpp
+++ b/searchlib/src/vespa/searchlib/attribute/enumstore.hpp
@@ -305,15 +305,15 @@ template <typename EntryT>
 std::unique_ptr<EntryComparator>
 EnumStoreT<EntryT>::allocate_comparator() const
 {
-    return std::make_unique<ComparatorType>(_store.get_data_store());
+    return std::make_unique<ComparatorType>(_store.get_comparator());
 }
 
 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)
+    return (has_string_type && folded)
+        ? std::make_unique<ComparatorType>(_store.get_comparator().make_folded())
             : std::unique_ptr<EntryComparator>();
 }
 
@@ -321,9 +321,9 @@ template <typename EntryT>
 typename 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());
+    return (has_string_type && folded)
+        ? _store.get_comparator().make_folded()
+        : _store.get_comparator();
 }
 
 }