diff options
author | Geir Storli <geirst@yahooinc.com> | 2023-04-11 15:59:42 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-04-11 15:59:42 +0200 |
commit | d4eeb7d873937b00869cd92a3538184b9863bd86 (patch) | |
tree | 40321b7bbb79a508980fd451b0240705019ec4fd | |
parent | 627a6e0d83794251a03854e383c3b829c6b21aba (diff) | |
parent | 1bc23ddd8b291331e33fc3822e4497d9ace55767 (diff) |
Merge pull request #26701 from vespa-engine/toregge/remove-array-size-from-buffer-free-list
Remove array size from BufferFreeList.
14 files changed, 40 insertions, 45 deletions
diff --git a/searchlib/src/tests/attribute/attribute_test.cpp b/searchlib/src/tests/attribute/attribute_test.cpp index 4549291b80f..91666eb29fd 100644 --- a/searchlib/src/tests/attribute/attribute_test.cpp +++ b/searchlib/src/tests/attribute/attribute_test.cpp @@ -913,7 +913,7 @@ AttributeTest::testSingle() AttributePtr ptr = createAttribute("sv-post-int32", cfg); ptr->updateStat(true); EXPECT_EQ(338972u, ptr->getStatus().getAllocated()); - EXPECT_EQ(101612u, ptr->getStatus().getUsed()); + EXPECT_EQ(101492u, ptr->getStatus().getUsed()); addDocs(ptr, numDocs); testSingle<IntegerAttribute, AttributeVector::largeint_t, int32_t>(ptr, values); } @@ -935,7 +935,7 @@ AttributeTest::testSingle() AttributePtr ptr = createAttribute("sv-post-float", cfg); ptr->updateStat(true); EXPECT_EQ(338972u, ptr->getStatus().getAllocated()); - EXPECT_EQ(101612u, ptr->getStatus().getUsed()); + EXPECT_EQ(101492u, ptr->getStatus().getUsed()); addDocs(ptr, numDocs); testSingle<FloatingPointAttribute, double, float>(ptr, values); } @@ -948,7 +948,7 @@ AttributeTest::testSingle() AttributePtr ptr = createAttribute("sv-string", Config(BasicType::STRING, CollectionType::SINGLE)); ptr->updateStat(true); EXPECT_EQ(116528u + sizeof_large_string_entry, ptr->getStatus().getAllocated()); - EXPECT_EQ(52920u + sizeof_large_string_entry, ptr->getStatus().getUsed()); + EXPECT_EQ(52760u + sizeof_large_string_entry, ptr->getStatus().getUsed()); addDocs(ptr, numDocs); testSingle<StringAttribute, string, string>(ptr, values); } @@ -958,7 +958,7 @@ AttributeTest::testSingle() AttributePtr ptr = createAttribute("sv-fs-string", cfg); ptr->updateStat(true); EXPECT_EQ(344848u + sizeof_large_string_entry, ptr->getStatus().getAllocated()); - EXPECT_EQ(104664u + sizeof_large_string_entry, ptr->getStatus().getUsed()); + EXPECT_EQ(104408u + sizeof_large_string_entry, ptr->getStatus().getUsed()); addDocs(ptr, numDocs); testSingle<StringAttribute, string, string>(ptr, values); } @@ -1089,8 +1089,8 @@ AttributeTest::testArray() { AttributePtr ptr = createAttribute("a-int32", Config(BasicType::INT32, CollectionType::ARRAY)); ptr->updateStat(true); - EXPECT_EQ(495672u, ptr->getStatus().getAllocated()); - EXPECT_EQ(487944u, ptr->getStatus().getUsed()); + EXPECT_EQ(487480u, ptr->getStatus().getAllocated()); + EXPECT_EQ(479720u, ptr->getStatus().getUsed()); addDocs(ptr, numDocs); testArray<IntegerAttribute, AttributeVector::largeint_t>(ptr, values); } @@ -1099,8 +1099,8 @@ AttributeTest::testArray() cfg.setFastSearch(true); AttributePtr ptr = createAttribute("flags", cfg); ptr->updateStat(true); - EXPECT_EQ(495672u, ptr->getStatus().getAllocated()); - EXPECT_EQ(487944u, ptr->getStatus().getUsed()); + EXPECT_EQ(487480u, ptr->getStatus().getAllocated()); + EXPECT_EQ(479720u, ptr->getStatus().getUsed()); addDocs(ptr, numDocs); testArray<IntegerAttribute, AttributeVector::largeint_t>(ptr, values); } @@ -1109,8 +1109,8 @@ AttributeTest::testArray() cfg.setFastSearch(true); AttributePtr ptr = createAttribute("a-fs-int32", cfg); ptr->updateStat(true); - EXPECT_EQ(852308u, ptr->getStatus().getAllocated()); - EXPECT_EQ(589576u, ptr->getStatus().getUsed()); + EXPECT_EQ(844116u, ptr->getStatus().getAllocated()); + EXPECT_EQ(581232u, ptr->getStatus().getUsed()); addDocs(ptr, numDocs); testArray<IntegerAttribute, AttributeVector::largeint_t>(ptr, values); } @@ -1128,8 +1128,8 @@ AttributeTest::testArray() cfg.setFastSearch(true); AttributePtr ptr = createAttribute("a-fs-float", cfg); ptr->updateStat(true); - EXPECT_EQ(852308u, ptr->getStatus().getAllocated()); - EXPECT_EQ(589576u, ptr->getStatus().getUsed()); + EXPECT_EQ(844116u, ptr->getStatus().getAllocated()); + EXPECT_EQ(581232u, ptr->getStatus().getUsed()); addDocs(ptr, numDocs); testArray<FloatingPointAttribute, double>(ptr, values); } @@ -1140,8 +1140,8 @@ AttributeTest::testArray() { AttributePtr ptr = createAttribute("a-string", Config(BasicType::STRING, CollectionType::ARRAY)); ptr->updateStat(true); - EXPECT_EQ(607976u + sizeof_large_string_entry, ptr->getStatus().getAllocated()); - EXPECT_EQ(540864u + sizeof_large_string_entry, ptr->getStatus().getUsed()); + EXPECT_EQ(599784u + sizeof_large_string_entry, ptr->getStatus().getAllocated()); + EXPECT_EQ(532480u + sizeof_large_string_entry, ptr->getStatus().getUsed()); addDocs(ptr, numDocs); testArray<StringAttribute, string>(ptr, values); } @@ -1150,8 +1150,8 @@ AttributeTest::testArray() cfg.setFastSearch(true); AttributePtr ptr = createAttribute("afs-string", cfg); ptr->updateStat(true); - EXPECT_EQ(858184u + sizeof_large_string_entry, ptr->getStatus().getAllocated()); - EXPECT_EQ(592628u + sizeof_large_string_entry, ptr->getStatus().getUsed()); + EXPECT_EQ(849992u + sizeof_large_string_entry, ptr->getStatus().getAllocated()); + EXPECT_EQ(584148u + sizeof_large_string_entry, ptr->getStatus().getUsed()); addDocs(ptr, numDocs); testArray<StringAttribute, string>(ptr, values); } diff --git a/searchlib/src/tests/memoryindex/memory_index/memory_index_test.cpp b/searchlib/src/tests/memoryindex/memory_index/memory_index_test.cpp index 388b708f9aa..e3b9cf9702d 100644 --- a/searchlib/src/tests/memoryindex/memory_index/memory_index_test.cpp +++ b/searchlib/src/tests/memoryindex/memory_index/memory_index_test.cpp @@ -463,7 +463,7 @@ TEST(MemoryIndexTest, require_that_num_docs_and_doc_id_limit_is_returned) TEST(MemoryIndexTest, require_that_we_understand_the_memory_footprint) { constexpr size_t BASE_ALLOCATED = 360936u; - constexpr size_t BASE_USED = 150932u; + constexpr size_t BASE_USED = 150804u; { MySetup setup; Index index(setup); diff --git a/searchlib/src/tests/predicate/document_features_store_test.cpp b/searchlib/src/tests/predicate/document_features_store_test.cpp index ee247f8c74e..d30df9dba6e 100644 --- a/searchlib/src/tests/predicate/document_features_store_test.cpp +++ b/searchlib/src/tests/predicate/document_features_store_test.cpp @@ -165,17 +165,17 @@ TEST("require that both features and ranges are removed by 'remove'") { TEST("require that both features and ranges counts towards memory usage") { DocumentFeaturesStore features_store(10); - EXPECT_EQUAL(50088u, features_store.getMemoryUsage().usedBytes()); + EXPECT_EQUAL(50064u, features_store.getMemoryUsage().usedBytes()); PredicateTreeAnnotations annotations; annotations.features.push_back(PredicateHash::hash64("foo=100-199")); features_store.insert(annotations, doc_id); - EXPECT_EQUAL(50096u, features_store.getMemoryUsage().usedBytes()); + EXPECT_EQUAL(50072u, features_store.getMemoryUsage().usedBytes()); annotations.features.clear(); annotations.range_features.push_back({"foo", 100, 199}); features_store.insert(annotations, doc_id + 1); - EXPECT_EQUAL(50192u, features_store.getMemoryUsage().usedBytes()); + EXPECT_EQUAL(50168u, features_store.getMemoryUsage().usedBytes()); } TEST("require that DocumentFeaturesStore can be serialized") { @@ -205,17 +205,17 @@ TEST("require that serialization cleans up wordstore") { PredicateTreeAnnotations annotations; annotations.range_features.push_back({"foo", 100, 199}); features_store.insert(annotations, doc_id); - EXPECT_EQUAL(50184u, features_store.getMemoryUsage().usedBytes()); + EXPECT_EQUAL(50160u, features_store.getMemoryUsage().usedBytes()); annotations.range_features.push_back({"bar", 100, 199}); features_store.insert(annotations, doc_id + 1); - EXPECT_EQUAL(50572u, features_store.getMemoryUsage().usedBytes()); + EXPECT_EQUAL(50548u, features_store.getMemoryUsage().usedBytes()); features_store.remove(doc_id + 1); - EXPECT_EQUAL(50524u, features_store.getMemoryUsage().usedBytes()); + EXPECT_EQUAL(50500u, features_store.getMemoryUsage().usedBytes()); vespalib::DataBuffer buffer; features_store.serialize(buffer); DocumentFeaturesStore features_store2(buffer); - EXPECT_EQUAL(50184u, features_store2.getMemoryUsage().usedBytes()); + EXPECT_EQUAL(50160u, features_store2.getMemoryUsage().usedBytes()); } diff --git a/vespalib/src/tests/btree/btree_test.cpp b/vespalib/src/tests/btree/btree_test.cpp index 386d38bac29..afad3523fa3 100644 --- a/vespalib/src/tests/btree/btree_test.cpp +++ b/vespalib/src/tests/btree/btree_test.cpp @@ -1065,7 +1065,7 @@ adjustAllocatedBytes(size_t nodeCount, size_t nodeSize) TEST_F(BTreeTest, require_that_memory_usage_is_calculated) { constexpr size_t BASE_ALLOCATED = 28744u; - constexpr size_t BASE_USED = 24952; + constexpr size_t BASE_USED = 24936; typedef BTreeNodeAllocator<int32_t, int8_t, btree::NoAggregated, MyTraits::INTERNAL_SLOTS, MyTraits::LEAF_SLOTS> NodeAllocator; diff --git a/vespalib/src/tests/datastore/array_store/array_store_test.cpp b/vespalib/src/tests/datastore/array_store/array_store_test.cpp index 8b368d05d90..1df03f6eb0a 100644 --- a/vespalib/src/tests/datastore/array_store/array_store_test.cpp +++ b/vespalib/src/tests/datastore/array_store/array_store_test.cpp @@ -215,7 +215,7 @@ TEST_P(NumberStoreTest, control_static_sizes) { EXPECT_EQ(104u, sizeof(NumberStoreTest::ArrayStoreType::SmallBufferType)); MemoryUsage usage = store.getMemoryUsage(); EXPECT_EQ(202116u, usage.allocatedBytes()); - EXPECT_EQ(197688u, usage.usedBytes()); + EXPECT_EQ(197656u, usage.usedBytes()); } TEST_P(NumberStoreTest, add_and_get_small_arrays_of_trivial_type) diff --git a/vespalib/src/tests/datastore/datastore/datastore_test.cpp b/vespalib/src/tests/datastore/datastore/datastore_test.cpp index f108c15c98c..9e27ed37dd3 100644 --- a/vespalib/src/tests/datastore/datastore/datastore_test.cpp +++ b/vespalib/src/tests/datastore/datastore/datastore_test.cpp @@ -475,7 +475,7 @@ TEST(DataStoreTest, require_that_memory_stats_are_calculated) TEST(DataStoreTest, require_that_memory_usage_is_calculated) { constexpr size_t BASE_ALLOCATED = 4228; - constexpr size_t BASE_USED = 292; + constexpr size_t BASE_USED = 284; MyStore s; MyRef r = s.addEntry(10); s.addEntry(20); @@ -494,7 +494,7 @@ TEST(DataStoreTest, require_that_memory_usage_is_calculated) TEST(DataStoreTest, require_that_we_can_disable_elemement_hold_list) { constexpr size_t BASE_ALLOCATED = 4228; - constexpr size_t BASE_USED = 292; + constexpr size_t BASE_USED = 284; MyStore s; MyRef r1 = s.addEntry(10); MyRef r2 = s.addEntry(20); @@ -667,9 +667,9 @@ TEST(DataStoreTest, can_reuse_active_buffer_as_primary_buffer) TEST(DataStoreTest, control_static_sizes) { EXPECT_EQ(88, sizeof(BufferTypeBase)); EXPECT_EQ(24, sizeof(FreeList)); - EXPECT_EQ(56, sizeof(BufferFreeList)); + EXPECT_EQ(48, sizeof(BufferFreeList)); EXPECT_EQ(1, sizeof(BufferState::State)); - EXPECT_EQ(128, sizeof(BufferState)); + EXPECT_EQ(120, sizeof(BufferState)); BufferState bs; EXPECT_EQ(0, bs.size()); } diff --git a/vespalib/src/tests/datastore/free_list/free_list_test.cpp b/vespalib/src/tests/datastore/free_list/free_list_test.cpp index 8532c7bc4c7..11c8bba4fe5 100644 --- a/vespalib/src/tests/datastore/free_list/free_list_test.cpp +++ b/vespalib/src/tests/datastore/free_list/free_list_test.cpp @@ -21,7 +21,6 @@ struct FreeListTest : public testing::Test { for (size_t i = 0; i < 3; ++i) { bufs.emplace_back(dead_entries); - bufs.back().set_array_size(array_size); } } void TearDown() override { 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 4f917079516..a09a7213bf5 100644 --- a/vespalib/src/tests/datastore/unique_store/unique_store_test.cpp +++ b/vespalib/src/tests/datastore/unique_store/unique_store_test.cpp @@ -465,13 +465,13 @@ TEST_F(DoubleTest, nan_is_handled) TEST_F(DoubleTest, control_memory_usage) { static constexpr size_t sizeof_deque = vespalib::datastore::DataStoreBase::sizeof_entry_ref_hold_list_deque; EXPECT_EQ(368u + sizeof_deque, sizeof(store)); - EXPECT_EQ(128u, sizeof(BufferState)); + EXPECT_EQ(120u, sizeof(BufferState)); EXPECT_EQ(28740u, store.get_values_memory_usage().allocatedBytes()); - EXPECT_EQ(24788u, store.get_values_memory_usage().usedBytes()); + EXPECT_EQ(24780u, store.get_values_memory_usage().usedBytes()); EXPECT_EQ(126952u, store.get_dictionary_memory_usage().allocatedBytes()); - EXPECT_EQ(25216u, store.get_dictionary_memory_usage().usedBytes()); + EXPECT_EQ(25200u, store.get_dictionary_memory_usage().usedBytes()); EXPECT_EQ(155692u, store.getMemoryUsage().allocatedBytes()); - EXPECT_EQ(50004, store.getMemoryUsage().usedBytes()); + EXPECT_EQ(49980u, store.getMemoryUsage().usedBytes()); } GTEST_MAIN_RUN_ALL_TESTS() diff --git a/vespalib/src/vespa/vespalib/datastore/buffer_free_list.cpp b/vespalib/src/vespa/vespalib/datastore/buffer_free_list.cpp index a44959c7811..851db0222c2 100644 --- a/vespalib/src/vespa/vespalib/datastore/buffer_free_list.cpp +++ b/vespalib/src/vespa/vespalib/datastore/buffer_free_list.cpp @@ -22,7 +22,6 @@ BufferFreeList::detach() BufferFreeList::BufferFreeList(std::atomic<EntryCount>& dead_entries) : _dead_entries(dead_entries), - _array_size(0), _free_list(), _free_refs() { diff --git a/vespalib/src/vespa/vespalib/datastore/buffer_free_list.h b/vespalib/src/vespa/vespalib/datastore/buffer_free_list.h index c570c8e3103..4348a41af04 100644 --- a/vespalib/src/vespa/vespalib/datastore/buffer_free_list.h +++ b/vespalib/src/vespa/vespalib/datastore/buffer_free_list.h @@ -20,7 +20,6 @@ private: using EntryRefArray = vespalib::Array<EntryRef>; std::atomic<EntryCount>& _dead_entries; - uint32_t _array_size; FreeList* _free_list; EntryRefArray _free_refs; @@ -37,10 +36,8 @@ public: void enable(FreeList& free_list); void disable(); - void set_array_size(uint32_t value) { _array_size = value; } bool enabled() const { return _free_list != nullptr; } bool empty() const { return _free_refs.empty(); } - uint32_t array_size() const { return _array_size; } void push_entry(EntryRef ref); EntryRef pop_entry(); }; diff --git a/vespalib/src/vespa/vespalib/datastore/bufferstate.cpp b/vespalib/src/vespa/vespalib/datastore/bufferstate.cpp index 10617289018..f312596d6f7 100644 --- a/vespalib/src/vespa/vespalib/datastore/bufferstate.cpp +++ b/vespalib/src/vespa/vespalib/datastore/bufferstate.cpp @@ -108,7 +108,6 @@ BufferState::on_active(uint32_t bufferId, uint32_t typeId, assert(typeId <= std::numeric_limits<uint16_t>::max()); _typeId = typeId; _arraySize = typeHandler->getArraySize(); - _free_list.set_array_size(_arraySize); _state.store(State::ACTIVE, std::memory_order_release); typeHandler->on_active(bufferId, &_stats.used_entries_ref(), &_stats.dead_entries_ref(), buffer.load(std::memory_order::relaxed)); @@ -145,7 +144,6 @@ BufferState::onFree(std::atomic<void*>& buffer) _state.store(State::FREE, std::memory_order_release); _typeHandler = nullptr; _arraySize = 0; - _free_list.set_array_size(_arraySize); assert(!_free_list.enabled()); assert(_free_list.empty()); _disable_entry_hold_list = false; diff --git a/vespalib/src/vespa/vespalib/datastore/free_list.h b/vespalib/src/vespa/vespalib/datastore/free_list.h index 20d4a6b96df..cd475af3104 100644 --- a/vespalib/src/vespa/vespalib/datastore/free_list.h +++ b/vespalib/src/vespa/vespalib/datastore/free_list.h @@ -30,7 +30,6 @@ public: bool empty() const { return _free_lists.empty(); } size_t size() const { return _free_lists.size(); } - uint32_t array_size() const { return _free_lists.back()->array_size(); } EntryRef pop_entry() { return _free_lists.back()->pop_entry(); } diff --git a/vespalib/src/vespa/vespalib/datastore/free_list_allocator.hpp b/vespalib/src/vespa/vespalib/datastore/free_list_allocator.hpp index 0bc26260127..4e69db08a3c 100644 --- a/vespalib/src/vespa/vespalib/datastore/free_list_allocator.hpp +++ b/vespalib/src/vespa/vespalib/datastore/free_list_allocator.hpp @@ -71,8 +71,9 @@ FreeListAllocator<EntryT, RefT, ReclaimerT>::allocArray(ConstArrayRef array) if (free_list.empty()) { return ParentType::allocArray(array); } - assert(free_list.array_size() == array.size()); RefT ref = free_list.pop_entry(); + auto& state = _store.getBufferState(ref.bufferId()); + assert(state.getArraySize() == array.size()); EntryT *buf = _store.template getEntryArray<EntryT>(ref, array.size()); for (size_t i = 0; i < array.size(); ++i) { *(buf + i) = array[i]; @@ -88,8 +89,9 @@ FreeListAllocator<EntryT, RefT, ReclaimerT>::allocArray() if (free_list.empty()) { return ParentType::allocArray(); } - auto size = free_list.array_size(); RefT ref = free_list.pop_entry(); + auto& state = _store.getBufferState(ref.bufferId()); + auto size = state.getArraySize(); EntryT *buf = _store.template getEntryArray<EntryT>(ref, size); return HandleType(ref, buf); } diff --git a/vespalib/src/vespa/vespalib/datastore/free_list_raw_allocator.hpp b/vespalib/src/vespa/vespalib/datastore/free_list_raw_allocator.hpp index c40e3db4dba..7680cd8a9a5 100644 --- a/vespalib/src/vespa/vespalib/datastore/free_list_raw_allocator.hpp +++ b/vespalib/src/vespa/vespalib/datastore/free_list_raw_allocator.hpp @@ -20,9 +20,10 @@ FreeListRawAllocator<EntryT, RefT>::alloc(size_t num_entries) if (free_list.empty()) { return ParentType::alloc(num_entries); } - auto array_size = free_list.array_size(); assert(num_entries == 1); RefT ref = free_list.pop_entry(); + auto& state = _store.getBufferState(ref.bufferId()); + auto array_size = state.getArraySize(); // We must scale the offset according to array size as it was divided when the entry ref was created. EntryT *entry = _store.template getEntryArray<EntryT>(ref, array_size); return HandleType(ref, entry); |