From 2d22492d7b9144f7f20b9b45a19d322c5b0b5743 Mon Sep 17 00:00:00 2001 From: Geir Storli Date: Fri, 7 Oct 2022 14:57:12 +0000 Subject: Hide implementation details in datastore classes. --- .../tests/datastore/datastore/datastore_test.cpp | 3 +- vespalib/src/vespa/vespalib/datastore/datastore.h | 1 - .../src/vespa/vespalib/datastore/datastorebase.h | 66 +++++++++++++--------- 3 files changed, 41 insertions(+), 29 deletions(-) diff --git a/vespalib/src/tests/datastore/datastore/datastore_test.cpp b/vespalib/src/tests/datastore/datastore/datastore_test.cpp index 484da33aeb2..2c4e68467da 100644 --- a/vespalib/src/tests/datastore/datastore/datastore_test.cpp +++ b/vespalib/src/tests/datastore/datastore/datastore_test.cpp @@ -17,7 +17,6 @@ using vespalib::alloc::MemoryAllocator; class MyStore : public DataStore > { private: using ParentType = DataStore >; - using ParentType::_primary_buffer_ids; public: MyStore() {} explicit MyStore(std::unique_ptr> type) @@ -44,7 +43,7 @@ public: void switch_primary_buffer() { ParentType::switch_primary_buffer(0, 0u); } - size_t primary_buffer_id() const { return _primary_buffer_ids[0]; } + size_t primary_buffer_id() const { return get_primary_buffer_id(0); } BufferState& get_active_buffer_state() { return ParentType::getBufferState(primary_buffer_id()); } diff --git a/vespalib/src/vespa/vespalib/datastore/datastore.h b/vespalib/src/vespa/vespalib/datastore/datastore.h index 75c77204837..6ff71238a89 100644 --- a/vespalib/src/vespa/vespalib/datastore/datastore.h +++ b/vespalib/src/vespa/vespalib/datastore/datastore.h @@ -86,7 +86,6 @@ class DataStore : public DataStoreT protected: typedef DataStoreT ParentType; using ParentType::ensureBufferCapacity; - using ParentType::_primary_buffer_ids; using ParentType::getEntry; using ParentType::dropBuffers; using ParentType::init_primary_buffers; diff --git a/vespalib/src/vespa/vespalib/datastore/datastorebase.h b/vespalib/src/vespa/vespalib/datastore/datastorebase.h index 712a171f51c..28c47207a9f 100644 --- a/vespalib/src/vespa/vespalib/datastore/datastorebase.h +++ b/vespalib/src/vespa/vespalib/datastore/datastorebase.h @@ -25,9 +25,10 @@ class CompactionStrategy; */ class DataStoreBase { -public: +protected: /** - * Hold list before freeze, before knowing how long elements must be held. + * Hold list element used in the first phase of holding (before freeze), + * before knowing how long elements must be held. */ class ElemHold1ListElem { @@ -41,10 +42,27 @@ public: { } }; -protected: using generation_t = vespalib::GenerationHandler::generation_t; using sgeneration_t = vespalib::GenerationHandler::sgeneration_t; + /** + * Hold list element used in the second phase of holding (at freeze), + * when knowing how long elements must be held. + */ + class ElemHold2ListElem : public ElemHold1ListElem + { + public: + generation_t _generation; + + ElemHold2ListElem(const ElemHold1ListElem &hold1, generation_t generation) + : ElemHold1ListElem(hold1), + _generation(generation) + { } + }; + + using ElemHold1List = vespalib::Array; + using ElemHold2List = std::deque; + private: class BufferAndTypeId { public: @@ -60,32 +78,16 @@ private: uint32_t _typeId; }; std::vector _buffers; // For fast mapping with known types -protected: + // Provides a mapping from typeId -> primary buffer for that type. // The primary buffer is used for allocations of new element(s) if no available slots are found in free lists. std::vector _primary_buffer_ids; +protected: void* getBuffer(uint32_t bufferId) { return _buffers[bufferId].get_buffer_relaxed(); } /** - * Hold list at freeze, when knowing how long elements must be held - */ - class ElemHold2ListElem : public ElemHold1ListElem - { - public: - generation_t _generation; - - ElemHold2ListElem(const ElemHold1ListElem &hold1, generation_t generation) - : ElemHold1ListElem(hold1), - _generation(generation) - { } - }; - - using ElemHold1List = vespalib::Array; - using ElemHold2List = std::deque; - - /** - * Class used to hold the old buffer as part of fallbackResize(). + * Class used to hold the entire old buffer as part of fallbackResize(). */ class FallbackHold : public vespalib::GenerationHeldBase { @@ -129,6 +131,7 @@ protected: virtual ~DataStoreBase(); +private: /** * Get the next buffer id after the given buffer id. */ @@ -138,6 +141,7 @@ protected: ret = 0; return ret; } +protected: /** * Get the primary buffer for the given type id. @@ -156,6 +160,7 @@ protected: virtual void clearElemHoldList() = 0; void markCompacting(uint32_t bufferId); + public: uint32_t addType(BufferTypeBase *typeHandler); void init_primary_buffers(); @@ -191,9 +196,11 @@ public: */ void switch_primary_buffer(uint32_t typeId, size_t elemsNeeded); +private: bool consider_grow_active_buffer(uint32_t type_id, size_t elems_needed); void switch_or_grow_primary_buffer(uint32_t typeId, size_t elemsNeeded); +public: vespalib::MemoryUsage getMemoryUsage() const; vespalib::AddressSpace getAddressSpaceUsage() const; @@ -205,6 +212,8 @@ public: const BufferState &getBufferState(uint32_t bufferId) const { return _states[bufferId]; } BufferState &getBufferState(uint32_t bufferId) { return _states[bufferId]; } uint32_t getNumBuffers() const { return _numBuffers; } + +private: bool hasElemHold1() const { return !_elemHold1List.empty(); } /** @@ -212,16 +221,19 @@ public: */ void transferElemHoldList(generation_t generation); +public: /** * Transfer holds from hold1 to hold2 lists, assigning generation. */ void transferHoldLists(generation_t generation); +private: /** * Hold of buffer has ended. */ void doneHoldBuffer(uint32_t bufferId); +public: /** * Trim hold lists, freeing buffers that no longer needs to be held. * @@ -264,16 +276,14 @@ public: */ void disableFreeLists(); +private: /** * Enable free list management. * This only works for fixed size elements. */ void enableFreeList(uint32_t bufferId); - /** - * Disable free list management. - */ - void disableFreeList(uint32_t bufferId); +public: void disableElemHoldList(); bool has_free_lists_enabled() const { return _freeListsEnabled; } @@ -306,14 +316,18 @@ private: void onActive(uint32_t bufferId, uint32_t typeId, size_t elemsNeeded); void inc_hold_buffer_count(); + public: uint32_t getTypeId(uint32_t bufferId) const { return _buffers[bufferId].getTypeId(); } void finishCompact(const std::vector &toHold); + +private: void fallbackResize(uint32_t bufferId, size_t elementsNeeded); +public: vespalib::GenerationHolder &getGenerationHolder() { return _genHolder; } -- cgit v1.2.3 From c704c939fc124fceacd3d8471343267a7ac006a3 Mon Sep 17 00:00:00 2001 From: Geir Storli Date: Fri, 7 Oct 2022 15:43:58 +0000 Subject: Use more modern C++ features. --- .../src/vespa/vespalib/datastore/datastore.hpp | 19 +++++----- .../src/vespa/vespalib/datastore/datastorebase.cpp | 40 ++++++++++++---------- 2 files changed, 30 insertions(+), 29 deletions(-) diff --git a/vespalib/src/vespa/vespalib/datastore/datastore.hpp b/vespalib/src/vespa/vespalib/datastore/datastore.hpp index e14b248e569..18abe29d04f 100644 --- a/vespalib/src/vespa/vespalib/datastore/datastore.hpp +++ b/vespalib/src/vespa/vespalib/datastore/datastore.hpp @@ -44,19 +44,18 @@ template void DataStoreT::trimElemHoldList(generation_t usedGen) { - ElemHold2List &elemHold2List = _elemHold2List; - - ElemHold2List::iterator it(elemHold2List.begin()); - ElemHold2List::iterator ite(elemHold2List.end()); + auto it = _elemHold2List.begin(); + auto ite = _elemHold2List.end(); uint32_t freed = 0; for (; it != ite; ++it) { - if (static_cast(it->_generation - usedGen) >= 0) + if (static_cast(it->_generation - usedGen) >= 0) { break; + } free_elem_internal(it->_ref, it->_len, true); ++freed; } if (freed != 0) { - elemHold2List.erase(elemHold2List.begin(), it); + _elemHold2List.erase(_elemHold2List.begin(), it); } } @@ -64,14 +63,12 @@ template void DataStoreT::clearElemHoldList() { - ElemHold2List &elemHold2List = _elemHold2List; - - ElemHold2List::iterator it(elemHold2List.begin()); - ElemHold2List::iterator ite(elemHold2List.end()); + auto it = _elemHold2List.begin(); + auto ite = _elemHold2List.end(); for (; it != ite; ++it) { free_elem_internal(it->_ref, it->_len, true); } - elemHold2List.clear(); + _elemHold2List.clear(); } template diff --git a/vespalib/src/vespa/vespalib/datastore/datastorebase.cpp b/vespalib/src/vespa/vespalib/datastore/datastorebase.cpp index a5ed2c05f22..dd6c767e9c6 100644 --- a/vespalib/src/vespa/vespalib/datastore/datastorebase.cpp +++ b/vespalib/src/vespa/vespalib/datastore/datastorebase.cpp @@ -223,9 +223,8 @@ DataStoreBase::addType(BufferTypeBase *typeHandler) void DataStoreBase::transferElemHoldList(generation_t generation) { - ElemHold2List &elemHold2List = _elemHold2List; - for (const ElemHold1ListElem & elemHold1 : _elemHold1List) { - elemHold2List.push_back(ElemHold2ListElem(elemHold1, generation)); + for (const auto& elemHold1 : _elemHold1List) { + _elemHold2List.push_back(ElemHold2ListElem(elemHold1, generation)); } _elemHold1List.clear(); } @@ -289,14 +288,14 @@ DataStoreBase::holdBuffer(uint32_t bufferId) { _states[bufferId].onHold(bufferId); size_t holdBytes = 0u; // getMemStats() still accounts held buffers - GenerationHeldBase::UP hold(new BufferHold(holdBytes, *this, bufferId)); + auto hold = std::make_unique(holdBytes, *this, bufferId); _genHolder.hold(std::move(hold)); } void DataStoreBase::enableFreeLists() { - for (BufferState & bState : _states) { + for (auto& bState : _states) { if (!bState.isActive() || bState.getCompacting()) { continue; } @@ -308,7 +307,7 @@ DataStoreBase::enableFreeLists() void DataStoreBase::disableFreeLists() { - for (BufferState & bState : _states) { + for (auto& bState : _states) { bState.disable_free_list(); } _freeListsEnabled = false; @@ -340,9 +339,9 @@ DataStoreBase::getMemStats() const { MemoryStats stats; - for (const BufferState & bState: _states) { + for (const auto& bState: _states) { auto typeHandler = bState.getTypeHandler(); - BufferState::State state = bState.getState(); + auto state = bState.getState(); if ((state == BufferState::State::FREE) || (typeHandler == nullptr)) { ++stats._freeBuffers; } else if (state == BufferState::State::ACTIVE) { @@ -370,7 +369,7 @@ DataStoreBase::getAddressSpaceUsage() const size_t usedArrays = 0; size_t deadArrays = 0; size_t limitArrays = 0; - for (const BufferState & bState: _states) { + for (const auto& bState: _states) { if (bState.isActive()) { uint32_t arraySize = bState.getArraySize(); usedArrays += bState.size() / arraySize; @@ -386,7 +385,7 @@ DataStoreBase::getAddressSpaceUsage() const LOG_ABORT("should not be reached"); } } - return vespalib::AddressSpace(usedArrays, deadArrays, limitArrays); + return {usedArrays, deadArrays, limitArrays}; } void @@ -423,12 +422,11 @@ DataStoreBase::fallbackResize(uint32_t bufferId, size_t elemsNeeded) state.fallbackResize(bufferId, elemsNeeded, _buffers[bufferId].get_atomic_buffer(), toHoldBuffer); - GenerationHeldBase::UP - hold(new FallbackHold(oldAllocElems * elementSize, - std::move(toHoldBuffer), - oldUsedElems, - state.getTypeHandler(), - state.getTypeId())); + auto hold = std::make_unique(oldAllocElems * elementSize, + std::move(toHoldBuffer), + oldUsedElems, + state.getTypeHandler(), + state.getTypeId()); if (!_initializing) { _genHolder.hold(std::move(hold)); } @@ -454,9 +452,15 @@ std::unique_ptr DataStoreBase::start_compact_worst_buffers(CompactionSpec compaction_spec, const CompactionStrategy& compaction_strategy) { // compact memory usage - CompactBufferCandidates elem_buffers(_numBuffers, compaction_strategy.get_max_buffers(), compaction_strategy.get_active_buffers_ratio(), compaction_strategy.getMaxDeadBytesRatio() / 2, CompactionStrategy::DEAD_BYTES_SLACK); + CompactBufferCandidates elem_buffers(_numBuffers, compaction_strategy.get_max_buffers(), + compaction_strategy.get_active_buffers_ratio(), + compaction_strategy.getMaxDeadBytesRatio() / 2, + CompactionStrategy::DEAD_BYTES_SLACK); // compact address space - CompactBufferCandidates array_buffers(_numBuffers, compaction_strategy.get_max_buffers(), compaction_strategy.get_active_buffers_ratio(), compaction_strategy.getMaxDeadAddressSpaceRatio() / 2, CompactionStrategy::DEAD_ADDRESS_SPACE_SLACK); + CompactBufferCandidates array_buffers(_numBuffers, compaction_strategy.get_max_buffers(), + compaction_strategy.get_active_buffers_ratio(), + compaction_strategy.getMaxDeadAddressSpaceRatio() / 2, + CompactionStrategy::DEAD_ADDRESS_SPACE_SLACK); uint32_t free_buffers = 0; for (uint32_t bufferId = 0; bufferId < _numBuffers; ++bufferId) { const auto &state = getBufferState(bufferId); -- cgit v1.2.3