diff options
-rw-r--r-- | searchcore/src/vespa/searchcore/config/proton.def | 2 | ||||
-rw-r--r-- | searchlib/src/tests/common/rcuvector/rcuvector_test.cpp | 116 | ||||
-rw-r--r-- | searchlib/src/vespa/searchlib/common/rcuvector.h | 19 | ||||
-rw-r--r-- | searchlib/src/vespa/searchlib/common/rcuvector.hpp | 59 | ||||
-rw-r--r-- | vespalib/src/tests/array/array_test.cpp | 129 | ||||
-rw-r--r-- | vespalib/src/vespa/vespalib/util/array.h | 6 | ||||
-rw-r--r-- | vespalib/src/vespa/vespalib/util/array.hpp | 12 |
7 files changed, 189 insertions, 154 deletions
diff --git a/searchcore/src/vespa/searchcore/config/proton.def b/searchcore/src/vespa/searchcore/config/proton.def index a61e30ebd13..d63f67782ad 100644 --- a/searchcore/src/vespa/searchcore/config/proton.def +++ b/searchcore/src/vespa/searchcore/config/proton.def @@ -181,7 +181,7 @@ distribution.searchablecopies long default=1 restart grow.initial long default=1024 restart ## Grow factor in percent for any per document tables. -grow.factor int default=50 restart +grow.factor int default=20 restart ## Constant added when growing any per document tables. grow.add int default=1 restart diff --git a/searchlib/src/tests/common/rcuvector/rcuvector_test.cpp b/searchlib/src/tests/common/rcuvector/rcuvector_test.cpp index aa659b00357..f8a18d13192 100644 --- a/searchlib/src/tests/common/rcuvector/rcuvector_test.cpp +++ b/searchlib/src/tests/common/rcuvector/rcuvector_test.cpp @@ -5,31 +5,15 @@ LOG_SETUP("rcuvector_test"); #include <vespa/vespalib/testkit/testapp.h> #include <vespa/searchlib/common/rcuvector.h> -namespace search { -namespace attribute { - +using namespace search::attribute; +using search::MemoryUsage; +using vespalib::alloc::Alloc; using vespalib::GenerationHandler; using vespalib::GenerationHolder; using vespalib::GenerationHeldBase; -class Test : public vespalib::TestApp { -private: - bool assertUsage(const MemoryUsage & exp, const MemoryUsage & act); - void testGenerationHolder(); - void testBasic(); - void testResize(); - void testGenerationHandling(); - void testMemoryUsage(); - - void - testShrink(); - void testSmallExpand(); -public: - int Main(); -}; - bool -Test::assertUsage(const MemoryUsage & exp, const MemoryUsage & act) +assertUsage(const MemoryUsage & exp, const MemoryUsage & act) { bool retval = true; if (!EXPECT_EQUAL(exp.allocatedBytes(), act.allocatedBytes())) retval = false; @@ -39,8 +23,7 @@ Test::assertUsage(const MemoryUsage & exp, const MemoryUsage & act) return retval; } -void -Test::testGenerationHolder() +TEST("test generation holder") { typedef std::unique_ptr<int32_t> IntPtr; GenerationHolder gh; @@ -75,8 +58,7 @@ Test::testGenerationHolder() EXPECT_EQUAL(0u * sizeof(int32_t), gh.getHeldBytes()); } -void -Test::testBasic() +TEST("test basic") { { // insert RcuVector<int32_t> v(4, 0, 4); @@ -93,8 +75,7 @@ Test::testBasic() } } -void -Test::testResize() +TEST("test resize") { { // resize percent RcuVector<int32_t> v(2, 50, 0); @@ -162,8 +143,7 @@ Test::testResize() } } -void -Test::testGenerationHandling() +TEST("test generation handling") { RcuVector<int32_t> v(2, 0, 2); v.push_back(0); @@ -186,8 +166,7 @@ Test::testGenerationHandling() EXPECT_EQUAL(24u, v.getMemoryUsage().allocatedBytesOnHold()); } -void -Test::testMemoryUsage() +TEST("test memory usage") { RcuVector<int8_t> v(2, 0, 2); EXPECT_TRUE(assertUsage(MemoryUsage(2,0,0,0), v.getMemoryUsage())); @@ -205,12 +184,10 @@ Test::testMemoryUsage() EXPECT_TRUE(assertUsage(MemoryUsage(6,5,0,0), v.getMemoryUsage())); } - -void -Test::testShrink() +TEST("test shrink() with buffer copying") { GenerationHolder g; - RcuVectorBase<int8_t> v(g); + RcuVectorBase<int8_t> v(16, 100, 0, g); v.push_back(1); v.push_back(2); v.push_back(3); @@ -222,7 +199,7 @@ Test::testShrink() mu.incAllocatedBytesOnHold(g.getHeldBytes()); EXPECT_TRUE(assertUsage(MemoryUsage(16, 4, 0, 0), mu)); EXPECT_EQUAL(4u, v.size()); - EXPECT_TRUE(v.capacity() >= 4u); + EXPECT_EQUAL(16u, v.capacity()); EXPECT_EQUAL(1, v[0]); EXPECT_EQUAL(2, v[1]); EXPECT_EQUAL(3, v[2]); @@ -231,7 +208,7 @@ Test::testShrink() v.shrink(2); g.transferHoldLists(1); EXPECT_EQUAL(2u, v.size()); - EXPECT_EQUAL(2u, v.capacity()); + EXPECT_EQUAL(4u, v.capacity()); EXPECT_EQUAL(1, v[0]); EXPECT_EQUAL(2, v[1]); EXPECT_EQUAL(1, old[0]); @@ -241,11 +218,50 @@ Test::testShrink() EXPECT_EQUAL(2, v[1]); mu = v.getMemoryUsage(); mu.incAllocatedBytesOnHold(g.getHeldBytes()); - EXPECT_TRUE(assertUsage(MemoryUsage(2, 2, 0, 0), mu)); + EXPECT_TRUE(assertUsage(MemoryUsage(4, 2, 0, 0), mu)); +} + +struct ShrinkFixture { + GenerationHolder g; + RcuVectorBase<int> vec; + int *oldPtr; + ShrinkFixture() : g(), vec(4096, 50, 0, g, Alloc::allocMMap()), oldPtr() + { + for (size_t i = 0; i < 4000; ++i) { + vec.push_back(7); + } + EXPECT_EQUAL(4000u, vec.size()); + EXPECT_EQUAL(4096u, vec.capacity()); + assertEmptyHoldList(); + oldPtr = &vec[0]; + } + void assertOldEqualNewBuffer() { + EXPECT_EQUAL(oldPtr, &vec[0]); + } + void assertEmptyHoldList() { + EXPECT_EQUAL(0u, g.getHeldBytes()); + } +}; + +TEST_F("require that shrink() does not increase allocated memory", ShrinkFixture) +{ + f.vec.shrink(2732); + EXPECT_EQUAL(2732u, f.vec.size()); + EXPECT_EQUAL(4096u, f.vec.capacity()); + TEST_DO(f.assertOldEqualNewBuffer()); + TEST_DO(f.assertEmptyHoldList()); } -void -Test::testSmallExpand() +TEST_F("require that shrink() can shrink mmap allocation", ShrinkFixture) +{ + f.vec.shrink(2048); + EXPECT_EQUAL(2048, f.vec.size()); + EXPECT_EQUAL(3072, f.vec.capacity()); + TEST_DO(f.assertOldEqualNewBuffer()); + TEST_DO(f.assertEmptyHoldList()); +} + +TEST("test small expand") { GenerationHolder g; RcuVectorBase<int8_t> v(1, 50, 0, g); @@ -261,24 +277,4 @@ Test::testSmallExpand() g.trimHoldLists(2); } - -int -Test::Main() -{ - TEST_INIT("rcuvector_test"); - - testGenerationHolder(); - testBasic(); - testResize(); - testGenerationHandling(); - testMemoryUsage(); - testShrink(); - testSmallExpand(); - - TEST_DONE(); -} - -} -} - -TEST_APPHOOK(search::attribute::Test); +TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/searchlib/src/vespa/searchlib/common/rcuvector.h b/searchlib/src/vespa/searchlib/common/rcuvector.h index f9d8cf209c6..32dd1d99f7a 100644 --- a/searchlib/src/vespa/searchlib/common/rcuvector.h +++ b/searchlib/src/vespa/searchlib/common/rcuvector.h @@ -5,6 +5,7 @@ #include <vespa/vespalib/util/generationholder.h> #include <vespa/searchlib/util/memoryusage.h> #include <vespa/searchcommon/common/growstrategy.h> +#include <vespa/vespalib/util/alloc.h> #include <vespa/vespalib/util/array.h> namespace search { @@ -37,9 +38,10 @@ class RcuVectorBase "Value type must be trivially destructible"); protected: - typedef vespalib::Array<T> Array; - typedef vespalib::GenerationHandler::generation_t generation_t; - typedef vespalib::GenerationHolder GenerationHolder; + using Array = vespalib::Array<T>; + using Alloc = vespalib::alloc::Alloc; + using generation_t = vespalib::GenerationHandler::generation_t; + using GenerationHolder = vespalib::GenerationHolder; Array _data; size_t _growPercent; size_t _growDelta; @@ -61,7 +63,8 @@ protected: public: using ValueType = T; - RcuVectorBase(GenerationHolder &genHolder); + RcuVectorBase(GenerationHolder &genHolder, + const Alloc &initialAlloc = Alloc::alloc()); /** * Construct a new vector with the given initial capacity and grow @@ -70,9 +73,13 @@ public: * New capacity is calculated based on old capacity and grow parameters: * nc = oc + (oc * growPercent / 100) + growDelta. **/ - RcuVectorBase(size_t initialCapacity, size_t growPercent, size_t growDelta, GenerationHolder &genHolder); + RcuVectorBase(size_t initialCapacity, size_t growPercent, size_t growDelta, + GenerationHolder &genHolder, + const Alloc &initialAlloc = Alloc::alloc()); - RcuVectorBase(GrowStrategy growStrategy, GenerationHolder &genHolder); + RcuVectorBase(GrowStrategy growStrategy, + GenerationHolder &genHolder, + const Alloc &initialAlloc = Alloc::alloc()); ~RcuVectorBase(); diff --git a/searchlib/src/vespa/searchlib/common/rcuvector.hpp b/searchlib/src/vespa/searchlib/common/rcuvector.hpp index 920c0292435..6d1934a2148 100644 --- a/searchlib/src/vespa/searchlib/common/rcuvector.hpp +++ b/searchlib/src/vespa/searchlib/common/rcuvector.hpp @@ -49,12 +49,6 @@ RcuVectorBase<T>::reset() { } template <typename T> -RcuVectorBase<T>::RcuVectorBase(GrowStrategy growStrategy, GenerationHolder &genHolder) - : RcuVectorBase(growStrategy.getDocsInitialCapacity(), growStrategy.getDocsGrowPercent(), - growStrategy.getDocsGrowDelta(), genHolder) -{ } - -template <typename T> RcuVectorBase<T>::~RcuVectorBase() { } template <typename T> @@ -79,33 +73,36 @@ RcuVectorBase<T>::expandAndInsert(const T & v) _data.push_back(v); } - template <typename T> void RcuVectorBase<T>::shrink(size_t newSize) { - // TODO: Extend Array class to support more optimial shrink when - // backing store is memory mapped. assert(newSize <= _data.size()); - std::unique_ptr<Array> tmpData(new Array()); - tmpData->reserve(newSize); - tmpData->resize(newSize); - for (uint32_t i = 0; i < newSize; ++i) { - (*tmpData)[i] = _data[i]; + _data.resize(newSize); + size_t wantedCapacity = calcSize(newSize); + if (wantedCapacity >= _data.capacity()) { + return; + } + if (!_data.try_unreserve(wantedCapacity)) { + std::unique_ptr <Array> tmpData(new Array()); + tmpData->reserve(wantedCapacity); + tmpData->resize(newSize); + for (uint32_t i = 0; i < newSize; ++i) { + (*tmpData)[i] = _data[i]; + } + // Users of RCU vector must ensure that no readers use old size + // after swap. Attribute vectors uses _committedDocIdLimit for this. + tmpData->swap(_data); // atomic switch of underlying data + size_t holdSize = tmpData->capacity() * sizeof(T); + vespalib::GenerationHeldBase::UP hold(new RcuVectorHeld<Array>(holdSize, std::move(tmpData))); + _genHolder.hold(std::move(hold)); } - // Users of RCU vector must ensure that no readers use old size - // after swap. Attribute vectors uses _committedDocIdLimit for this. - tmpData->swap(_data); // atomic switch of underlying data - // Use capacity() instead of size() ? - size_t holdSize = tmpData->size() * sizeof(T); - vespalib::GenerationHeldBase::UP hold(new RcuVectorHeld<Array>(holdSize, std::move(tmpData))); - _genHolder.hold(std::move(hold)); } - template <typename T> -RcuVectorBase<T>::RcuVectorBase(GenerationHolder &genHolder) - : _data(), +RcuVectorBase<T>::RcuVectorBase(GenerationHolder &genHolder, + const Alloc &initialAlloc) + : _data(initialAlloc), _growPercent(100), _growDelta(0), _genHolder(genHolder) @@ -117,8 +114,9 @@ template <typename T> RcuVectorBase<T>::RcuVectorBase(size_t initialCapacity, size_t growPercent, size_t growDelta, - GenerationHolder &genHolder) - : _data(), + GenerationHolder &genHolder, + const Alloc &initialAlloc) + : _data(initialAlloc), _growPercent(growPercent), _growDelta(growDelta), _genHolder(genHolder) @@ -127,6 +125,15 @@ RcuVectorBase<T>::RcuVectorBase(size_t initialCapacity, } template <typename T> +RcuVectorBase<T>::RcuVectorBase(GrowStrategy growStrategy, + GenerationHolder &genHolder, + const Alloc &initialAlloc) + : RcuVectorBase(growStrategy.getDocsInitialCapacity(), growStrategy.getDocsGrowPercent(), + growStrategy.getDocsGrowDelta(), genHolder, initialAlloc) +{ +} + +template <typename T> MemoryUsage RcuVectorBase<T>::getMemoryUsage() const { diff --git a/vespalib/src/tests/array/array_test.cpp b/vespalib/src/tests/array/array_test.cpp index fb4a824ec00..13e46111bfb 100644 --- a/vespalib/src/tests/array/array_test.cpp +++ b/vespalib/src/tests/array/array_test.cpp @@ -7,22 +7,6 @@ using namespace vespalib; -class Test : public TestApp -{ -public: - int Main(); -private: - template <typename T> - void testArray(const T & a, const T & b); - void testComplicated(); - void testBeginEnd(); - void testThatOrganicGrowthIsBy2InNAndReserveResizeAreExact(); - template<class T> - void testBeginEnd(T & v); - void testMoveConstructor(); - void testMoveAssignment(); -}; - namespace vespalib { template <typename T> @@ -80,10 +64,38 @@ std::ostream & operator << (std::ostream & os, const Clever & clever) return os; } -int -Test::Main() +template <typename T> +void +testArray(const T & a, const T & b) +{ + Array<T> array; + + ASSERT_EQUAL(sizeof(array), 32u); + ASSERT_EQUAL(array.size(), 0u); + ASSERT_EQUAL(array.capacity(), 0u); + for(size_t i(0); i < 5; i++) { + array.push_back(a); + array.push_back(b); + for (size_t j(0); j <= i; j++) { + ASSERT_EQUAL(array[j*2 + 0], a); + ASSERT_EQUAL(array[j*2 + 1], b); + } + } + ASSERT_EQUAL(array.size(), 10u); + ASSERT_EQUAL(array.capacity(), 16u); + for (size_t i(array.size()), m(array.capacity()); i < m; i+=2) { + array.push_back(a); + array.push_back(b); + for (size_t j(0); j <= (i/2); j++) { + ASSERT_EQUAL(array[j*2 + 0], a); + ASSERT_EQUAL(array[j*2 + 1], b); + } + } + ASSERT_EQUAL(array.size(), array.capacity()); +} + +TEST("test basic array functionality") { - TEST_INIT("array_test"); testArray<int>(7, 9); testArray<vespalib::string>("7", "9"); const char * longS1 = "more than 48 bytes bytes that are needed to avoid the small string optimisation in vespalib::string"; @@ -105,15 +117,9 @@ Test::Main() size_t counter(0); testArray(Clever(&counter), Clever(&counter)); EXPECT_EQUAL(0ul, counter); - testComplicated(); - testBeginEnd(); - testThatOrganicGrowthIsBy2InNAndReserveResizeAreExact(); - testMoveConstructor(); - testMoveAssignment(); - TEST_DONE(); } -void Test::testThatOrganicGrowthIsBy2InNAndReserveResizeAreExact() +TEST("test that organic growth is by 2 in N and reserve resize are exact") { Array<char> c(256); EXPECT_EQUAL(256u, c.size()); @@ -149,38 +155,9 @@ void Test::testThatOrganicGrowthIsBy2InNAndReserveResizeAreExact() EXPECT_EQUAL(2048u, c.capacity()); } -template <typename T> -void Test::testArray(const T & a, const T & b) -{ - Array<T> array; - - ASSERT_EQUAL(sizeof(array), 32u); - ASSERT_EQUAL(array.size(), 0u); - ASSERT_EQUAL(array.capacity(), 0u); - for(size_t i(0); i < 5; i++) { - array.push_back(a); - array.push_back(b); - for (size_t j(0); j <= i; j++) { - ASSERT_EQUAL(array[j*2 + 0], a); - ASSERT_EQUAL(array[j*2 + 1], b); - } - } - ASSERT_EQUAL(array.size(), 10u); - ASSERT_EQUAL(array.capacity(), 16u); - for (size_t i(array.size()), m(array.capacity()); i < m; i+=2) { - array.push_back(a); - array.push_back(b); - for (size_t j(0); j <= (i/2); j++) { - ASSERT_EQUAL(array[j*2 + 0], a); - ASSERT_EQUAL(array[j*2 + 1], b); - } - } - ASSERT_EQUAL(array.size(), array.capacity()); -} - size_t Clever::_global = 0; -void Test::testComplicated() +TEST("test complicated") { volatile size_t counter(0); { @@ -226,7 +203,8 @@ void Test::testComplicated() } template<class T> -void Test::testBeginEnd(T & v) +void +testBeginEnd(T & v) { EXPECT_EQUAL(0u, v.end() - v.begin()); EXPECT_EQUAL(0u, v.rend() - v.rbegin()); @@ -288,7 +266,7 @@ void Test::testBeginEnd(T & v) EXPECT_EQUAL(3u, v.rend() - v.rbegin()); } -void Test::testBeginEnd() +TEST("test begin end") { std::vector<size_t> v; Array<size_t> a; @@ -296,7 +274,7 @@ void Test::testBeginEnd() testBeginEnd(a); } -void Test::testMoveConstructor() +TEST("test move constructor") { Array<size_t> orig; orig.push_back(42); @@ -318,7 +296,7 @@ void Test::testMoveConstructor() } } -void Test::testMoveAssignment() +TEST("test move assignment") { Array<size_t> orig; orig.push_back(44); @@ -342,4 +320,33 @@ void Test::testMoveAssignment() } } -TEST_APPHOOK(Test) +struct UnreserveFixture { + Array<int> arr; + UnreserveFixture() : arr(1025, 7, alloc::Alloc::allocMMap(0)) + { + EXPECT_EQUAL(1025u, arr.size()); + EXPECT_EQUAL(2048u, arr.capacity()); + } +}; + +TEST_F("require that try_unreserve() fails if wanted capacity >= current capacity", UnreserveFixture) +{ + EXPECT_FALSE(f.arr.try_unreserve(2048)); +} + +TEST_F("require that try_unreserve() fails if wanted capacity < current size", UnreserveFixture) +{ + EXPECT_FALSE(f.arr.try_unreserve(1024)); +} + +TEST_F("require that try_unreserve() succeedes if mmap can be shrinked", UnreserveFixture) +{ + int *oldPtr = &f.arr[0]; + f.arr.resize(512); + EXPECT_TRUE(f.arr.try_unreserve(1023)); + EXPECT_EQUAL(1024u, f.arr.capacity()); + int *newPtr = &f.arr[0]; + EXPECT_EQUAL(oldPtr, newPtr); +} + +TEST_MAIN() { TEST_RUN_ALL(); } diff --git a/vespalib/src/vespa/vespalib/util/array.h b/vespalib/src/vespa/vespalib/util/array.h index 269bf67bc26..4ca09bd093a 100644 --- a/vespalib/src/vespa/vespalib/util/array.h +++ b/vespalib/src/vespa/vespalib/util/array.h @@ -106,6 +106,12 @@ public: void resize(size_t n); void assign(const_iterator begin_, const_iterator end_); void reserve(size_t n); + /** + * Try to unreserve memory from the underlying memory buffer inplace down to the given limit. + * The existing memory buffer is unmodified up to the new size (no copying occurs). + * Returns true if it was possible to unreserve memory, false if not. + */ + bool try_unreserve(size_t n); void push_back(const T & v) { std::_Construct(push_back(), v); } iterator push_back() { extend(size()+1); return array(_sz++); } iterator push_back_fast() { return array(_sz++); } diff --git a/vespalib/src/vespa/vespalib/util/array.hpp b/vespalib/src/vespa/vespalib/util/array.hpp index dbdda73ad66..ec418d7d566 100644 --- a/vespalib/src/vespa/vespalib/util/array.hpp +++ b/vespalib/src/vespa/vespalib/util/array.hpp @@ -94,6 +94,18 @@ void Array<T>::reserve(size_t n) { } template <typename T> +bool Array<T>::try_unreserve(size_t n) +{ + if (n >= capacity()) { + return false; + } + if (n < size()) { + return false; + } + return _array.resize_inplace(n * sizeof(T)); +} + +template <typename T> void Array<T>::resize(size_t n) { if (n > capacity()) { |