Stop using DataStoreBase::startCompact() member function.

16 files changed, 45 insertions, 72 deletions

diff --git a/searchlib/src/vespa/searchlib/memoryindex/feature_store.cpp b/searchlib/src/vespa/searchlib/memoryindex/feature_store.cpp
index d2a28ebc04a..eb5f0ca843b 100644
--- a/searchlib/src/vespa/searchlib/memoryindex/feature_store.cpp
+++ b/searchlib/src/vespa/searchlib/memoryindex/feature_store.cpp
@@ -2,6 +2,9 @@
 
 #include "feature_store.h"
 #include <vespa/searchlib/index/schemautil.h>
+#include <vespa/vespalib/datastore/compacting_buffers.h>
+#include <vespa/vespalib/datastore/compaction_spec.h>
+#include <vespa/vespalib/datastore/compaction_strategy.h>
 #include <vespa/vespalib/datastore/datastore.hpp>
 
 namespace search::memoryindex {
@@ -9,6 +12,8 @@ namespace search::memoryindex {
 constexpr size_t MIN_BUFFER_ARRAYS = 1024u;
 
 using index::SchemaUtil;
+using vespalib::datastore::CompactionSpec;
+using vespalib::datastore::CompactionStrategy;
 using vespalib::datastore::EntryRef;
 
 uint64_t
@@ -143,4 +148,13 @@ FeatureStore::moveFeatures(uint32_t packedIndex, EntryRef ref)
     return moveFeatures(ref, bitLen);
 }
 
+std::unique_ptr<vespalib::datastore::CompactingBuffers>
+FeatureStore::start_compact()
+{
+    // Use a compaction strategy that will compact all active buffers
+    auto compaction_strategy = CompactionStrategy::make_compact_all_active_buffers_strategy();
+    CompactionSpec compaction_spec(true, false);
+    return _store.start_compact_worst_buffers(compaction_spec, compaction_strategy);
+}
+
 }
diff --git a/searchlib/src/vespa/searchlib/memoryindex/feature_store.h b/searchlib/src/vespa/searchlib/memoryindex/feature_store.h
index 3ecb61f1cd1..35a9ebcaa53 100644
--- a/searchlib/src/vespa/searchlib/memoryindex/feature_store.h
+++ b/searchlib/src/vespa/searchlib/memoryindex/feature_store.h
@@ -208,8 +208,7 @@ public:
     void trimHoldLists(generation_t usedGen) { _store.trimHoldLists(usedGen); }
     void transferHoldLists(generation_t generation) { _store.transferHoldLists(generation); }
     void clearHoldLists() { _store.clearHoldLists();}
-    std::vector<uint32_t> startCompact() { return _store.startCompact(_typeId); }
-    void finishCompact(const std::vector<uint32_t> & toHold) { _store.finishCompact(toHold); }
+    std::unique_ptr<vespalib::datastore::CompactingBuffers> start_compact();
     vespalib::MemoryUsage getMemoryUsage() const { return _store.getMemoryUsage(); }
     vespalib::datastore::DataStoreBase::MemStats getMemStats() const { return _store.getMemStats(); }
 };
diff --git a/searchlib/src/vespa/searchlib/memoryindex/field_index.cpp b/searchlib/src/vespa/searchlib/memoryindex/field_index.cpp
index d731be7fe22..da95f2598b7 100644
--- a/searchlib/src/vespa/searchlib/memoryindex/field_index.cpp
+++ b/searchlib/src/vespa/searchlib/memoryindex/field_index.cpp
@@ -103,9 +103,7 @@ template <bool interleaved_features>
 void
 FieldIndex<interleaved_features>::compactFeatures()
 {
-    std::vector<uint32_t> toHold;
-
-    toHold = _featureStore.startCompact();
+    auto compacting_buffers = _featureStore.start_compact();
     auto itr = _dict.begin();
     uint32_t packedIndex = _fieldId;
     for (; itr.valid(); ++itr) {
@@ -143,7 +141,7 @@ FieldIndex<interleaved_features>::compactFeatures()
         }
     }
     using generation_t = GenerationHandler::generation_t;
-    _featureStore.finishCompact(toHold);
+    compacting_buffers->finish();
     generation_t generation = _generationHandler.getCurrentGeneration();
     _featureStore.transferHoldLists(generation);
 }
diff --git a/searchlib/src/vespa/searchlib/test/fakedata/fakememtreeocc.cpp b/searchlib/src/vespa/searchlib/test/fakedata/fakememtreeocc.cpp
index 2597aec3dd7..6e86d70fe52 100644
--- a/searchlib/src/vespa/searchlib/test/fakedata/fakememtreeocc.cpp
+++ b/searchlib/src/vespa/searchlib/test/fakedata/fakememtreeocc.cpp
@@ -12,12 +12,14 @@
 #include <vespa/vespalib/btree/btreenodeallocator.hpp>
 #include <vespa/vespalib/btree/btreenodestore.hpp>
 #include <vespa/vespalib/btree/btreeroot.hpp>
+#include <vespa/vespalib/datastore/compaction_strategy.h>
 
 #include <vespa/log/log.h>
 LOG_SETUP(".fakememtreeocc");
 
 using search::fef::TermFieldMatchData;
 using search::fef::TermFieldMatchDataPosition;
+using vespalib::datastore::CompactionStrategy;
 
 namespace search::fakedata {
 
@@ -280,7 +282,9 @@ FakeMemTreeOccMgr::compactTrees()
 {
     // compact full trees by calling incremental compaction methods in a loop
 
-    std::vector<uint32_t> toHold = _allocator.startCompact();
+    // Use a compaction strategy that will compact all active buffers
+    auto compaction_strategy = CompactionStrategy::make_compact_all_active_buffers_strategy();
+    auto compacting_buffers = _allocator.start_compact_worst(compaction_strategy);
     for (uint32_t wordIdx = 0; wordIdx < _postingIdxs.size(); ++wordIdx) {
         PostingIdx &pidx(*_postingIdxs[wordIdx].get());
         Tree &tree = pidx._tree;
@@ -291,7 +295,7 @@ FakeMemTreeOccMgr::compactTrees()
             itr.moveNextLeafNode();
         }
     }
-    _allocator.finishCompact(toHold);
+    compacting_buffers->finish();
     sync();
 }
 
diff --git a/vespalib/src/tests/btree/btree-stress/btree_stress_test.cpp b/vespalib/src/tests/btree/btree-stress/btree_stress_test.cpp
index c68ff07491e..3ba7bf85e42 100644
--- a/vespalib/src/tests/btree/btree-stress/btree_stress_test.cpp
+++ b/vespalib/src/tests/btree/btree-stress/btree_stress_test.cpp
@@ -66,8 +66,6 @@ public:
     uint32_t get_relaxed(const AtomicEntryRef& ref) const { return get(ref.load_relaxed()); }
     std::unique_ptr<vespalib::datastore::CompactingBuffers> start_compact();
     static constexpr bool is_indirect = true;
-    static uint32_t get_offset_bits() { return StoreRefType::offset_bits; }
-    static uint32_t get_num_buffers() { return StoreRefType::numBuffers(); }
     bool has_held_buffers() const noexcept { return _store.has_held_buffers(); }
 };
 
@@ -82,7 +80,7 @@ std::unique_ptr<vespalib::datastore::CompactingBuffers>
 RealIntStore::start_compact()
 {
     // Use a compaction strategy that will compact all active buffers
-    CompactionStrategy compaction_strategy(0.0, 0.0, get_num_buffers(), 1.0);
+    auto compaction_strategy = CompactionStrategy::make_compact_all_active_buffers_strategy();
     CompactionSpec compaction_spec(true, false);
     return _store.start_compact_worst_buffers(compaction_spec, compaction_strategy);
 }
@@ -329,7 +327,7 @@ void
 Fixture<Params>::compact_tree()
 {
     // Use a compaction strategy that will compact all active buffers
-    CompactionStrategy compaction_strategy(0.0, 0.0, RefType::numBuffers(), 1.0);
+    auto compaction_strategy = CompactionStrategy::make_compact_all_active_buffers_strategy();
     _tree.compact_worst(compaction_strategy);
     _writeItr = _tree.begin();
     _compact_tree.track_compacted();
diff --git a/vespalib/src/tests/btree/btree_test.cpp b/vespalib/src/tests/btree/btree_test.cpp
index 92f55681c0f..3fd00a26189 100644
--- a/vespalib/src/tests/btree/btree_test.cpp
+++ b/vespalib/src/tests/btree/btree_test.cpp
@@ -862,15 +862,17 @@ TEST_F(BTreeTest, require_that_we_can_insert_and_remove_from_tree)
     }
     // compact full tree by calling incremental compaction methods in a loop
     {
+        // Use a compaction strategy that will compact all active buffers
+        auto compaction_strategy = CompactionStrategy::make_compact_all_active_buffers_strategy();
         MyTree::NodeAllocatorType &manager = tree.getAllocator();
-        std::vector<uint32_t> toHold = manager.startCompact();
+        auto compacting_buffers = manager.start_compact_worst(compaction_strategy);
         MyTree::Iterator itr = tree.begin();
         tree.setRoot(itr.moveFirstLeafNode(tree.getRoot()));
         while (itr.valid()) {
             // LOG(info, "Leaf moved to %d", UNWRAP(itr.getKey()));
             itr.moveNextLeafNode();
         }
-        manager.finishCompact(toHold);
+        compacting_buffers->finish();
         manager.freeze();
         manager.transferHoldLists(g.getCurrentGeneration());
         g.incGeneration();
diff --git a/vespalib/src/tests/btree/btreeaggregation_test.cpp b/vespalib/src/tests/btree/btreeaggregation_test.cpp
index f4300499fcd..dff7de6660f 100644
--- a/vespalib/src/tests/btree/btreeaggregation_test.cpp
+++ b/vespalib/src/tests/btree/btreeaggregation_test.cpp
@@ -15,6 +15,7 @@
 #include <vespa/vespalib/btree/btreestore.hpp>
 #include <vespa/vespalib/btree/btreeaggregator.hpp>
 #include <vespa/vespalib/datastore/buffer_type.hpp>
+#include <vespa/vespalib/datastore/compaction_strategy.h>
 #include <vespa/vespalib/test/btree/btree_printer.h>
 #include <vespa/vespalib/testkit/testapp.h>
 #include <vespa/vespalib/util/rand48.h>
@@ -28,6 +29,7 @@
 LOG_SETUP("btreeaggregation_test");
 
 using vespalib::GenerationHandler;
+using vespalib::datastore::CompactionStrategy;
 using vespalib::datastore::EntryRef;
 
 namespace vespalib::btree {
@@ -877,15 +879,17 @@ Test::requireThatWeCanInsertAndRemoveFromTree()
     }
     // compact full tree by calling incremental compaction methods in a loop
     {
+        // Use a compaction strategy that will compact all active buffers
+        auto compaction_strategy = CompactionStrategy::make_compact_all_active_buffers_strategy();
         MyTree::NodeAllocatorType &manager = tree.getAllocator();
-        std::vector<uint32_t> toHold = manager.startCompact();
+        auto compacting_buffers = manager.start_compact_worst(compaction_strategy);
         MyTree::Iterator itr = tree.begin();
         tree.setRoot(itr.moveFirstLeafNode(tree.getRoot()));
         while (itr.valid()) {
             // LOG(info, "Leaf moved to %d", UNWRAP(itr.getKey()));
             itr.moveNextLeafNode();
         }
-        manager.finishCompact(toHold);
+        compacting_buffers->finish();
         manager.freeze();
         manager.transferHoldLists(g.getCurrentGeneration());
         g.incGeneration();
diff --git a/vespalib/src/tests/datastore/unique_store/unique_store_test.cpp b/vespalib/src/tests/datastore/unique_store/unique_store_test.cpp
index 92bd5502406..56c1d0c0f63 100644
--- a/vespalib/src/tests/datastore/unique_store/unique_store_test.cpp
+++ b/vespalib/src/tests/datastore/unique_store/unique_store_test.cpp
@@ -120,7 +120,7 @@ struct TestBase : public ::testing::Test {
     void compactWorst() {
         CompactionSpec compaction_spec(true, true);
         // Use a compaction strategy that will compact all active buffers
-        CompactionStrategy compaction_strategy(0.0, 0.0, EntryRefType::numBuffers(), 1.0);
+        auto compaction_strategy = CompactionStrategy::make_compact_all_active_buffers_strategy();
         auto remapper = store.compact_worst(compaction_spec, compaction_strategy);
         std::vector<AtomicEntryRef> refs;
         for (const auto &elem : refStore) {
diff --git a/vespalib/src/vespa/vespalib/btree/btreenodeallocator.h b/vespalib/src/vespa/vespalib/btree/btreenodeallocator.h
index 86c9621f869..c631ac4041a 100644
--- a/vespalib/src/vespa/vespalib/btree/btreenodeallocator.h
+++ b/vespalib/src/vespa/vespalib/btree/btreenodeallocator.h
@@ -164,14 +164,9 @@ public:
     vespalib::string toString(const BTreeNode * node) const;
 
     bool getCompacting(EntryRef ref) const { return _nodeStore.getCompacting(ref); }
-    std::vector<uint32_t> startCompact() { return _nodeStore.startCompact(); }
 
     std::unique_ptr<vespalib::datastore::CompactingBuffers> start_compact_worst(const CompactionStrategy& compaction_strategy) { return _nodeStore.start_compact_worst(compaction_strategy); }
 
-    void finishCompact(const std::vector<uint32_t> &toHold) {
-        return _nodeStore.finishCompact(toHold);
-    }
-
     template <typename FunctionType>
     void foreach_key(EntryRef ref, FunctionType func) const {
         _nodeStore.foreach_key(ref, func);
diff --git a/vespalib/src/vespa/vespalib/btree/btreenodestore.h b/vespalib/src/vespa/vespalib/btree/btreenodestore.h
index d05ec840f83..7a03c236637 100644
--- a/vespalib/src/vespa/vespalib/btree/btreenodestore.h
+++ b/vespalib/src/vespa/vespalib/btree/btreenodestore.h
@@ -160,12 +160,8 @@ public:
         _store.freeElem(ref, 1);
     }
 
-    std::vector<uint32_t> startCompact();
-
     std::unique_ptr<vespalib::datastore::CompactingBuffers> start_compact_worst(const CompactionStrategy& compaction_strategy);
 
-    void finishCompact(const std::vector<uint32_t> &toHold);
-
     void transferHoldLists(generation_t generation) {
         _store.transferHoldLists(generation);
     }
diff --git a/vespalib/src/vespa/vespalib/btree/btreenodestore.hpp b/vespalib/src/vespa/vespalib/btree/btreenodestore.hpp
index 0f9eeb9daec..a1ffb4d445d 100644
--- a/vespalib/src/vespa/vespalib/btree/btreenodestore.hpp
+++ b/vespalib/src/vespa/vespalib/btree/btreenodestore.hpp
@@ -55,20 +55,6 @@ BTreeNodeStore<KeyT, DataT, AggrT, INTERNAL_SLOTS, LEAF_SLOTS>::
     _store.dropBuffers(); // Drop buffers before type handlers are dropped
 }
 
-
-template <typename KeyT, typename DataT, typename AggrT,
-          size_t INTERNAL_SLOTS, size_t LEAF_SLOTS>
-std::vector<uint32_t>
-BTreeNodeStore<KeyT, DataT, AggrT, INTERNAL_SLOTS, LEAF_SLOTS>::
-startCompact()
-{
-    std::vector<uint32_t> iToHold = _store.startCompact(NODETYPE_INTERNAL);
-    std::vector<uint32_t> lToHold = _store.startCompact(NODETYPE_LEAF);
-    std::vector<uint32_t> ret = iToHold;
-    ret.insert(ret.end(), lToHold.begin(), lToHold.end());
-    return ret;
-}
-
 template <typename KeyT, typename DataT, typename AggrT,
           size_t INTERNAL_SLOTS, size_t LEAF_SLOTS>
 std::unique_ptr<vespalib::datastore::CompactingBuffers>
@@ -78,15 +64,6 @@ start_compact_worst(const CompactionStrategy &compaction_strategy)
     return _store.start_compact_worst_buffers(datastore::CompactionSpec(true, false), compaction_strategy);
 }
 
-template <typename KeyT, typename DataT, typename AggrT,
-          size_t INTERNAL_SLOTS, size_t LEAF_SLOTS>
-void
-BTreeNodeStore<KeyT, DataT, AggrT, INTERNAL_SLOTS, LEAF_SLOTS>::
-finishCompact(const std::vector<uint32_t> &toHold)
-{
-    _store.finishCompact(toHold);
-}
-
 }
 
 #define VESPALIB_DATASTORE_INSTANTIATE_BUFFERTYPE_INTERNALNODE(K, A, S) \
diff --git a/vespalib/src/vespa/vespalib/datastore/compact_buffer_candidates.cpp b/vespalib/src/vespa/vespalib/datastore/compact_buffer_candidates.cpp
index 41c216a9684..dd47a159e9b 100644
--- a/vespalib/src/vespa/vespalib/datastore/compact_buffer_candidates.cpp
+++ b/vespalib/src/vespa/vespalib/datastore/compact_buffer_candidates.cpp
@@ -13,7 +13,7 @@ CompactBufferCandidates::CompactBufferCandidates(uint32_t num_buffers, uint32_t
       _max_buffers(std::max(max_buffers, 1u)),
       _active_buffers_ratio(std::min(1.0, std::max(0.0001, active_buffers_ratio))),
       _ratio(ratio),
-      _slack(slack),
+      _slack(_ratio == 0.0 ? 0u : slack),
       _free_buffers(0)
 {
     _candidates.reserve(num_buffers);
diff --git a/vespalib/src/vespa/vespalib/datastore/compaction_strategy.cpp b/vespalib/src/vespa/vespalib/datastore/compaction_strategy.cpp
index 2dbd501f78e..4eb4ff16864 100644
--- a/vespalib/src/vespa/vespalib/datastore/compaction_strategy.cpp
+++ b/vespalib/src/vespa/vespalib/datastore/compaction_strategy.cpp
@@ -5,6 +5,7 @@
 #include <vespa/vespalib/util/memoryusage.h>
 #include <vespa/vespalib/util/address_space.h>
 #include <iostream>
+#include <limits>
 
 namespace vespalib::datastore {
 
@@ -34,4 +35,10 @@ std::ostream& operator<<(std::ostream& os, const CompactionStrategy& compaction_
     return os;
 }
 
+CompactionStrategy
+CompactionStrategy::make_compact_all_active_buffers_strategy()
+{
+    return CompactionStrategy(0.0, 0.0, std::numeric_limits<uint32_t>::max(), 1.0);
+}
+
 }
diff --git a/vespalib/src/vespa/vespalib/datastore/compaction_strategy.h b/vespalib/src/vespa/vespalib/datastore/compaction_strategy.h
index 2bcf30fc6fc..f78e123e5de 100644
--- a/vespalib/src/vespa/vespalib/datastore/compaction_strategy.h
+++ b/vespalib/src/vespa/vespalib/datastore/compaction_strategy.h
@@ -74,6 +74,7 @@ public:
     bool should_compact_memory(const MemoryUsage& memory_usage) const;
     bool should_compact_address_space(const AddressSpace& address_space) const;
     CompactionSpec should_compact(const MemoryUsage& memory_usage, const AddressSpace& address_space) const;
+    static CompactionStrategy make_compact_all_active_buffers_strategy();
 };
 
 std::ostream& operator<<(std::ostream& os, const CompactionStrategy& compaction_strategy);
diff --git a/vespalib/src/vespa/vespalib/datastore/datastorebase.cpp b/vespalib/src/vespa/vespalib/datastore/datastorebase.cpp
index 67749ee913d..a67fceec1b5 100644
--- a/vespalib/src/vespa/vespalib/datastore/datastorebase.cpp
+++ b/vespalib/src/vespa/vespalib/datastore/datastorebase.cpp
@@ -427,26 +427,6 @@ DataStoreBase::onActive(uint32_t bufferId, uint32_t typeId, size_t elemsNeeded)
     enableFreeList(bufferId);
 }
 
-std::vector<uint32_t>
-DataStoreBase::startCompact(uint32_t typeId)
-{
-    std::vector<uint32_t> toHold;
-
-    for (uint32_t bufferId = 0; bufferId < _numBuffers; ++bufferId) {
-        BufferState &state = getBufferState(bufferId);
-        if (state.isActive() &&
-            state.getTypeId() == typeId &&
-            !state.getCompacting()) {
-            state.setCompacting();
-            toHold.push_back(bufferId);
-            disableFreeList(bufferId);
-        }
-    }
-    switch_primary_buffer(typeId, 0u);
-    inc_compaction_count();
-    return toHold;
-}
-
 void
 DataStoreBase::finishCompact(const std::vector<uint32_t> &toHold)
 {
diff --git a/vespalib/src/vespa/vespalib/datastore/datastorebase.h b/vespalib/src/vespa/vespalib/datastore/datastorebase.h
index d83b3a84847..bbd0ced123c 100644
--- a/vespalib/src/vespa/vespalib/datastore/datastorebase.h
+++ b/vespalib/src/vespa/vespalib/datastore/datastorebase.h
@@ -364,8 +364,6 @@ public:
         return _buffers[bufferId].getTypeId();
     }
 
-    std::vector<uint32_t> startCompact(uint32_t typeId);
-
     void finishCompact(const std::vector<uint32_t> &toHold);
     void fallbackResize(uint32_t bufferId, size_t elementsNeeded);