diff options
author | Tor Egge <Tor.Egge@online.no> | 2022-10-14 16:25:41 +0200 |
---|---|---|
committer | Tor Egge <Tor.Egge@online.no> | 2022-10-14 16:25:41 +0200 |
commit | 4bca2e390cbfbf46cdf3558bd2fcdbc9713e18a9 (patch) | |
tree | 95809b8417858e3bfcfc9a92890603553c51b0e4 /vespalib | |
parent | 86eda2f92b594723b679ca137eae847c4668456c (diff) |
Stop bypassing hold list for btree nodes that were never reachable for readers.
Diffstat (limited to 'vespalib')
7 files changed, 16 insertions, 45 deletions
diff --git a/vespalib/src/tests/datastore/datastore/datastore_test.cpp b/vespalib/src/tests/datastore/datastore/datastore_test.cpp index 10b96a87444..645871d3ef6 100644 --- a/vespalib/src/tests/datastore/datastore/datastore_test.cpp +++ b/vespalib/src/tests/datastore/datastore/datastore_test.cpp @@ -670,7 +670,7 @@ TEST(DataStoreTest, control_static_sizes) { namespace { -void test_free_element_to_held_buffer(bool direct, bool before_hold_buffer) +void test_free_element_to_held_buffer(bool before_hold_buffer) { MyStore s; auto ref = s.addEntry(1); @@ -679,19 +679,11 @@ void test_free_element_to_held_buffer(bool direct, bool before_hold_buffer) EXPECT_EQ(1u, s.primary_buffer_id()); if (before_hold_buffer) { - if (direct) { - s.freeElem(ref, 1); - } else { - s.holdElem(ref, 1); - } + s.holdElem(ref, 1); } s.holdBuffer(0); // hold last buffer if (!before_hold_buffer) { - if (direct) { - ASSERT_DEATH({ s.freeElem(ref, 1); }, "isOnHold\\(\\) && was_held"); - } else { - ASSERT_DEATH({ s.holdElem(ref, 1); }, "isActive\\(\\)"); - } + ASSERT_DEATH({ s.holdElem(ref, 1); }, "isActive\\(\\)"); } s.assign_generation(100); s.reclaim_memory(101); @@ -699,25 +691,15 @@ void test_free_element_to_held_buffer(bool direct, bool before_hold_buffer) } -TEST(DataStoreTest, free_to_active_then_held_buffer_is_ok) -{ - test_free_element_to_held_buffer(true, true); -} - TEST(DataStoreTest, hold_to_active_then_held_buffer_is_ok) { - test_free_element_to_held_buffer(false, true); + test_free_element_to_held_buffer(true); } #ifndef NDEBUG -TEST(DataStoreDeathTest, free_to_held_buffer_is_not_ok) -{ - test_free_element_to_held_buffer(true, false); -} - TEST(DataStoreDeathTest, hold_to_held_buffer_is_not_ok) { - test_free_element_to_held_buffer(false, false); + test_free_element_to_held_buffer(false); } #endif diff --git a/vespalib/src/vespa/vespalib/btree/btreenodeallocator.hpp b/vespalib/src/vespa/vespalib/btree/btreenodeallocator.hpp index 4968bbaf4a7..a38b68afe73 100644 --- a/vespalib/src/vespa/vespalib/btree/btreenodeallocator.hpp +++ b/vespalib/src/vespa/vespalib/btree/btreenodeallocator.hpp @@ -235,7 +235,7 @@ freeze() InternalNodeType *inode = mapInternalRef(i); (void) inode; assert(inode->getFrozen()); - _nodeStore.freeElem(i); + _nodeStore.holdElem(i); } _internalHoldUntilFreeze.clear(); } @@ -245,7 +245,7 @@ freeze() LeafNodeType *lnode = mapLeafRef(i); (void) lnode; assert(lnode->getFrozen()); - _nodeStore.freeElem(i); + _nodeStore.holdElem(i); } _leafHoldUntilFreeze.clear(); } diff --git a/vespalib/src/vespa/vespalib/btree/btreenodestore.h b/vespalib/src/vespa/vespalib/btree/btreenodestore.h index 7b89e2d0ddb..a63a4d20170 100644 --- a/vespalib/src/vespa/vespalib/btree/btreenodestore.h +++ b/vespalib/src/vespa/vespalib/btree/btreenodestore.h @@ -156,10 +156,6 @@ public: _store.holdElem(ref, 1); } - void freeElem(EntryRef ref) { - _store.freeElem(ref, 1); - } - std::unique_ptr<vespalib::datastore::CompactingBuffers> start_compact_worst(const CompactionStrategy& compaction_strategy); void assign_generation(generation_t current_gen) { diff --git a/vespalib/src/vespa/vespalib/datastore/bufferstate.cpp b/vespalib/src/vespa/vespalib/datastore/bufferstate.cpp index e9ff243fb08..45a94693eeb 100644 --- a/vespalib/src/vespa/vespalib/datastore/bufferstate.cpp +++ b/vespalib/src/vespa/vespalib/datastore/bufferstate.cpp @@ -191,19 +191,17 @@ BufferState::hold_elems(size_t num_elems, size_t extra_bytes) } void -BufferState::free_elems(EntryRef ref, size_t num_elems, bool was_held, size_t ref_offset) +BufferState::free_elems(EntryRef ref, size_t num_elems, size_t ref_offset) { if (isActive()) { if (_free_list.enabled() && (num_elems == getArraySize())) { _free_list.push_entry(ref); } } else { - assert(isOnHold() && was_held); + assert(isOnHold()); } _stats.inc_dead_elems(num_elems); - if (was_held) { - _stats.dec_hold_elems(num_elems); - } + _stats.dec_hold_elems(num_elems); getTypeHandler()->cleanHold(_buffer.get(), (ref_offset * _arraySize), num_elems, BufferTypeBase::CleanContext(_stats.extra_used_bytes_ref(), _stats.extra_hold_bytes_ref())); diff --git a/vespalib/src/vespa/vespalib/datastore/bufferstate.h b/vespalib/src/vespa/vespalib/datastore/bufferstate.h index 0acad7d3ab1..3f023b41c51 100644 --- a/vespalib/src/vespa/vespalib/datastore/bufferstate.h +++ b/vespalib/src/vespa/vespalib/datastore/bufferstate.h @@ -100,7 +100,7 @@ public: * The given entry ref is put on the free list (if enabled). * Hold cleaning of elements is executed on the buffer type. */ - void free_elems(EntryRef ref, size_t num_elems, bool was_held, size_t ref_offset); + void free_elems(EntryRef ref, size_t num_elems, size_t ref_offset); BufferStats& stats() { return _stats; } const BufferStats& stats() const { return _stats; } diff --git a/vespalib/src/vespa/vespalib/datastore/datastore.h b/vespalib/src/vespa/vespalib/datastore/datastore.h index 25b451deae7..95f47e98ef5 100644 --- a/vespalib/src/vespa/vespalib/datastore/datastore.h +++ b/vespalib/src/vespa/vespalib/datastore/datastore.h @@ -27,7 +27,7 @@ template <typename RefT = EntryRefT<22> > class DataStoreT : public DataStoreBase { private: - void free_elem_internal(EntryRef ref, size_t numElems, bool was_held); + void free_elem_internal(EntryRef ref, size_t numElems); public: typedef RefT RefType; @@ -38,11 +38,6 @@ public: ~DataStoreT() override; /** - * Free element(s). - */ - void freeElem(EntryRef ref, size_t numElems) { free_elem_internal(ref, numElems, false); } - - /** * Hold element(s). */ void holdElem(EntryRef ref, size_t numElems) { diff --git a/vespalib/src/vespa/vespalib/datastore/datastore.hpp b/vespalib/src/vespa/vespalib/datastore/datastore.hpp index 190ef994c0b..bfb63954875 100644 --- a/vespalib/src/vespa/vespalib/datastore/datastore.hpp +++ b/vespalib/src/vespa/vespalib/datastore/datastore.hpp @@ -22,11 +22,11 @@ DataStoreT<RefT>::~DataStoreT() = default; template <typename RefT> void -DataStoreT<RefT>::free_elem_internal(EntryRef ref, size_t numElems, bool was_held) +DataStoreT<RefT>::free_elem_internal(EntryRef ref, size_t numElems) { RefType intRef(ref); BufferState &state = getBufferState(intRef.bufferId()); - state.free_elems(ref, numElems, was_held, intRef.offset()); + state.free_elems(ref, numElems, intRef.offset()); } template <typename RefT> @@ -45,7 +45,7 @@ void DataStoreT<RefT>::reclaim_entry_refs(generation_t oldest_used_gen) { _entry_ref_hold_list.reclaim(oldest_used_gen, [this](const auto& elem) { - free_elem_internal(elem.ref, elem.num_elems, true); + free_elem_internal(elem.ref, elem.num_elems); }); } @@ -54,7 +54,7 @@ void DataStoreT<RefT>::reclaim_all_entry_refs() { _entry_ref_hold_list.reclaim_all([this](const auto& elem) { - free_elem_internal(elem.ref, elem.num_elems, true); + free_elem_internal(elem.ref, elem.num_elems); }); } |