From 1dffab88191e9fce8c8711350d8e8de36168b927 Mon Sep 17 00:00:00 2001 From: Geir Storli Date: Tue, 10 Sep 2019 11:01:51 +0000 Subject: Rename addEnum() -> insert() and change semantics to increase ref count. --- .../enum_comparator/enum_comparator_test.cpp | 33 +++++++--------- .../tests/attribute/enumstore/enumstore_test.cpp | 46 ++++++++-------------- .../src/vespa/searchlib/attribute/enumstore.h | 8 +--- .../src/vespa/searchlib/attribute/enumstore.hpp | 19 ++++++--- 4 files changed, 46 insertions(+), 60 deletions(-) (limited to 'searchlib') 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::quiet_NaN(), e3); + EnumIndex e1 = es.insert(10.5); + EnumIndex e2 = es.insert(30.5); + EnumIndex e3 = es.insert(std::numeric_limits::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 - void testAddEnum(bool hasPostings); + void testInsert(bool hasPostings); template 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::quiet_NaN(), idx); + es.insert(std::numeric_limits::quiet_NaN()); EXPECT_TRUE(es.findIndex(std::numeric_limits::quiet_NaN(), idx)); EXPECT_TRUE(es.findIndex(std::numeric_limits::quiet_NaN(), idx)); @@ -129,10 +129,8 @@ EnumStoreTest::testFindFolded() std::vector indices; std::vector 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(false); + testInsert(false); - testAddEnum(true); + testInsert(true); } template void -EnumStoreTest::testAddEnum(bool hasPostings) +EnumStoreTest::testInsert(bool hasPostings) { - // TODO: Rewrite test to use BatchUpdater EnumStoreType ses(hasPostings); - EnumIndex idx; std::vector indices; std::vector 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 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::getValue(Index idx, EntryType& value) const template EnumStoreT::NonEnumeratedLoader::~NonEnumeratedLoader() = default; +template +void +EnumStoreT::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 void EnumStoreT::writeValues(BufferWriter& writer, vespalib::ConstArrayRef idxs) const @@ -199,12 +210,10 @@ EnumStoreT::freeUnusedEnums(const IndexSet& toRemove) } template -void -EnumStoreT::addEnum(EntryType value, Index& newIdx) +IEnumStore::Index +EnumStoreT::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 -- cgit v1.2.3