From 208a7c1db7a7a67aa71fae59ca0cd5615b688b6e Mon Sep 17 00:00:00 2001 From: Håvard Pettersen Date: Mon, 9 May 2022 14:14:51 +0000 Subject: move functions to more appropriate classes preparing to make GrowableBitVector an atomic switch between bitvectors rather than a bitvector itself (to avoid overwriting its own state while being visible to other threads). --- .../proton/feedoperation/lidvectorcontext.cpp | 4 ++- .../document_store_visitor_test.cpp | 10 ++++---- .../vespa/searchlib/attribute/flagattribute.cpp | 10 ++++---- .../src/vespa/searchlib/attribute/flagattribute.h | 9 ++++--- .../vespa/searchlib/common/allocatedbitvector.cpp | 30 ---------------------- .../vespa/searchlib/common/allocatedbitvector.h | 8 +++--- searchlib/src/vespa/searchlib/common/bitvector.cpp | 21 +-------------- searchlib/src/vespa/searchlib/common/bitvector.h | 11 +++----- .../vespa/searchlib/common/growablebitvector.cpp | 27 +++++++++++++++++++ .../src/vespa/searchlib/common/growablebitvector.h | 2 ++ 10 files changed, 55 insertions(+), 77 deletions(-) diff --git a/searchcore/src/vespa/searchcore/proton/feedoperation/lidvectorcontext.cpp b/searchcore/src/vespa/searchcore/proton/feedoperation/lidvectorcontext.cpp index a3a180a74df..3aecbc3ca0d 100644 --- a/searchcore/src/vespa/searchcore/proton/feedoperation/lidvectorcontext.cpp +++ b/searchcore/src/vespa/searchcore/proton/feedoperation/lidvectorcontext.cpp @@ -2,6 +2,7 @@ #include "lidvectorcontext.h" #include +#include #include #include @@ -9,6 +10,7 @@ LOG_SETUP(".proton.feedoperation.lidvectorcontext"); using search::BitVector; +using search::AllocatedBitVector; namespace proton { @@ -73,7 +75,7 @@ LidVectorContext::deserialize(vespalib::nbostream &is) LOG(debug, "deserialize: format = %d", format); // Use of bitvector when > 1/32 of docs if (format == BITVECTOR) { - BitVector::UP bitVector = BitVector::create(_docIdLimit); + auto bitVector = std::make_unique(_docIdLimit); is >> *bitVector; uint32_t sz(bitVector->size()); assert(sz == _docIdLimit); diff --git a/searchlib/src/tests/docstore/document_store_visitor/document_store_visitor_test.cpp b/searchlib/src/tests/docstore/document_store_visitor/document_store_visitor_test.cpp index e811e68f4b5..0658287f29b 100644 --- a/searchlib/src/tests/docstore/document_store_visitor/document_store_visitor_test.cpp +++ b/searchlib/src/tests/docstore/document_store_visitor/document_store_visitor_test.cpp @@ -5,7 +5,7 @@ #include #include #include -#include +#include #include #include #include @@ -95,7 +95,7 @@ public: uint32_t _visitCount; uint32_t _visitRmCount; uint32_t _docIdLimit; - BitVector::UP _valid; + std::unique_ptr _valid; bool _before; MyVisitorBase(DocumentTypeRepo &repo, uint32_t docIdLimit, bool before); @@ -106,7 +106,7 @@ MyVisitorBase::MyVisitorBase(DocumentTypeRepo &repo, uint32_t docIdLimit, bool b _visitCount(0u), _visitRmCount(0u), _docIdLimit(docIdLimit), - _valid(BitVector::create(docIdLimit)), + _valid(std::make_unique(docIdLimit)), _before(before) { } @@ -216,7 +216,7 @@ struct Fixture std::unique_ptr _store; uint64_t _syncToken; uint32_t _docIdLimit; - BitVector::UP _valid; + std::unique_ptr _valid; Fixture(); ~Fixture(); @@ -246,7 +246,7 @@ Fixture::Fixture() _store(), _syncToken(0u), _docIdLimit(0u), - _valid(BitVector::create(0u)) + _valid(std::make_unique(0u)) { rmdir(); mkdir(); diff --git a/searchlib/src/vespa/searchlib/attribute/flagattribute.cpp b/searchlib/src/vespa/searchlib/attribute/flagattribute.cpp index 8d137530dd0..d6aa136e15f 100644 --- a/searchlib/src/vespa/searchlib/attribute/flagattribute.cpp +++ b/searchlib/src/vespa/searchlib/attribute/flagattribute.cpp @@ -122,7 +122,7 @@ void FlagAttributeT::setNewValues(DocId doc, const std::vector= this->getNumDocs()); - _bitVectorStore[offset] = BitVector::create(_bitVectorSize); + _bitVectorStore[offset] = std::make_shared(_bitVectorSize, _bitVectorSize, _bitVectorHolder); _bitVectors[offset] = _bitVectorStore[offset].get(); bv = _bitVectors[offset]; ensureGuardBit(*bv); @@ -139,7 +139,7 @@ FlagAttributeT::setNewBVValue(DocId doc, multivalue::ValueType_t= this->getNumDocs()); - _bitVectorStore[offset] = BitVector::create(_bitVectorSize); + _bitVectorStore[offset] = std::make_shared(_bitVectorSize, _bitVectorSize, _bitVectorHolder); _bitVectors[offset] = _bitVectorStore[offset].get(); bv = _bitVectors[offset]; ensureGuardBit(*bv); @@ -210,11 +210,11 @@ FlagAttributeT::resizeBitVectors(uint32_t neededSize) { const GrowStrategy & gs = this->getConfig().getGrowStrategy(); uint32_t newSize = neededSize + (neededSize * gs.getDocsGrowFactor()) + gs.getDocsGrowDelta(); - for (BitVector * bv : _bitVectors) { + for (size_t i(0), m(_bitVectors.size()); i < m; i++) { + BitVector *bv = _bitVectors[i]; if (bv != nullptr) { - vespalib::GenerationHeldBase::UP hold(bv->grow(newSize)); + _bitVectorStore[i]->extend(newSize); ensureGuardBit(*bv); - _bitVectorHolder.hold(std::move(hold)); } } _bitVectorSize = newSize; diff --git a/searchlib/src/vespa/searchlib/attribute/flagattribute.h b/searchlib/src/vespa/searchlib/attribute/flagattribute.h index f88f46bf974..c4f4980e372 100644 --- a/searchlib/src/vespa/searchlib/attribute/flagattribute.h +++ b/searchlib/src/vespa/searchlib/attribute/flagattribute.h @@ -3,6 +3,7 @@ #include "multinumericattribute.h" #include "multi_numeric_search_context.h" +#include namespace search { @@ -34,10 +35,10 @@ private: void removeOldGenerations(vespalib::GenerationHandler::generation_t firstUsed) override; uint32_t getOffset(int8_t value) const { return value + 128; } - vespalib::GenerationHolder _bitVectorHolder; - std::vector > _bitVectorStore; - std::vector _bitVectors; - uint32_t _bitVectorSize; + vespalib::GenerationHolder _bitVectorHolder; + std::vector > _bitVectorStore; + std::vector _bitVectors; + uint32_t _bitVectorSize; }; typedef FlagAttributeT FlagAttribute; diff --git a/searchlib/src/vespa/searchlib/common/allocatedbitvector.cpp b/searchlib/src/vespa/searchlib/common/allocatedbitvector.cpp index ab56317db6f..f37968c70a0 100644 --- a/searchlib/src/vespa/searchlib/common/allocatedbitvector.cpp +++ b/searchlib/src/vespa/searchlib/common/allocatedbitvector.cpp @@ -6,10 +6,6 @@ namespace search { -using vespalib::GenerationHeldBase; -using vespalib::GenerationHeldAlloc; -using vespalib::GenerationHolder; - namespace { size_t computeCapacity(size_t capacity, size_t allocatedBytes) { @@ -131,30 +127,4 @@ AllocatedBitVector::operator=(const BitVector & rhs) return *this; } -GenerationHeldBase::UP -AllocatedBitVector::grow(Index newSize, Index newCapacity) -{ - assert(newCapacity >= newSize); - GenerationHeldBase::UP ret; - if (newCapacity != capacity()) { - AllocatedBitVector tbv(newSize, newCapacity, _alloc.get(), size(), &_alloc); - if (newSize > size()) { - tbv.clearBitAndMaintainCount(size()); // Clear old guard bit. - } - ret = std::make_unique>(_alloc); - swap(tbv); - } else { - if (newSize > size()) { - Range clearRange(size(), newSize); - setSize(newSize); - clearIntervalNoInvalidation(clearRange); - } else { - clearIntervalNoInvalidation(Range(newSize, size())); - setSize(newSize); - updateCount(); - } - } - return ret; -} - } // namespace search diff --git a/searchlib/src/vespa/searchlib/common/allocatedbitvector.h b/searchlib/src/vespa/searchlib/common/allocatedbitvector.h index 2223c7b4701..5106717dad0 100644 --- a/searchlib/src/vespa/searchlib/common/allocatedbitvector.h +++ b/searchlib/src/vespa/searchlib/common/allocatedbitvector.h @@ -6,6 +6,7 @@ namespace search { +class GrowableBitVector; class BitVectorTest; /** @@ -39,7 +40,7 @@ public: AllocatedBitVector(const BitVector &other); AllocatedBitVector(const AllocatedBitVector &other); - virtual ~AllocatedBitVector(); + ~AllocatedBitVector() override; AllocatedBitVector &operator=(const AllocatedBitVector &other); AllocatedBitVector &operator=(const BitVector &other); @@ -57,9 +58,7 @@ public: * * @param newLength the new length of the bit vector (in bits) */ - void resize(Index newLength) override; - - GenerationHeldBase::UP grow(Index newLength, Index newCapacity) override; + void resize(Index newLength); protected: Index _capacityBits; @@ -67,6 +66,7 @@ protected: private: friend class BitVectorTest; + friend class GrowableBitVector; void swap(AllocatedBitVector & rhs) { std::swap(_capacityBits, rhs._capacityBits); _alloc.swap(rhs._alloc); diff --git a/searchlib/src/vespa/searchlib/common/bitvector.cpp b/searchlib/src/vespa/searchlib/common/bitvector.cpp index d5b21a38c3f..36ebc6597fa 100644 --- a/searchlib/src/vespa/searchlib/common/bitvector.cpp +++ b/searchlib/src/vespa/searchlib/common/bitvector.cpp @@ -2,7 +2,6 @@ #include "bitvector.h" #include "allocatedbitvector.h" -#include "growablebitvector.h" #include "partialbitvector.h" #include #include @@ -299,18 +298,6 @@ BitVector::hasTrueBitsInternal() const } ////////////////////////////////////////////////////////////////////// -// Set new length. Destruction of content -////////////////////////////////////////////////////////////////////// -void -BitVector::resize(Index) -{ - LOG_ABORT("should not be reached"); -} -GenerationHeldBase::UP -BitVector::grow(Index, Index ) -{ - LOG_ABORT("should not be reached"); -} size_t BitVector::getFileBytes(Index bits) @@ -383,12 +370,6 @@ BitVector::create(const BitVector & rhs) return std::make_unique(rhs); } -BitVector::UP -BitVector::create(Index numberOfElements, Index newCapacity, GenerationHolder &generationHolder) -{ - return std::make_unique(numberOfElements, newCapacity, generationHolder); -} - MMappedBitVector::MMappedBitVector(Index numberOfElements, FastOS_FileInterface &file, int64_t offset, Index doccount) : BitVector() @@ -425,7 +406,7 @@ operator<<(nbostream &out, const BitVector &bv) nbostream & -operator>>(nbostream &in, BitVector &bv) +operator>>(nbostream &in, AllocatedBitVector &bv) { uint64_t size; uint64_t cachedHits; diff --git a/searchlib/src/vespa/searchlib/common/bitvector.h b/searchlib/src/vespa/searchlib/common/bitvector.h index 53e9ed45d5e..7fea95661d7 100644 --- a/searchlib/src/vespa/searchlib/common/bitvector.h +++ b/searchlib/src/vespa/searchlib/common/bitvector.h @@ -18,6 +18,7 @@ class FastOS_FileInterface; namespace search { class PartialBitVector; +class AllocatedBitVector; class BitVector : protected BitWord { @@ -253,11 +254,6 @@ public: return getFileBytes(size()); } - virtual void resize(Index newLength); - - virtual GenerationHeldBase::UP grow(Index newLength, Index newCapacity); - GenerationHeldBase::UP grow(Index newLength) { return grow(newLength, newLength); } - /** * This will create the appropriate vector. * @@ -271,7 +267,6 @@ public: static UP create(const BitVector & org, Index start, Index end); static UP create(Index numberOfElements); static UP create(const BitVector & rhs); - static UP create(Index newSize, Index newCapacity, GenerationHolder &generationHolder); protected: using Alloc = vespalib::alloc::Alloc; VESPA_DLL_LOCAL BitVector(void * buf, Index start, Index end); @@ -384,14 +379,14 @@ protected: friend vespalib::nbostream & operator<<(vespalib::nbostream &out, const BitVector &bv); friend vespalib::nbostream & - operator>>(vespalib::nbostream &in, BitVector &bv); + operator>>(vespalib::nbostream &in, AllocatedBitVector &bv); }; vespalib::nbostream & operator<<(vespalib::nbostream &out, const BitVector &bv); vespalib::nbostream & -operator>>(vespalib::nbostream &in, BitVector &bv); +operator>>(vespalib::nbostream &in, AllocatedBitVector &bv); template void BitVector::andNotWithT(T it) { diff --git a/searchlib/src/vespa/searchlib/common/growablebitvector.cpp b/searchlib/src/vespa/searchlib/common/growablebitvector.cpp index 7c29945292e..7a548b3e7ac 100644 --- a/searchlib/src/vespa/searchlib/common/growablebitvector.cpp +++ b/searchlib/src/vespa/searchlib/common/growablebitvector.cpp @@ -7,8 +7,35 @@ namespace search { using vespalib::GenerationHeldBase; +using vespalib::GenerationHeldAlloc; using vespalib::GenerationHolder; +GenerationHeldBase::UP +GrowableBitVector::grow(Index newSize, Index newCapacity) +{ + assert(newCapacity >= newSize); + GenerationHeldBase::UP ret; + if (newCapacity != capacity()) { + AllocatedBitVector tbv(newSize, newCapacity, _alloc.get(), size(), &_alloc); + if (newSize > size()) { + tbv.clearBitAndMaintainCount(size()); // Clear old guard bit. + } + ret = std::make_unique>(_alloc); + swap(tbv); + } else { + if (newSize > size()) { + Range clearRange(size(), newSize); + setSize(newSize); + clearIntervalNoInvalidation(clearRange); + } else { + clearIntervalNoInvalidation(Range(newSize, size())); + setSize(newSize); + updateCount(); + } + } + return ret; +} + GrowableBitVector::GrowableBitVector(Index newSize, Index newCapacity, GenerationHolder &generationHolder, const Alloc* init_alloc) diff --git a/searchlib/src/vespa/searchlib/common/growablebitvector.h b/searchlib/src/vespa/searchlib/common/growablebitvector.h index 71261e130b6..8773f1573d6 100644 --- a/searchlib/src/vespa/searchlib/common/growablebitvector.h +++ b/searchlib/src/vespa/searchlib/common/growablebitvector.h @@ -16,6 +16,8 @@ public: bool shrink(Index newCapacity); bool extend(Index newCapacity); private: + GenerationHeldBase::UP grow(Index newLength, Index newCapacity); + VESPA_DLL_LOCAL bool hold(GenerationHeldBase::UP v); GenerationHolder &_generationHolder; }; -- cgit v1.2.3