diff options
author | Henning Baldersheim <balder@yahoo-inc.com> | 2020-12-17 23:40:18 +0000 |
---|---|---|
committer | Henning Baldersheim <balder@yahoo-inc.com> | 2021-01-10 22:35:28 +0000 |
commit | dfd9a1b244725de2f594053cdb11c7ef0f3cb9ae (patch) | |
tree | ad22e0e180410289f38b92daa251786aa36fd6d5 | |
parent | 00d005ea8544ee5677c2f18d5358cf65afebbf32 (diff) |
The bufferstate vector normally has 8k elements. Since the BufferState was 160 bytes this is aroung 1.2M.
This reduces the BufferState to 128 making it fit nicely in a 1M allocation.
However the cost is that you get an extra small allocation for the _freeList. We should see if we could reclain that by further reducing the size of the BufferState.
Perhaps the element count will be sufficient with uint32_t.
5 files changed, 87 insertions, 74 deletions
diff --git a/vespalib/src/tests/datastore/datastore/datastore_test.cpp b/vespalib/src/tests/datastore/datastore/datastore_test.cpp index a5bb51e5074..a319a423201 100644 --- a/vespalib/src/tests/datastore/datastore/datastore_test.cpp +++ b/vespalib/src/tests/datastore/datastore/datastore_test.cpp @@ -590,6 +590,14 @@ TEST(DataStoreTest, require_that_offset_in_EntryRefT_is_within_bounds_when_alloc assertGrowStats<uint32_t>({16384,16384,16384,32768,32768,65536,131072,131072,229376,229376,229376,229376}, 7); } +TEST(DataStoreTest, control_static_sizes) { + EXPECT_EQ(32, sizeof(BufferState::FreeList)); + EXPECT_EQ(4, sizeof(BufferState::State)); + EXPECT_EQ(128, sizeof(BufferState)); + BufferState bs; + EXPECT_EQ(0, bs.size()); +} + } GTEST_MAIN_RUN_ALL_TESTS() diff --git a/vespalib/src/vespa/vespalib/datastore/bufferstate.cpp b/vespalib/src/vespa/vespalib/datastore/bufferstate.cpp index eb301556c4b..8db1b6ff076 100644 --- a/vespalib/src/vespa/vespalib/datastore/bufferstate.cpp +++ b/vespalib/src/vespa/vespalib/datastore/bufferstate.cpp @@ -10,7 +10,7 @@ namespace vespalib::datastore { BufferState::FreeListList::~FreeListList() { - assert(_head == NULL); // Owner should have disabled free lists + assert(_head == nullptr); // Owner should have disabled free lists } @@ -18,20 +18,20 @@ BufferState::BufferState() : _usedElems(0), _allocElems(0), _deadElems(0u), - _state(FREE), - _disableElemHoldList(false), _holdElems(0u), _extraUsedBytes(0), _extraHoldBytes(0), _freeList(), - _freeListList(NULL), - _nextHasFree(NULL), - _prevHasFree(NULL), - _typeHandler(NULL), + _freeListList(nullptr), + _nextHasFree(nullptr), + _prevHasFree(nullptr), + _typeHandler(nullptr), + _buffer(Alloc::alloc(0, MemoryAllocator::HUGEPAGE_SIZE)), _typeId(0), _arraySize(0), - _compacting(false), - _buffer(Alloc::alloc(0, MemoryAllocator::HUGEPAGE_SIZE)) + _state(FREE), + _disableElemHoldList(false), + _compacting(false) { } @@ -39,11 +39,11 @@ BufferState::BufferState() BufferState::~BufferState() { assert(_state == FREE); - assert(_freeListList == NULL); - assert(_nextHasFree == NULL); - assert(_prevHasFree == NULL); + assert(_freeListList == nullptr); + assert(_nextHasFree == nullptr); + assert(_prevHasFree == nullptr); assert(_holdElems == 0); - assert(_freeList.empty()); + assert(isFreeListEmpty()); } namespace { @@ -97,20 +97,20 @@ BufferState::onActive(uint32_t bufferId, uint32_t typeId, size_t elementsNeeded, void *&buffer) { - assert(buffer == NULL); - assert(_buffer.get() == NULL); + assert(buffer == nullptr); + assert(_buffer.get() == nullptr); assert(_state == FREE); - assert(_typeHandler == NULL); + assert(_typeHandler == nullptr); assert(_allocElems == 0); assert(_usedElems == 0); assert(_deadElems == 0u); assert(_holdElems == 0); assert(_extraUsedBytes == 0); assert(_extraHoldBytes == 0); - assert(_freeList.empty()); - assert(_nextHasFree == NULL); - assert(_prevHasFree == NULL); - assert(_freeListList == NULL || _freeListList->_head != this); + assert(isFreeListEmpty()); + assert(_nextHasFree == nullptr); + assert(_prevHasFree == nullptr); + assert(_freeListList == nullptr || _freeListList->_head != this); size_t reservedElements = typeHandler->getReservedElements(bufferId); (void) reservedElements; @@ -118,7 +118,7 @@ BufferState::onActive(uint32_t bufferId, uint32_t typeId, assert(alloc.elements >= reservedElements + elementsNeeded); _buffer.create(alloc.bytes).swap(_buffer); buffer = _buffer.get(); - assert(buffer != NULL || alloc.elements == 0u); + assert(buffer != nullptr || alloc.elements == 0u); _allocElems = alloc.elements; _state = ACTIVE; _typeHandler = typeHandler; @@ -132,7 +132,7 @@ void BufferState::onHold() { assert(_state == ACTIVE); - assert(_typeHandler != NULL); + assert(_typeHandler != nullptr); _state = HOLD; _compacting = false; assert(_deadElems <= _usedElems); @@ -140,14 +140,14 @@ BufferState::onHold() _deadElems = 0; _holdElems = _usedElems; // Put everyting on hold _typeHandler->onHold(&_usedElems); - if (!_freeList.empty()) { + if ( ! isFreeListEmpty()) { removeFromFreeListList(); - FreeList().swap(_freeList); + _freeList.reset(); } - assert(_nextHasFree == NULL); - assert(_prevHasFree == NULL); - assert(_freeListList == NULL || _freeListList->_head != this); - setFreeListList(NULL); + assert(_nextHasFree == nullptr); + assert(_prevHasFree == nullptr); + assert(_freeListList == nullptr || _freeListList->_head != this); + setFreeListList(nullptr); } @@ -156,13 +156,13 @@ BufferState::onFree(void *&buffer) { assert(buffer == _buffer.get()); assert(_state == HOLD); - assert(_typeHandler != NULL); + assert(_typeHandler != nullptr); assert(_deadElems <= _usedElems); assert(_holdElems == _usedElems - _deadElems); _typeHandler->destroyElements(buffer, _usedElems); Alloc::alloc().swap(_buffer); _typeHandler->onFree(_usedElems); - buffer = NULL; + buffer = nullptr; _usedElems = 0; _allocElems = 0; _deadElems = 0u; @@ -170,13 +170,13 @@ BufferState::onFree(void *&buffer) _extraUsedBytes = 0; _extraHoldBytes = 0; _state = FREE; - _typeHandler = NULL; + _typeHandler = nullptr; _arraySize = 0; - assert(_freeList.empty()); - assert(_nextHasFree == NULL); - assert(_prevHasFree == NULL); - assert(_freeListList == NULL || _freeListList->_head != this); - setFreeListList(NULL); + assert(isFreeListEmpty()); + assert(_nextHasFree == nullptr); + assert(_prevHasFree == nullptr); + assert(_freeListList == nullptr || _freeListList->_head != this); + setFreeListList(nullptr); _disableElemHoldList = false; } @@ -185,10 +185,10 @@ void BufferState::dropBuffer(void *&buffer) { if (_state == FREE) { - assert(buffer == NULL); + assert(buffer == nullptr); return; } - assert(buffer != NULL || _allocElems == 0); + assert(buffer != nullptr || _allocElems == 0); if (_state == ACTIVE) { onHold(); } @@ -196,28 +196,28 @@ BufferState::dropBuffer(void *&buffer) onFree(buffer); } assert(_state == FREE); - assert(buffer == NULL); + assert(buffer == nullptr); } void BufferState::setFreeListList(FreeListList *freeListList) { - if (_state == FREE && freeListList != NULL) { + if (_state == FREE && freeListList != nullptr) { return; } if (freeListList == _freeListList) { return; // No change } - if (_freeListList != NULL && !_freeList.empty()) { + if (_freeListList != nullptr && ! isFreeListEmpty()) { removeFromFreeListList(); // Remove from old free list } _freeListList = freeListList; - if (!_freeList.empty()) { - if (freeListList != NULL) { + if ( ! isFreeListEmpty() ) { + if (freeListList != nullptr) { addToFreeListList(); // Changed free list list } else { - FreeList().swap(_freeList); // Free lists have been disabled + _freeList.reset(); // Free lists have been disabled } } } @@ -226,10 +226,10 @@ BufferState::setFreeListList(FreeListList *freeListList) void BufferState::addToFreeListList() { - assert(_freeListList != NULL && _freeListList->_head != this); - assert(_nextHasFree == NULL); - assert(_prevHasFree == NULL); - if (_freeListList->_head != NULL) { + assert(_freeListList != nullptr && _freeListList->_head != this); + assert(_nextHasFree == nullptr); + assert(_prevHasFree == nullptr); + if (_freeListList->_head != nullptr) { _nextHasFree = _freeListList->_head; _prevHasFree = _nextHasFree->_prevHasFree; _nextHasFree->_prevHasFree = this; @@ -239,27 +239,30 @@ BufferState::addToFreeListList() _prevHasFree = this; } _freeListList->_head = this; + if ( ! _freeList) { + _freeList = std::make_unique<FreeList>(); + } } void BufferState::removeFromFreeListList() { - assert(_freeListList != NULL); - assert(_nextHasFree != NULL); - assert(_prevHasFree != NULL); + assert(_freeListList != nullptr); + assert(_nextHasFree != nullptr); + assert(_prevHasFree != nullptr); if (_nextHasFree == this) { assert(_prevHasFree == this); assert(_freeListList->_head == this); - _freeListList->_head = NULL; + _freeListList->_head = nullptr; } else { assert(_prevHasFree != this); _freeListList->_head = _nextHasFree; _nextHasFree->_prevHasFree = _prevHasFree; _prevHasFree->_nextHasFree = _nextHasFree; } - _nextHasFree = NULL; - _prevHasFree = NULL; + _nextHasFree = nullptr; + _prevHasFree = nullptr; } @@ -277,8 +280,8 @@ BufferState::fallbackResize(uint32_t bufferId, Alloc &holdBuffer) { assert(_state == ACTIVE); - assert(_typeHandler != NULL); - assert(holdBuffer.get() == NULL); + assert(_typeHandler != nullptr); + assert(holdBuffer.get() == nullptr); AllocResult alloc = calcAllocation(bufferId, *_typeHandler, elementsNeeded, true); assert(alloc.elements >= _usedElems + elementsNeeded); assert(alloc.elements > _allocElems); diff --git a/vespalib/src/vespa/vespalib/datastore/bufferstate.h b/vespalib/src/vespa/vespalib/datastore/bufferstate.h index 814b4fe95f7..5f0a2dedbd7 100644 --- a/vespalib/src/vespa/vespalib/datastore/bufferstate.h +++ b/vespalib/src/vespa/vespalib/datastore/bufferstate.h @@ -30,11 +30,11 @@ public: public: BufferState *_head; - FreeListList() : _head(NULL) { } + FreeListList() : _head(nullptr) { } ~FreeListList(); }; - typedef vespalib::Array<EntryRef> FreeList; + using FreeList = vespalib::Array<EntryRef>; enum State { FREE, @@ -46,8 +46,6 @@ private: size_t _usedElems; size_t _allocElems; size_t _deadElems; - State _state; - bool _disableElemHoldList; size_t _holdElems; // Number of bytes that are heap allocated by elements that are stored in this buffer. // For simple types this is 0. @@ -55,18 +53,20 @@ private: // Number of bytes that are heap allocated by elements that are stored in this buffer and is now on hold. // For simple types this is 0. size_t _extraHoldBytes; - FreeList _freeList; - FreeListList *_freeListList; // non-NULL if free lists are enabled + std::unique_ptr<FreeList> _freeList; + FreeListList *_freeListList; // non-nullptr if free lists are enabled - // NULL pointers if not on circular list of buffer states with free elems + // nullptr if not on circular list of buffer states with free elems BufferState *_nextHasFree; BufferState *_prevHasFree; BufferTypeBase *_typeHandler; + Alloc _buffer; uint32_t _typeId; uint32_t _arraySize; + State _state; + bool _disableElemHoldList; bool _compacting; - Alloc _buffer; public: /* @@ -75,6 +75,8 @@ public: */ BufferState(); + BufferState(const BufferState &) = delete; + BufferState & operator=(const BufferState &) = delete; ~BufferState(); /** @@ -102,7 +104,7 @@ public: /** * Set list of buffer states with nonempty free lists. * - * @param freeListList List of buffer states. If NULL then free lists + * @param freeListList List of buffer states. If nullptr then free lists * are disabled. */ void setFreeListList(FreeListList *freeListList); @@ -130,9 +132,9 @@ public: * Pop element from free list. */ EntryRef popFreeList() { - EntryRef ret = _freeList.back(); - _freeList.pop_back(); - if (_freeList.empty()) { + EntryRef ret = _freeList->back(); + _freeList->pop_back(); + if (_freeList->empty()) { removeFromFreeListList(); } _deadElems -= _arraySize; @@ -182,8 +184,8 @@ public: } bool hasDisabledElemHoldList() const { return _disableElemHoldList; } - const FreeList &freeList() const { return _freeList; } - FreeList &freeList() { return _freeList; } + bool isFreeListEmpty() const { return !_freeList || _freeList->empty();} + FreeList &freeList() { return *_freeList; } const FreeListList *freeListList() const { return _freeListList; } FreeListList *freeListList() { return _freeListList; } diff --git a/vespalib/src/vespa/vespalib/datastore/datastore.hpp b/vespalib/src/vespa/vespalib/datastore/datastore.hpp index f4e37804317..42146eab9aa 100644 --- a/vespalib/src/vespa/vespalib/datastore/datastore.hpp +++ b/vespalib/src/vespa/vespalib/datastore/datastore.hpp @@ -29,7 +29,7 @@ DataStoreT<RefT>::freeElem(EntryRef ref, size_t numElems) BufferState &state = getBufferState(intRef.bufferId()); if (state.isActive()) { if (state.freeListList() != nullptr && numElems == state.getArraySize()) { - if (state.freeList().empty()) { + if (state.isFreeListEmpty()) { state.addToFreeListList(); } state.freeList().push_back(ref); diff --git a/vespalib/src/vespa/vespalib/datastore/free_list_allocator.hpp b/vespalib/src/vespa/vespalib/datastore/free_list_allocator.hpp index 32ae33a56c0..c73576a619e 100644 --- a/vespalib/src/vespa/vespalib/datastore/free_list_allocator.hpp +++ b/vespalib/src/vespa/vespalib/datastore/free_list_allocator.hpp @@ -53,7 +53,7 @@ typename Allocator<EntryT, RefT>::HandleType FreeListAllocator<EntryT, RefT, ReclaimerT>::alloc(Args && ... args) { BufferState::FreeListList &freeListList = _store.getFreeList(_typeId); - if (freeListList._head == NULL) { + if (freeListList._head == nullptr) { return ParentType::alloc(std::forward<Args>(args)...); } BufferState &state = *freeListList._head; @@ -70,7 +70,7 @@ typename Allocator<EntryT, RefT>::HandleType FreeListAllocator<EntryT, RefT, ReclaimerT>::allocArray(ConstArrayRef array) { BufferState::FreeListList &freeListList = _store.getFreeList(_typeId); - if (freeListList._head == NULL) { + if (freeListList._head == nullptr) { return ParentType::allocArray(array); } BufferState &state = *freeListList._head; @@ -89,7 +89,7 @@ typename Allocator<EntryT, RefT>::HandleType FreeListAllocator<EntryT, RefT, ReclaimerT>::allocArray(size_t size) { BufferState::FreeListList &freeListList = _store.getFreeList(_typeId); - if (freeListList._head == NULL) { + if (freeListList._head == nullptr) { return ParentType::allocArray(size); } BufferState &state = *freeListList._head; |