Merge pull request #27553 from vespa-engine/toregge/add-max-buffer-size-parameter-to-array-store-dynamic-type-mapper

Add max buffer size parameter to array store dynamic type mapper.

9 files changed, 51 insertions, 32 deletions

diff --git a/searchlib/src/tests/attribute/multi_value_mapping/multi_value_mapping_test.cpp b/searchlib/src/tests/attribute/multi_value_mapping/multi_value_mapping_test.cpp
index 4b01808e855..e3e4f391cc4 100644
--- a/searchlib/src/tests/attribute/multi_value_mapping/multi_value_mapping_test.cpp
+++ b/searchlib/src/tests/attribute/multi_value_mapping/multi_value_mapping_test.cpp
@@ -95,7 +95,7 @@ public:
         ArrayStoreConfig config(max_array_store_type_id,
                                 ArrayStoreConfig::AllocSpec(0, RefType::offsetSize(), 8_Ki, ALLOC_GROW_FACTOR));
         config.enable_free_lists(enable_free_lists);
-        _mvMapping = std::make_unique<MvMapping>(config, vespalib::GrowStrategy(), std::make_unique<MemoryAllocatorObserver>(_stats));
+        _mvMapping = std::make_unique<MvMapping>(config, ArrayStoreConfig::default_max_buffer_size, vespalib::GrowStrategy(), std::make_unique<MemoryAllocatorObserver>(_stats));
         _attr = std::make_unique<AttributeType>(*_mvMapping);
         _maxSmallArraySize = _mvMapping->get_mapper().get_array_size(max_array_store_type_id);
     }
@@ -103,7 +103,7 @@ public:
         ArrayStoreConfig config(max_array_store_type_id,
                                 ArrayStoreConfig::AllocSpec(min_entries, max_entries, num_entries_for_new_buffer, ALLOC_GROW_FACTOR));
         config.enable_free_lists(enable_free_lists);
-        _mvMapping = std::make_unique<MvMapping>(config, vespalib::GrowStrategy(), std::make_unique<MemoryAllocatorObserver>(_stats));
+        _mvMapping = std::make_unique<MvMapping>(config, ArrayStoreConfig::default_max_buffer_size, vespalib::GrowStrategy(), std::make_unique<MemoryAllocatorObserver>(_stats));
         _attr = std::make_unique<AttributeType>(*_mvMapping);
         _maxSmallArraySize = _mvMapping->get_mapper().get_array_size(max_array_store_type_id);
     }
diff --git a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.h b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.h
index 25ca7729a32..ced076dc632 100644
--- a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.h
+++ b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.h
@@ -36,6 +36,7 @@ public:
     MultiValueMapping(const MultiValueMapping &) = delete;
     MultiValueMapping & operator = (const MultiValueMapping &) = delete;
     MultiValueMapping(const vespalib::datastore::ArrayStoreConfig &storeCfg,
+                      size_t max_buffer_size,
                       const vespalib::GrowStrategy &gs,
                       std::shared_ptr<vespalib::alloc::MemoryAllocator> memory_allocator);
     ~MultiValueMapping() override;
diff --git a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.hpp b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.hpp
index 3c9a52f2e5c..64c4777ffda 100644
--- a/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.hpp
+++ b/searchlib/src/vespa/searchlib/attribute/multi_value_mapping.hpp
@@ -9,10 +9,11 @@ namespace search::attribute {
 
 template <typename ElemT, typename RefT>
 MultiValueMapping<ElemT,RefT>::MultiValueMapping(const vespalib::datastore::ArrayStoreConfig &storeCfg,
+                                                 size_t max_buffer_size,
                                                   const vespalib::GrowStrategy &gs,
                                                   std::shared_ptr<vespalib::alloc::MemoryAllocator> memory_allocator)
   : MultiValueMappingBase(gs, ArrayStore::getGenerationHolderLocation(_store), memory_allocator),
-    _store(storeCfg, std::move(memory_allocator), ArrayStoreTypeMapper(storeCfg.max_type_id(), array_store_grow_factor))
+    _store(storeCfg, std::move(memory_allocator), ArrayStoreTypeMapper(storeCfg.max_type_id(), array_store_grow_factor, max_buffer_size))
 {
 }
 
@@ -73,7 +74,7 @@ MultiValueMapping<ElemT, RefT>::optimizedConfigForHugePage(size_t max_type_id,
                                                            float allocGrowFactor,
                                                            bool enable_free_lists)
 {
-    ArrayStoreTypeMapper mapper(max_type_id, array_store_grow_factor);
+    ArrayStoreTypeMapper mapper(max_type_id, array_store_grow_factor, max_buffer_size);
     auto result = ArrayStore::optimizedConfigForHugePage(max_type_id, mapper, hugePageSize, smallPageSize, max_buffer_size, min_num_entries_for_new_buffer, allocGrowFactor);
     result.enable_free_lists(enable_free_lists);
     return result;
diff --git a/searchlib/src/vespa/searchlib/attribute/multivalueattribute.hpp b/searchlib/src/vespa/searchlib/attribute/multivalueattribute.hpp
index 3cf75b450af..56c6d010582 100644
--- a/searchlib/src/vespa/searchlib/attribute/multivalueattribute.hpp
+++ b/searchlib/src/vespa/searchlib/attribute/multivalueattribute.hpp
@@ -12,6 +12,8 @@
 #include <vespa/vespalib/util/memory_allocator.h>
 #include <vespa/vespalib/util/stash.h>
 
+using vespalib::datastore::ArrayStoreConfig;
+
 namespace search {
 
 namespace multivalueattribute {
@@ -28,10 +30,11 @@ MultiValueAttribute(const vespalib::string &baseFileName,
       _mvMapping(MultiValueMapping::optimizedConfigForHugePage(MultiValueMapping::array_store_max_type_id,
                                                                vespalib::alloc::MemoryAllocator::HUGEPAGE_SIZE,
                                                                vespalib::alloc::MemoryAllocator::PAGE_SIZE,
-                                                               vespalib::datastore::ArrayStoreConfig::default_max_buffer_size,
+                                                               ArrayStoreConfig::default_max_buffer_size,
                                                                8 * 1024,
                                                                cfg.getGrowStrategy().getMultiValueAllocGrowFactor(),
                                                                multivalueattribute::enable_free_lists),
+                 ArrayStoreConfig::default_max_buffer_size,
                  cfg.getGrowStrategy(), this->get_memory_allocator())
 {
 }
diff --git a/searchlib/src/vespa/searchlib/attribute/raw_buffer_store.cpp b/searchlib/src/vespa/searchlib/attribute/raw_buffer_store.cpp
index 00c195b9eb7..0c6dd9c75a8 100644
--- a/searchlib/src/vespa/searchlib/attribute/raw_buffer_store.cpp
+++ b/searchlib/src/vespa/searchlib/attribute/raw_buffer_store.cpp
@@ -5,6 +5,7 @@
 #include <cassert>
 
 using vespalib::alloc::MemoryAllocator;
+using vespalib::datastore::ArrayStoreConfig;
 using vespalib::datastore::EntryRef;
 
 namespace {
@@ -17,12 +18,12 @@ namespace search::attribute {
 
 RawBufferStore::RawBufferStore(std::shared_ptr<vespalib::alloc::MemoryAllocator> allocator, uint32_t max_small_buffer_type_id, double grow_factor)
     : _array_store(ArrayStoreType::optimizedConfigForHugePage(max_small_buffer_type_id,
-                                                              TypeMapper(max_small_buffer_type_id, grow_factor),
+                                                              TypeMapper(max_small_buffer_type_id, grow_factor, ArrayStoreConfig::default_max_buffer_size),
                                                               MemoryAllocator::HUGEPAGE_SIZE,
                                                               MemoryAllocator::PAGE_SIZE,
-                                                              vespalib::datastore::ArrayStoreConfig::default_max_buffer_size,
+                                                              ArrayStoreConfig::default_max_buffer_size,
                                                               8_Ki, ALLOC_GROW_FACTOR),
-                   std::move(allocator), TypeMapper(max_small_buffer_type_id, grow_factor))
+                   std::move(allocator), TypeMapper(max_small_buffer_type_id, grow_factor, ArrayStoreConfig::default_max_buffer_size))
 {
 }
 
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 797dc97c963..6e433e48d88 100644
--- a/vespalib/src/tests/datastore/array_store/array_store_test.cpp
+++ b/vespalib/src/tests/datastore/array_store/array_store_test.cpp
@@ -32,7 +32,7 @@ constexpr float ALLOC_GROW_FACTOR = 0.2;
 template <typename ElemT>
 class MyArrayStoreSimpleTypeMapper : public ArrayStoreSimpleTypeMapper<ElemT> {
 public:
-    MyArrayStoreSimpleTypeMapper(uint32_t, double)
+    MyArrayStoreSimpleTypeMapper(uint32_t, double, size_t)
         : ArrayStoreSimpleTypeMapper<ElemT>()
     {
     }
@@ -62,7 +62,7 @@ struct ArrayStoreTest : public TestT
     bool add_using_allocate;
     double type_mapper_grow_factor;
     ArrayStoreTest(uint32_t max_type_id = 3, bool enable_free_lists = true, bool add_using_allocate_in = false, double type_mapper_grow_factor_in = 2.0)
-        : type_mapper(max_type_id, type_mapper_grow_factor_in),
+        : type_mapper(max_type_id, type_mapper_grow_factor_in, ArrayStoreConfig::default_max_buffer_size),
           store(ArrayStoreConfig(max_type_id,
                                  ArrayStoreConfig::AllocSpec(16, RefT::offsetSize(), 8_Ki,
                                                              ALLOC_GROW_FACTOR)).enable_free_lists(enable_free_lists),
@@ -74,7 +74,7 @@ struct ArrayStoreTest : public TestT
           type_mapper_grow_factor(type_mapper_grow_factor_in)
     {}
     explicit ArrayStoreTest(const ArrayStoreConfig &storeCfg)
-        : type_mapper(storeCfg.max_type_id(), 2.0),
+        : type_mapper(storeCfg.max_type_id(), 2.0, ArrayStoreConfig::default_max_buffer_size),
           store(storeCfg, std::make_unique<MemoryAllocatorObserver>(stats), TypeMapperType(type_mapper)),
           refStore(),
           generation(1),
diff --git a/vespalib/src/tests/datastore/array_store_dynamic_type_mapper/array_store_dynamic_type_mapper_test.cpp b/vespalib/src/tests/datastore/array_store_dynamic_type_mapper/array_store_dynamic_type_mapper_test.cpp
index 3ab38a4113d..f53f5a8ff22 100644
--- a/vespalib/src/tests/datastore/array_store_dynamic_type_mapper/array_store_dynamic_type_mapper_test.cpp
+++ b/vespalib/src/tests/datastore/array_store_dynamic_type_mapper/array_store_dynamic_type_mapper_test.cpp
@@ -2,10 +2,18 @@
 
 #include <vespa/vespalib/datastore/array_store_dynamic_type_mapper.h>
 #include <vespa/vespalib/gtest/gtest.h>
+#include <vespa/vespalib/util/size_literals.h>
+#include <limits>
 
 using vespalib::datastore::ArrayStoreDynamicTypeMapper;
 
+namespace {
+
 constexpr double default_grow_factor = 1.03;
+constexpr size_t default_max_buffer_size = 256_Mi;
+constexpr size_t small_max_buffer_size = 256_Ki;
+constexpr size_t max_max_buffer_size = std::numeric_limits<uint32_t>::max();
+}
 
 template <typename ElemT>
 class TestBase : public testing::Test
@@ -19,13 +27,13 @@ protected:
     std::vector<size_t> get_large_array_sizes(uint32_t num_large_arrays);
     void select_type_ids(std::vector<size_t> array_sizes);
     void setup_mapper(uint32_t max_buffer_type_id, double grow_factor);
-    static uint32_t calc_max_buffer_type_id(double grow_factor);
+    static uint32_t calc_max_buffer_type_id(double grow_factor, size_t max_buffer_size = default_max_buffer_size);
 };
 
 template <typename ElemT>
 TestBase<ElemT>::TestBase()
     : testing::Test(),
-      _mapper(5, default_grow_factor)
+      _mapper(5, default_grow_factor, default_max_buffer_size)
 {
 }
 
@@ -36,7 +44,7 @@ template <typename ElemT>
 void
 TestBase<ElemT>::setup_mapper(uint32_t max_buffer_type_id, double grow_factor)
 {
-    _mapper = ArrayStoreDynamicTypeMapper<ElemT>(max_buffer_type_id, grow_factor);
+    _mapper = ArrayStoreDynamicTypeMapper<ElemT>(max_buffer_type_id, grow_factor, default_max_buffer_size);
 }
 
 template <typename ElemT>
@@ -108,9 +116,9 @@ TestBase<ElemT>::select_type_ids(std::vector<size_t> array_sizes)
 
 template <typename ElemT>
 uint32_t
-TestBase<ElemT>::calc_max_buffer_type_id(double grow_factor)
+TestBase<ElemT>::calc_max_buffer_type_id(double grow_factor, size_t max_buffer_size)
 {
-    ArrayStoreDynamicTypeMapper<ElemT> mapper(1000, grow_factor);
+    ArrayStoreDynamicTypeMapper<ElemT> mapper(1000, grow_factor, max_buffer_size);
     return mapper.get_max_type_id(1000);
 }
 
@@ -139,11 +147,13 @@ TEST_F(ArrayStoreDynamicTypeMapperCharTest, large_arrays_grows_exponentially)
 
 TEST_F(ArrayStoreDynamicTypeMapperCharTest, avoid_entry_size_overflow)
 {
-    EXPECT_EQ(32, calc_max_buffer_type_id(2.0));
-    EXPECT_EQ(410, calc_max_buffer_type_id(1.05));
-    EXPECT_EQ(507, calc_max_buffer_type_id(1.04));
-    EXPECT_EQ(661, calc_max_buffer_type_id(1.03));
-    EXPECT_EQ(968, calc_max_buffer_type_id(1.02));
+    EXPECT_EQ(29, calc_max_buffer_type_id(2.0));
+    EXPECT_EQ(367, calc_max_buffer_type_id(1.05));
+    EXPECT_EQ(454, calc_max_buffer_type_id(1.04));
+    EXPECT_EQ(591, calc_max_buffer_type_id(1.03));
+    EXPECT_EQ(357, calc_max_buffer_type_id(1.03, small_max_buffer_size));
+    EXPECT_EQ(661, calc_max_buffer_type_id(1.03, max_max_buffer_size));
+    EXPECT_EQ(863, calc_max_buffer_type_id(1.02));
 }
 
 using ArrayStoreDynamicTypeMapperInt32Test = TestBase<int32_t>;
@@ -159,11 +169,13 @@ TEST_F(ArrayStoreDynamicTypeMapperInt32Test, array_sizes_are_calculated)
 
 TEST_F(ArrayStoreDynamicTypeMapperInt32Test, avoid_entry_size_overflow)
 {
-    EXPECT_EQ(30, calc_max_buffer_type_id(2.0));
-    EXPECT_EQ(379, calc_max_buffer_type_id(1.05));
-    EXPECT_EQ(462, calc_max_buffer_type_id(1.04));
-    EXPECT_EQ(596, calc_max_buffer_type_id(1.03));
-    EXPECT_EQ(849, calc_max_buffer_type_id(1.02));
+    EXPECT_EQ(27, calc_max_buffer_type_id(2.0));
+    EXPECT_EQ(337, calc_max_buffer_type_id(1.05));
+    EXPECT_EQ(409, calc_max_buffer_type_id(1.04));
+    EXPECT_EQ(525, calc_max_buffer_type_id(1.03));
+    EXPECT_EQ(291, calc_max_buffer_type_id(1.03, small_max_buffer_size));
+    EXPECT_EQ(596, calc_max_buffer_type_id(1.03, max_max_buffer_size));
+    EXPECT_EQ(744, calc_max_buffer_type_id(1.02));
 }
 
 GTEST_MAIN_RUN_ALL_TESTS()
diff --git a/vespalib/src/vespa/vespalib/datastore/array_store_dynamic_type_mapper.h b/vespalib/src/vespa/vespalib/datastore/array_store_dynamic_type_mapper.h
index 73c998e82a5..6797b2a79b4 100644
--- a/vespalib/src/vespa/vespalib/datastore/array_store_dynamic_type_mapper.h
+++ b/vespalib/src/vespa/vespalib/datastore/array_store_dynamic_type_mapper.h
@@ -36,9 +36,9 @@ public:
     using LargeBufferType = vespalib::datastore::LargeArrayBufferType<ElemT>;
 
     ArrayStoreDynamicTypeMapper();
-    ArrayStoreDynamicTypeMapper(uint32_t max_buffer_type_id, double grow_factor);
+    ArrayStoreDynamicTypeMapper(uint32_t max_buffer_type_id, double grow_factor, size_t max_buffer_size);
     ~ArrayStoreDynamicTypeMapper();
-    void setup_array_sizes(uint32_t max_buffer_type_id, double grow_factor);
+    void setup_array_sizes(uint32_t max_buffer_type_id, double grow_factor, size_t max_buffer_size);
     size_t get_entry_size(uint32_t type_id) const;
     bool is_dynamic_buffer(uint32_t type_id) const noexcept { return type_id > _max_static_array_buffer_type_id; }
     uint32_t count_dynamic_buffer_types(uint32_t max_type_id) const noexcept { return (max_type_id > _max_static_array_buffer_type_id) ? (max_type_id - _max_static_array_buffer_type_id) : 0u; }
diff --git a/vespalib/src/vespa/vespalib/datastore/array_store_dynamic_type_mapper.hpp b/vespalib/src/vespa/vespalib/datastore/array_store_dynamic_type_mapper.hpp
index e74cd92e6aa..48de5cf5332 100644
--- a/vespalib/src/vespa/vespalib/datastore/array_store_dynamic_type_mapper.hpp
+++ b/vespalib/src/vespa/vespalib/datastore/array_store_dynamic_type_mapper.hpp
@@ -18,16 +18,16 @@ ArrayStoreDynamicTypeMapper<ElemT>::ArrayStoreDynamicTypeMapper()
 }
 
 template <typename ElemT>
-ArrayStoreDynamicTypeMapper<ElemT>::ArrayStoreDynamicTypeMapper(uint32_t max_buffer_type_id, double grow_factor)
+ArrayStoreDynamicTypeMapper<ElemT>::ArrayStoreDynamicTypeMapper(uint32_t max_buffer_type_id, double grow_factor, size_t max_buffer_size)
     : ArrayStoreTypeMapper(),
       _max_static_array_buffer_type_id(0)
 {
-    setup_array_sizes(max_buffer_type_id, grow_factor);
+    setup_array_sizes(max_buffer_type_id, grow_factor, max_buffer_size);
 }
 
 template <typename ElemT>
 void
-ArrayStoreDynamicTypeMapper<ElemT>::setup_array_sizes(uint32_t max_buffer_type_id, double grow_factor)
+ArrayStoreDynamicTypeMapper<ElemT>::setup_array_sizes(uint32_t max_buffer_type_id, double grow_factor, size_t max_buffer_size)
 {
     _array_sizes.clear();
     _array_sizes.reserve(max_buffer_type_id + 1);
@@ -49,7 +49,8 @@ ArrayStoreDynamicTypeMapper<ElemT>::setup_array_sizes(uint32_t max_buffer_type_i
                 entry_size = array_size * sizeof(ElemT);
             }
         }
-        if (entry_size > std::numeric_limits<uint32_t>::max()) {
+        if (entry_size > std::numeric_limits<uint32_t>::max() ||
+            entry_size >= 2 * max_buffer_size) {
             break;
         }
         _array_sizes.emplace_back(array_size);