Change DataStore::addEntry() to use free lists if enabled.

3 files changed, 28 insertions, 40 deletions

diff --git a/vespalib/src/tests/datastore/datastore/datastore_test.cpp b/vespalib/src/tests/datastore/datastore/datastore_test.cpp
index 2429ad3853f..00da8835c06 100644
--- a/vespalib/src/tests/datastore/datastore/datastore_test.cpp
+++ b/vespalib/src/tests/datastore/datastore/datastore_test.cpp
@@ -337,6 +337,12 @@ operator<<(std::ostream &os, const IntHandle &rhs)
 }
 
 void
+expect_successive_refs(EntryRef first, EntryRef second)
+{
+    EXPECT_EQ(MyRef(first).offset() + 1, MyRef(second).offset());
+}
+
+void
 expect_successive_handles(const IntHandle &first, const IntHandle &second)
 {
     EXPECT_EQ(to_ref(first).offset() + 1, to_ref(second).offset());
@@ -346,30 +352,29 @@ TEST(DataStoreTest, require_that_we_can_use_free_lists)
 {
     MyStore s;
     s.enableFreeLists();
-    auto allocator = s.freeListAllocator<IntReclaimer>();
-    auto h1 = allocator.alloc(1);
-    s.holdElem(h1.ref, 1);
+    auto r1 = s.addEntry(1);
+    s.holdElem(r1, 1);
     s.transferHoldLists(10);
-    auto h2 = allocator.alloc(2);
-    expect_successive_handles(h1, h2);
-    s.holdElem(h2.ref, 1);
+    auto r2 = s.addEntry(2);
+    expect_successive_refs(r1, r2);
+    s.holdElem(r2, 1);
     s.transferHoldLists(20);
     s.trimElemHoldList(11);
-    auto h3 = allocator.alloc(3); // reuse h1.ref
-    EXPECT_EQ(h1, h3);
-    auto h4 = allocator.alloc(4);
-    expect_successive_handles(h2, h4);
+    auto r3 = s.addEntry(3); // reuse r1
+    EXPECT_EQ(r1, r3);
+    auto r4 = s.addEntry(4);
+    expect_successive_refs(r2, r4);
     s.trimElemHoldList(21);
-    auto h5 = allocator.alloc(5); // reuse h2.ref
-    EXPECT_EQ(h2, h5);
-    auto h6 = allocator.alloc(6);
-    expect_successive_handles(h4, h6);
-    EXPECT_EQ(3, s.getEntry(h1.ref));
-    EXPECT_EQ(5, s.getEntry(h2.ref));
-    EXPECT_EQ(3, s.getEntry(h3.ref));
-    EXPECT_EQ(4, s.getEntry(h4.ref));
-    EXPECT_EQ(5, s.getEntry(h5.ref));
-    EXPECT_EQ(6, s.getEntry(h6.ref));
+    auto r5 = s.addEntry(5); // reuse r2
+    EXPECT_EQ(r2, r5);
+    auto r6 = s.addEntry(6);
+    expect_successive_refs(r4, r6);
+    EXPECT_EQ(3, s.getEntry(r1));
+    EXPECT_EQ(5, s.getEntry(r2));
+    EXPECT_EQ(3, s.getEntry(r3));
+    EXPECT_EQ(4, s.getEntry(r4));
+    EXPECT_EQ(5, s.getEntry(r5));
+    EXPECT_EQ(6, s.getEntry(r6));
 }
 
 TEST(DataStoreTest, require_that_we_can_use_free_lists_with_raw_allocator)
diff --git a/vespalib/src/vespa/vespalib/datastore/datastore.h b/vespalib/src/vespa/vespalib/datastore/datastore.h
index 4bcd06fdb21..f437616af2a 100644
--- a/vespalib/src/vespa/vespalib/datastore/datastore.h
+++ b/vespalib/src/vespa/vespalib/datastore/datastore.h
@@ -110,9 +110,6 @@ public:
 
     EntryRef addEntry(const EntryType &e);
     const EntryType &getEntry(EntryRef ref) const;
-
-    template <typename ReclaimerT>
-    FreeListAllocator<EntryType, RefT, ReclaimerT> freeListAllocator();
 };
 
 extern template class DataStoreT<EntryRefT<22> >;
diff --git a/vespalib/src/vespa/vespalib/datastore/datastore.hpp b/vespalib/src/vespa/vespalib/datastore/datastore.hpp
index ad35f3c7383..71cc52a6838 100644
--- a/vespalib/src/vespa/vespalib/datastore/datastore.hpp
+++ b/vespalib/src/vespa/vespalib/datastore/datastore.hpp
@@ -156,15 +156,9 @@ template <typename EntryType, typename RefT>
 EntryRef
 DataStore<EntryType, RefT>::addEntry(const EntryType &e)
 {
-    ensureBufferCapacity(0, 1);
-    uint32_t activeBufferId = _activeBufferIds[0];
-    BufferState &state = this->getBufferState(activeBufferId);
-    size_t oldSize = state.size();
-    EntryType *be = static_cast<EntryType *>(this->getBuffer(activeBufferId)) + oldSize;
-    new (static_cast<void *>(be)) EntryType(e);
-    RefType ref(oldSize, activeBufferId);
-    state.pushed_back(1);
-    return ref;
+    using NoOpReclaimer = DefaultReclaimer<EntryType>;
+    // Note: This will fallback to regular allocation if free lists are not enabled.
+    return FreeListAllocator<EntryType, RefT, NoOpReclaimer>(*this, 0).alloc(e).ref;
 }
 
 template <typename EntryType, typename RefT>
@@ -176,14 +170,6 @@ DataStore<EntryType, RefT>::getEntry(EntryRef ref) const
     return *be;
 }
 
-template <typename EntryType, typename RefT>
-template <typename ReclaimerT>
-FreeListAllocator<EntryType, RefT, ReclaimerT>
-DataStore<EntryType, RefT>::freeListAllocator()
-{
-    return FreeListAllocator<EntryType, RefT, ReclaimerT>(*this, 0);
-}
-
 extern template class DataStoreT<EntryRefT<22> >;
 
 }