Rename addEnum() -> insert() and change semantics to increase ref count.

4 files changed, 46 insertions, 60 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 34f8833f9be..97b08c5683f 100644
--- a/searchlib/src/tests/attribute/enum_comparator/enum_comparator_test.cpp
+++ b/searchlib/src/tests/attribute/enum_comparator/enum_comparator_test.cpp
@@ -43,9 +43,8 @@ void
 Test::requireThatNumericComparatorIsWorking()
 {
     NumericEnumStore es(false);
-    EnumIndex e1, e2;
-    es.addEnum(10, e1);
-    es.addEnum(30, e2);
+    EnumIndex e1 = es.insert(10);
+    EnumIndex e2 = es.insert(30);
     auto cmp1 = es.make_comparator();
     EXPECT_TRUE(cmp1(e1, e2));
     EXPECT_TRUE(!cmp1(e2, e1));
@@ -59,10 +58,9 @@ void
 Test::requireThatFloatComparatorIsWorking()
 {
     FloatEnumStore es(false);
-    EnumIndex e1, e2, e3;
-    es.addEnum(10.5, e1);
-    es.addEnum(30.5, e2);
-    es.addEnum(std::numeric_limits<float>::quiet_NaN(), e3);
+    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();
     EXPECT_TRUE(cmp1(e1, e2));
     EXPECT_TRUE(!cmp1(e2, e1));
@@ -79,10 +77,9 @@ void
 Test::requireThatStringComparatorIsWorking()
 {
     StringEnumStore es(false);
-    EnumIndex e1, e2, e3;
-    es.addEnum("Aa", e1);
-    es.addEnum("aa", e2);
-    es.addEnum("aB", e3);
+    EnumIndex e1 = es.insert("Aa");
+    EnumIndex e2 = es.insert("aa");
+    EnumIndex e3 = es.insert("aB");
     auto cmp1 = es.make_comparator();
     EXPECT_TRUE(cmp1(e1, e2)); // similar folded, fallback to regular
     EXPECT_TRUE(!cmp1(e2, e1));
@@ -101,12 +98,11 @@ Test::requireThatComparatorWithTreeIsWorking()
     vespalib::GenerationHandler g;
     TreeType t;
     NodeAllocator m;
-    EnumIndex ei;
     for (int32_t v = 100; v > 0; --v) {
         auto cmp = es.make_comparator(v);
         EXPECT_TRUE(!t.find(EnumIndex(), m, cmp).valid());
-        es.addEnum(v, ei);
-        t.insert(ei, BTreeNoLeafData(), m, cmp);
+        EnumIndex idx = es.insert(v);
+        t.insert(idx, BTreeNoLeafData(), m, cmp);
     }
     EXPECT_EQUAL(100u, t.size(m));
     int32_t exp = 1;
@@ -125,11 +121,10 @@ void
 Test::requireThatFoldedComparatorIsWorking()
 {
     StringEnumStore es(false);
-    EnumIndex e1, e2, e3, e4;
-    es.addEnum("Aa", e1);
-    es.addEnum("aa", e2);
-    es.addEnum("aB", e3);
-    es.addEnum("Folded", e4);
+    EnumIndex e1 = es.insert("Aa");
+    EnumIndex e2 = es.insert("aa");
+    EnumIndex e3 = es.insert("aB");
+    EnumIndex e4 = es.insert("Folded");
     auto cmp1 = es.make_folded_comparator();
     EXPECT_TRUE(!cmp1(e1, e2)); // similar folded
     EXPECT_TRUE(!cmp1(e2, e1)); // similar folded
diff --git a/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp b/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp
index ca40b5cc259..d4431b8a990 100644
--- a/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp
+++ b/searchlib/src/tests/attribute/enumstore/enumstore_test.cpp
@@ -32,9 +32,9 @@ private:
     void testFloatEnumStore();
 
     void testFindFolded();
-    void testAddEnum();
+    void testInsert();
     template <typename EnumStoreType>
-    void testAddEnum(bool hasPostings);
+    void testInsert(bool hasPostings);
 
     template <typename EnumStoreType, typename Dictionary>
     void
@@ -91,7 +91,7 @@ EnumStoreTest::testFloatEnumStore(EnumStoreType & es)
     T b[5] = {-25.5f, -15.5f, -5.5f, 4.5f, 14.5f};
 
     for (uint32_t i = 0; i < 5; ++i) {
-        es.addEnum(a[i], idx);
+        es.insert(a[i]);
     }
 
     for (uint32_t i = 0; i < 5; ++i) {
@@ -99,7 +99,7 @@ EnumStoreTest::testFloatEnumStore(EnumStoreType & es)
         EXPECT_TRUE(!es.findIndex(b[i], idx));
     }
 
-    es.addEnum(std::numeric_limits<T>::quiet_NaN(), idx);
+    es.insert(std::numeric_limits<T>::quiet_NaN());
     EXPECT_TRUE(es.findIndex(std::numeric_limits<T>::quiet_NaN(), idx));
     EXPECT_TRUE(es.findIndex(std::numeric_limits<T>::quiet_NaN(), idx));
 
@@ -129,10 +129,8 @@ EnumStoreTest::testFindFolded()
     std::vector<EnumIndex> indices;
     std::vector<std::string> unique({"", "one", "two", "TWO", "Two", "three"});
     for (std::string &str : unique) {
-        EnumIndex idx;
-        ses.addEnum(str.c_str(), idx);
+        EnumIndex idx = ses.insert(str.c_str());
         indices.push_back(idx);
-        ses.incRefCount(idx);
         EXPECT_EQUAL(1u, ses.getRefCount(idx));
     }
     ses.freezeTree();
@@ -154,21 +152,19 @@ EnumStoreTest::testFindFolded()
 }
 
 void
-EnumStoreTest::testAddEnum()
+EnumStoreTest::testInsert()
 {
-    testAddEnum<StringEnumStore>(false);
+    testInsert<StringEnumStore>(false);
 
-    testAddEnum<StringEnumStore>(true);
+    testInsert<StringEnumStore>(true);
 }
 
 template <typename EnumStoreType>
 void
-EnumStoreTest::testAddEnum(bool hasPostings)
+EnumStoreTest::testInsert(bool hasPostings)
 {
-    // TODO: Rewrite test to use BatchUpdater
     EnumStoreType ses(hasPostings);
 
-    EnumIndex idx;
     std::vector<EnumIndex> indices;
     std::vector<std::string> unique;
     unique.push_back("");
@@ -177,8 +173,7 @@ EnumStoreTest::testAddEnum(bool hasPostings)
     unique.push_back("unique");
 
     for (uint32_t i = 0; i < unique.size(); ++i) {
-        ses.addEnum(unique[i].c_str(), idx);
-        ses.incRefCount(idx);
+        EnumIndex idx = ses.insert(unique[i].c_str());
         EXPECT_EQUAL(1u, ses.getRefCount(idx));
         indices.push_back(idx);
         EXPECT_TRUE(ses.findIndex(unique[i].c_str(), idx));
@@ -190,6 +185,7 @@ EnumStoreTest::testAddEnum(bool hasPostings)
         EXPECT_TRUE(ses.findEnum(unique[i].c_str(), e));
         EXPECT_EQUAL(1u, ses.findFoldedEnums(unique[i].c_str()).size());
         EXPECT_EQUAL(e, ses.findFoldedEnums(unique[i].c_str())[0]);
+        EnumIndex idx;
         EXPECT_TRUE(ses.findIndex(unique[i].c_str(), idx));
         EXPECT_TRUE(idx == indices[i]);
         EXPECT_EQUAL(1u, ses.getRefCount(indices[i]));
@@ -228,7 +224,6 @@ EnumStoreTest::testHoldListAndGeneration()
 {
     // TODO: Rewrite test to use BatchUpdater
     StringEnumStore ses(false);
-    StringEnumStore::Index idx;
     StringVector uniques;
     generation_t sesGen = 0u;
     uniques.reserve(100);
@@ -249,8 +244,7 @@ EnumStoreTest::testHoldListAndGeneration()
 
     // insert first batch of unique strings
     for (uint32_t i = 0; i < 100; ++i) {
-        ses.addEnum(uniques[i].c_str(), idx);
-        ses.incRefCount(idx);
+        EnumIndex idx = ses.insert(uniques[i].c_str());
         EXPECT_TRUE(ses.getRefCount(idx));
 
         // associate readers
@@ -276,6 +270,7 @@ EnumStoreTest::testHoldListAndGeneration()
 
     // remove all uniques
     for (uint32_t i = 0; i < 100; ++i) {
+        EnumIndex idx;
         EXPECT_TRUE(ses.findIndex(uniques[i].c_str(), idx));
         ses.decRefCount(idx);
         EXPECT_EQUAL(0u, ses.getRefCount(idx));
@@ -291,15 +286,6 @@ EnumStoreTest::testHoldListAndGeneration()
 
 namespace {
 
-NumericEnumStore::Index
-addEnum(NumericEnumStore &store, uint32_t value)
-{
-    NumericEnumStore::Index result;
-    store.addEnum(value, result);
-    store.incRefCount(result);
-    return result;
-}
-
 void
 decRefCount(NumericEnumStore& store, NumericEnumStore::Index idx)
 {
@@ -321,9 +307,9 @@ EnumStoreTest::requireThatAddressSpaceUsageIsReported()
 
     using vespalib::AddressSpace;
     EXPECT_EQUAL(AddressSpace(1, 1, ADDRESS_LIMIT), store.getAddressSpaceUsage());
-    NumericEnumStore::Index idx1 = addEnum(store, 10);
+    EnumIndex idx1 = store.insert(10);
     EXPECT_EQUAL(AddressSpace(2, 1, ADDRESS_LIMIT), store.getAddressSpaceUsage());
-    NumericEnumStore::Index idx2 = addEnum(store, 20);
+    EnumIndex idx2 = store.insert(20);
     // Address limit increases because buffer is re-sized.
     EXPECT_EQUAL(AddressSpace(3, 1, ADDRESS_LIMIT + 2), store.getAddressSpaceUsage());
     decRefCount(store, idx1);
@@ -357,7 +343,7 @@ EnumStoreTest::Main()
 
     testFloatEnumStore();
     testFindFolded();
-    testAddEnum();
+    testInsert();
     testHoldListAndGeneration();
     TEST_DO(requireThatAddressSpaceUsageIsReported());
 
diff --git a/searchlib/src/vespa/searchlib/attribute/enumstore.h b/searchlib/src/vespa/searchlib/attribute/enumstore.h
index c9784678340..8155f1ba537 100644
--- a/searchlib/src/vespa/searchlib/attribute/enumstore.h
+++ b/searchlib/src/vespa/searchlib/attribute/enumstore.h
@@ -166,11 +166,7 @@ public:
             : _store(store),
               _possibly_unused()
         {}
-        void insert(EntryType value) {
-            Index idx;
-            _store.addEnum(value, idx);
-            _possibly_unused.insert(idx);
-        }
+        void insert(EntryType value);
         void inc_ref_count(Index idx) {
             _store.get_entry_base(idx).inc_ref_count();
         }
@@ -211,7 +207,7 @@ public:
     bool foldedChange(const Index &idx1, const Index &idx2) const override;
     bool findEnum(EntryType value, IEnumStore::EnumHandle &e) const;
     std::vector<IEnumStore::EnumHandle> findFoldedEnums(EntryType value) const;
-    void addEnum(EntryType value, Index &newIdx);
+    Index insert(EntryType value);
     bool findIndex(EntryType value, Index &idx) const;
     void freeUnusedEnums() override;
     void freeUnusedEnums(const IndexSet& toRemove);
diff --git a/searchlib/src/vespa/searchlib/attribute/enumstore.hpp b/searchlib/src/vespa/searchlib/attribute/enumstore.hpp
index 8c230457c0e..fc8ea284222 100644
--- a/searchlib/src/vespa/searchlib/attribute/enumstore.hpp
+++ b/searchlib/src/vespa/searchlib/attribute/enumstore.hpp
@@ -135,6 +135,17 @@ EnumStoreT<EntryType>::getValue(Index idx, EntryType& value) const
 template <typename EntryType>
 EnumStoreT<EntryType>::NonEnumeratedLoader::~NonEnumeratedLoader() = default;
 
+template <typename EntryType>
+void
+EnumStoreT<EntryType>::BatchUpdater::insert(EntryType value)
+{
+    auto cmp = _store.make_comparator(value);
+    auto result = _store._dict->add(cmp, [this, &value]() -> EntryRef { return _store._store.get_allocator().allocate(value); });
+    if (result.inserted()) {
+        _possibly_unused.insert(result.ref());
+    }
+}
+
 template <class EntryType>
 void
 EnumStoreT<EntryType>::writeValues(BufferWriter& writer, vespalib::ConstArrayRef<Index> idxs) const
@@ -199,12 +210,10 @@ EnumStoreT<EntryType>::freeUnusedEnums(const IndexSet& toRemove)
 }
 
 template <typename EntryType>
-void
-EnumStoreT<EntryType>::addEnum(EntryType value, Index& newIdx)
+IEnumStore::Index
+EnumStoreT<EntryType>::insert(EntryType value)
 {
-    auto cmp = make_comparator(value);
-    auto add_result = _dict->add(cmp, [this, &value]() -> EntryRef { return _store.get_allocator().allocate(value); });
-    newIdx = add_result.ref();
+    return _store.add(value).ref();
 }
 
 template <typename EntryType>