From 9e3157f40daba3cb9125ec437914cfb56e2713e7 Mon Sep 17 00:00:00 2001 From: Joseph Birkner Date: Wed, 4 Mar 2026 13:53:30 +0100 Subject: [PATCH 1/4] Move singleton array storage to dedicated feature branch --- include/simfil/model/arena.h | 375 +++++++++++++++++++++++--- include/simfil/model/bitsery-traits.h | 28 +- include/simfil/model/model.h | 3 +- include/simfil/model/nodes.h | 4 +- src/model/model.cpp | 12 +- test/arena.cpp | 74 +++-- 6 files changed, 438 insertions(+), 58 deletions(-) diff --git a/include/simfil/model/arena.h b/include/simfil/model/arena.h index 642279ca..cf386576 100644 --- a/include/simfil/model/arena.h +++ b/include/simfil/model/arena.h @@ -10,6 +10,8 @@ #include #include #include +#include +#include #include #include "simfil/model/column.h" @@ -27,11 +29,15 @@ namespace bitsery::ext { namespace simfil { -/// Address of an array within an ArrayArena -using ArrayIndex = int32_t; +/// Address of an array within an ArrayArena. Note, that only the lowest 3B may be +/// used. This is to allow passing ArrayIndex as the value of a ModelNodeAddress. +using ArrayIndex = uint32_t; /// Array index which can be used to indicate a default/invalid value. -constexpr static ArrayIndex InvalidArrayIndex = -1; +constexpr static ArrayIndex InvalidArrayIndex = 0x00ffffffu; +constexpr static ArrayIndex FirstRegularArrayIndex = 1u; +constexpr static ArrayIndex SingletonArrayHandleMask = 0x00800000u; +constexpr static ArrayIndex SingletonArrayHandlePayloadMask = 0x007fffffu; /** * ArrayArena - An arena allocator for append-only vectors. @@ -53,6 +59,16 @@ class ArrayArena public: using ElementType = ElementType_; using SizeType = SizeType_; + struct SingletonStats + { + size_t handleCount = 0; + size_t occupiedCount = 0; + size_t emptyCount = 0; + size_t singletonStorageBytes = 0; + size_t hypotheticalRegularBytes = 0; + size_t estimatedSavedBytes = 0; + }; + struct CompactArrayChunk { MODEL_COLUMN_TYPE(8); @@ -68,6 +84,22 @@ class ArrayArena }; using CompactHeadStorage = ModelColumn; + ArrayArena() + { + ensure_regular_head_pool(); + } + + static constexpr bool is_singleton_handle(ArrayIndex arrayIndex) + { + return arrayIndex != InvalidArrayIndex && + (arrayIndex & SingletonArrayHandleMask) != 0; + } + + static constexpr ArrayIndex singleton_payload(ArrayIndex handle) + { + return handle & SingletonArrayHandlePayloadMask; + } + /** * Creates a new array with the specified initial capacity. * @@ -80,9 +112,25 @@ class ArrayArena std::unique_lock guard(lock_); #endif ensure_runtime_heads_from_compact(); + + if (initialCapacity == 1U) { + auto singletonIndex = to_array_index(singletonValues_.size()); + if (singletonIndex > SingletonArrayHandlePayloadMask) { + raise("ArrayArena singleton pool exhausted."); + } + singletonValues_.emplace_back(ElementType_{}); + singletonOccupied_.emplace_back(static_cast(0)); + compactHeads_.reset(); + return SingletonArrayHandleMask | singletonIndex; + } + + ensure_regular_head_pool(); size_t offset = data_.size(); data_.resize(offset + initialCapacity); - auto index = static_cast(heads_.size()); + auto index = to_array_index(heads_.size()); + if ((index & SingletonArrayHandleMask) != 0) { + raise("ArrayArena regular head index exceeded handle bit range."); + } heads_.push_back({(SizeType_)offset, (SizeType_)initialCapacity, 0, InvalidArrayIndex, InvalidArrayIndex}); @@ -103,18 +151,85 @@ class ArrayArena return heads_.size(); } + [[nodiscard]] size_t singleton_handle_count() const + { + return singletonValues_.size(); + } + + [[nodiscard]] size_t singleton_occupied_count() const + { + size_t occupiedCount = 0; + for (auto const occupied : singletonOccupied_) { + occupiedCount += occupied == 0 ? 0 : 1; + } + return occupiedCount; + } + + [[nodiscard]] SingletonStats singleton_stats() const + { + const auto handleCount = singleton_handle_count(); + const auto occupiedCount = singleton_occupied_count(); + const auto emptyCount = handleCount >= occupiedCount ? handleCount - occupiedCount : 0; + + const auto singletonStorageBytes = + singletonValues_.byte_size() + singletonOccupied_.byte_size(); + const auto hypotheticalRegularBytes = + handleCount * sizeof(CompactArrayChunk) + occupiedCount * sizeof(ElementType_); + + return SingletonStats{ + .handleCount = handleCount, + .occupiedCount = occupiedCount, + .emptyCount = emptyCount, + .singletonStorageBytes = singletonStorageBytes, + .hypotheticalRegularBytes = hypotheticalRegularBytes, + .estimatedSavedBytes = hypotheticalRegularBytes > singletonStorageBytes + ? hypotheticalRegularBytes - singletonStorageBytes + : 0}; + } + + [[nodiscard]] bool valid(ArrayIndex a) const + { + if (a == InvalidArrayIndex) { + return false; + } + if (is_singleton_handle(a)) { + auto singletonIndex = singleton_payload(a); + return singletonIndex < singletonValues_.size() && + singletonIndex < singletonOccupied_.size(); + } + if (heads_.empty() && compactHeads_) { + return a < compactHeads_->size(); + } + return a < heads_.size(); + } + /** * Returns the size of the specified array. * * @param a The index of the array. * @return The size of the array. */ - [[nodiscard]] SizeType_ size(ArrayIndex const& a) const { + [[nodiscard]] SizeType_ size(ArrayIndex a) const { #ifdef ARRAY_ARENA_THREAD_SAFE std::shared_lock guard(lock_); #endif - if (heads_.empty() && compactHeads_) + if (is_singleton_handle(a)) { + auto singletonIndex = singleton_payload(a); + if (singletonIndex >= singletonOccupied_.size()) { + raise("ArrayArena singleton handle index out of range."); + } + return singletonOccupied_.at(singletonIndex) == 0 ? 0 : 1; + } + + if (heads_.empty() && compactHeads_) { + if (a >= compactHeads_->size()) { + raise("ArrayArena head index out of range."); + } return static_cast((*compactHeads_)[a].size); + } + if (a >= heads_.size()) { + raise("ArrayArena head index out of range."); + } return heads_[a].size; } @@ -122,14 +237,17 @@ class ArrayArena * @return The current size, in bytes, of the array arena if serialized. */ [[nodiscard]] size_t byte_size() const { + auto singletonBytes = + singletonValues_.byte_size() + + singletonOccupied_.byte_size(); if (compactHeads_) { - return compactHeads_->byte_size() + data_.byte_size(); + return compactHeads_->byte_size() + data_.byte_size() + singletonBytes; } auto result = heads_.size() * sizeof(CompactArrayChunk); for (auto const& head : heads_) { result += head.size * sizeof(ElementType_); } - return result; + return result + singletonBytes; } /** @@ -141,11 +259,11 @@ class ArrayArena * @throws std::out_of_range if the index is out of the array bounds. */ tl::expected, Error> - at(ArrayIndex const& a, size_t const& i) { + at(ArrayIndex a, size_t i) { return at_impl(*this, a, i); } tl::expected, Error> - at(ArrayIndex const& a, size_t const& i) const { + at(ArrayIndex a, size_t i) const { return at_impl(*this, a, i); } @@ -156,8 +274,28 @@ class ArrayArena * @param data The element to be appended. * @return A reference to the appended element. */ - ElementType_& push_back(ArrayIndex const& a, ElementType_ const& data) + ElementType_& push_back(ArrayIndex a, ElementType_ const& data) { + if (is_singleton_handle(a)) { + #ifdef ARRAY_ARENA_THREAD_SAFE + std::unique_lock guard(lock_); + #endif + auto singletonIndex = singleton_payload(a); + if (singletonIndex >= singletonValues_.size() || + singletonIndex >= singletonOccupied_.size()) { + raise("ArrayArena singleton handle index out of range."); + } + auto& occupied = singletonOccupied_.at(singletonIndex); + if (occupied != 0) { + raise( + "Cannot append more than one element to a singleton array handle."); + } + singletonValues_.at(singletonIndex) = data; + occupied = 1; + compactHeads_.reset(); + return singletonValues_.at(singletonIndex); + } + Chunk& updatedLast = ensure_capacity_and_get_last_chunk(a); #ifdef ARRAY_ARENA_THREAD_SAFE std::shared_lock guard(lock_); @@ -180,8 +318,28 @@ class ArrayArena * @return A reference to the appended element. */ template - ElementType_& emplace_back(ArrayIndex const& a, Args&&... args) + ElementType_& emplace_back(ArrayIndex a, Args&&... args) { + if (is_singleton_handle(a)) { + #ifdef ARRAY_ARENA_THREAD_SAFE + std::unique_lock guard(lock_); + #endif + auto singletonIndex = singleton_payload(a); + if (singletonIndex >= singletonValues_.size() || + singletonIndex >= singletonOccupied_.size()) { + raise("ArrayArena singleton handle index out of range."); + } + auto& occupied = singletonOccupied_.at(singletonIndex); + if (occupied != 0) { + raise( + "Cannot append more than one element to a singleton array handle."); + } + singletonValues_.at(singletonIndex) = ElementType_(std::forward(args)...); + occupied = 1; + compactHeads_.reset(); + return singletonValues_.at(singletonIndex); + } + Chunk& updatedLast = ensure_capacity_and_get_last_chunk(a); #ifdef ARRAY_ARENA_THREAD_SAFE std::shared_lock guard(lock_); @@ -208,7 +366,10 @@ class ArrayArena heads_.clear(); continuations_.clear(); data_.clear(); + singletonValues_.clear(); + singletonOccupied_.clear(); compactHeads_.reset(); + ensure_regular_head_pool(); } /** @@ -225,6 +386,8 @@ class ArrayArena heads_.shrink_to_fit(); continuations_.shrink_to_fit(); data_.shrink_to_fit(); + singletonValues_.shrink_to_fit(); + singletonOccupied_.shrink_to_fit(); if (compactHeads_) { compactHeads_->shrink_to_fit(); } @@ -233,10 +396,14 @@ class ArrayArena /** * Check if a compact chunk representation is available. */ - [[nodiscard]] bool isCompact() const { + [[nodiscard]] bool is_compact() const { return compactHeads_.has_value(); } + [[nodiscard]] bool isCompact() const { + return is_compact(); + } + // Iterator-related types and functions template class ArrayIterator; @@ -296,7 +463,7 @@ class ArrayArena iterator begin() const { return begin_; } iterator end() const { return end_; } [[nodiscard]] size_t size() const { return begin_.arena_.size(begin_.array_index_); } - decltype(auto) operator[] (size_t const& i) const { return begin_.arena_.at(begin_.array_index_, i); } + decltype(auto) operator[] (size_t i) const { return begin_.arena_.at(begin_.array_index_, i); } private: iterator begin_; @@ -306,8 +473,12 @@ class ArrayArena class ArrayArenaIterator { public: - ArrayArenaIterator(ArrayArena& arena, ArrayIndex index) - : arena_(arena), index_(index) {} + ArrayArenaIterator(ArrayArena& arena, size_t ordinal) + : arena_(arena), + ordinal_(ordinal) + { + update_array_index(); + } iterator begin() { return arena_.begin(index_); } iterator end() { return arena_.end(index_); } @@ -319,12 +490,13 @@ class ArrayArena } ArrayArenaIterator& operator++() { - ++index_; + ++ordinal_; + update_array_index(); return *this; } bool operator==(const ArrayArenaIterator& other) const { - return &arena_ == &other.arena_ && index_ == other.index_; + return &arena_ == &other.arena_ && ordinal_ == other.ordinal_; } bool operator!=(const ArrayArenaIterator& other) const { @@ -338,28 +510,109 @@ class ArrayArena using reference = value_type&; private: + [[nodiscard]] size_t regular_array_count() const + { + if (arena_.heads_.empty() && arena_.compactHeads_) { + return arena_.compactHeads_->size(); + } + return arena_.heads_.size(); + } + + [[nodiscard]] size_t visible_regular_array_count() const + { + const auto count = regular_array_count(); + return count > FirstRegularArrayIndex ? count - FirstRegularArrayIndex : 0; + } + + [[nodiscard]] size_t total_visible_array_count() const + { + return visible_regular_array_count() + arena_.singleton_handle_count(); + } + + void update_array_index() + { + const auto regularCount = visible_regular_array_count(); + if (ordinal_ < regularCount) { + index_ = to_array_index(FirstRegularArrayIndex + ordinal_); + return; + } + + const auto singletonOrdinal = ordinal_ - regularCount; + if (ordinal_ < total_visible_array_count() && + singletonOrdinal <= SingletonArrayHandlePayloadMask) { + index_ = SingletonArrayHandleMask | to_array_index(singletonOrdinal); + return; + } + + index_ = InvalidArrayIndex; + } + ArrayArena& arena_; + size_t ordinal_ = 0; ArrayIndex index_; }; - iterator begin(ArrayIndex const& a) { return iterator(*this, a, 0); } - iterator end(ArrayIndex const& a) { return iterator(*this, a, size(a)); } - const_iterator begin(ArrayIndex const& a) const { return const_iterator(*this, a, 0); } - const_iterator end(ArrayIndex const& a) const { return const_iterator(*this, a, size(a)); } + iterator begin(ArrayIndex a) { return iterator(*this, a, 0); } + iterator end(ArrayIndex a) { return iterator(*this, a, size(a)); } + const_iterator begin(ArrayIndex a) const { return const_iterator(*this, a, 0); } + const_iterator end(ArrayIndex a) const { return const_iterator(*this, a, size(a)); } ArrayArenaIterator begin() { return ArrayArenaIterator(*this, 0); } - ArrayArenaIterator end() { return ArrayArenaIterator(*this, static_cast(size())); } - ArrayArenaIterator begin() const { return ArrayArenaIterator(*this, 0); } - ArrayArenaIterator end() const { return ArrayArenaIterator(*this, static_cast(size())); } + ArrayArenaIterator end() + { + const auto regularCount = size(); + const auto visibleRegularCount = regularCount > FirstRegularArrayIndex + ? regularCount - FirstRegularArrayIndex + : 0; + return ArrayArenaIterator(*this, visibleRegularCount + singleton_handle_count()); + } + ArrayArenaIterator begin() const + { + return ArrayArenaIterator(const_cast(*this), 0); + } + ArrayArenaIterator end() const + { + const auto regularCount = size(); + const auto visibleRegularCount = regularCount > FirstRegularArrayIndex + ? regularCount - FirstRegularArrayIndex + : 0; + return ArrayArenaIterator( + const_cast(*this), + visibleRegularCount + singleton_handle_count()); + } - ArrayRange range(ArrayIndex const& array) {return ArrayRange(begin(array), end(array));} + ArrayRange range(ArrayIndex array) {return ArrayRange(begin(array), end(array));} /// Support fast iteration via callback. The passed lambda needs to return true, /// as long as the iteration is supposed to continue. template - void iterate(ArrayIndex const& a, Func&& lambda) + void iterate(ArrayIndex a, Func&& lambda) { + if (is_singleton_handle(a)) { + auto singletonIndex = singleton_payload(a); + if (singletonIndex >= singletonValues_.size() || + singletonIndex >= singletonOccupied_.size()) { + raise("ArrayArena singleton handle index out of range."); + } + if (singletonOccupied_.at(singletonIndex) == 0) { + return; + } + if constexpr (std::is_invocable_r_v) { + (void)lambda(singletonValues_.at(singletonIndex)); + } + else if constexpr (std::is_invocable_v) { + lambda(singletonValues_.at(singletonIndex), 0); + } + else { + lambda(singletonValues_.at(singletonIndex)); + } + return; + } + if (heads_.empty() && compactHeads_) { + if (a >= compactHeads_->size()) { + raise("ArrayArena head index out of range."); + } auto const& compact = (*compactHeads_)[a]; for (size_t i = 0; i < static_cast(compact.size); ++i) { @@ -377,6 +630,9 @@ class ArrayArena return; } + if (a >= heads_.size()) { + raise("ArrayArena head index out of range."); + } Chunk const* current = &heads_[a]; size_t globalIndex = 0; while (current != nullptr) @@ -418,12 +674,36 @@ class ArrayArena ModelColumn heads_; // Head chunks of all arrays. ModelColumn continuations_; // Continuation chunks of all arrays. ModelColumn data_; // Underlying element storage. + ModelColumn singletonValues_; + ModelColumn singletonOccupied_; std::optional compactHeads_; #ifdef ARRAY_ARENA_THREAD_SAFE mutable std::shared_mutex lock_; // Mutex for synchronizing access to the data structure during growth. #endif + static ArrayIndex to_array_index(size_t value) + { + if (value > std::numeric_limits::max()) { + raise("ArrayArena index exceeds address space."); + } + return static_cast(value); + } + + void ensure_regular_head_pool() + { + if (!heads_.empty()) { + return; + } + heads_.push_back({ + 0, + 0, + 0, + InvalidArrayIndex, + InvalidArrayIndex + }); + } + void ensure_runtime_heads_from_compact() { if (!heads_.empty() || !compactHeads_) @@ -441,6 +721,7 @@ class ArrayArena InvalidArrayIndex }); } + ensure_regular_head_pool(); } /** @@ -453,10 +734,15 @@ class ArrayArena * @param a The index of the array. * @return A reference to the last chunk of the array, after ensuring there's capacity. */ - Chunk& ensure_capacity_and_get_last_chunk(ArrayIndex const& a) + Chunk& ensure_capacity_and_get_last_chunk(ArrayIndex a) { + if (is_singleton_handle(a)) { + raise("Singleton handles do not use chunk growth."); + } + #ifndef ARRAY_ARENA_THREAD_SAFE ensure_runtime_heads_from_compact(); + ensure_regular_head_pool(); #endif #ifdef ARRAY_ARENA_THREAD_SAFE @@ -468,7 +754,17 @@ class ArrayArena write_guard.unlock(); read_guard.lock(); } + if (heads_.empty()) { + read_guard.unlock(); + std::unique_lock write_guard(lock_); + ensure_regular_head_pool(); + write_guard.unlock(); + read_guard.lock(); + } #endif + if (a >= heads_.size()) { + raise("ArrayArena head index out of range."); + } Chunk& head = heads_[a]; Chunk& last = (head.last == InvalidArrayIndex) ? head : continuations_[head.last]; if (last.size < last.capacity) @@ -485,7 +781,7 @@ class ArrayArena head.capacity = static_cast(newCapacity); return head; } - auto newIndex = static_cast(continuations_.size()); + auto newIndex = to_array_index(continuations_.size()); continuations_.push_back({(SizeType_)offset, (SizeType_)newCapacity, 0, InvalidArrayIndex, InvalidArrayIndex}); last.next = newIndex; head.last = newIndex; @@ -494,18 +790,37 @@ class ArrayArena template static tl::expected, Error> - at_impl(Self& self, ArrayIndex const& a, size_t const& i) + at_impl(Self& self, ArrayIndex a, size_t i) { #ifdef ARRAY_ARENA_THREAD_SAFE std::shared_lock guard(self.lock_); #endif + if (is_singleton_handle(a)) { + auto singletonIndex = singleton_payload(a); + if (singletonIndex >= self.singletonValues_.size() || + singletonIndex >= self.singletonOccupied_.size()) { + return tl::unexpected(Error::IndexOutOfRange, "singleton handle index out of range"); + } + if (self.singletonOccupied_.at(singletonIndex) == 0 || i > 0) { + return tl::unexpected(Error::IndexOutOfRange, "index out of range"); + } + return self.singletonValues_.at(singletonIndex); + } + if (self.heads_.empty() && self.compactHeads_) { + if (a >= self.compactHeads_->size()) { + return tl::unexpected(Error::IndexOutOfRange, "array index out of range"); + } auto const& compact = (*self.compactHeads_)[a]; if (i < static_cast(compact.size)) return self.data_[static_cast(compact.offset) + i]; return tl::unexpected(Error::IndexOutOfRange, "index out of range"); } + if (a >= self.heads_.size()) { + return tl::unexpected(Error::IndexOutOfRange, "array index out of range"); + } + typename Self::Chunk const* current = &self.heads_[a]; size_t remaining = i; while (true) { diff --git a/include/simfil/model/bitsery-traits.h b/include/simfil/model/bitsery-traits.h index d78f4228..ab813330 100644 --- a/include/simfil/model/bitsery-traits.h +++ b/include/simfil/model/bitsery-traits.h @@ -63,6 +63,8 @@ struct ArrayArenaExt if (arena.isCompact()) { s.object(*arena.compactHeads_); s.object(arena.data_); + s.object(arena.singletonValues_); + s.object(arena.singletonOccupied_); return; } @@ -77,7 +79,7 @@ struct ArrayArenaExt size_t totalElements = 0; for (auto const& head : arena.heads_) { - totalElements += static_cast(head.size); + totalElements += head.size; } compactData.resize(totalElements); @@ -90,26 +92,30 @@ struct ArrayArenaExt }); auto const* current = &head; - auto remaining = static_cast(head.size); + size_t remaining = head.size; while (current != nullptr && remaining > 0) { size_t chunkUsed = 0; if (current == &head) { - chunkUsed = std::min(static_cast(head.capacity), remaining); + chunkUsed = std::min(head.capacity, remaining); } else { - chunkUsed = std::min(static_cast(current->size), remaining); + chunkUsed = std::min(current->size, remaining); } for (size_t i = 0; i < chunkUsed; ++i) { compactData[writeIndex++] = arena.data_[current->offset + i]; } remaining -= chunkUsed; - current = (current->next != simfil::InvalidArrayIndex) ? &arena.continuations_[current->next] : nullptr; + current = (current->next != simfil::InvalidArrayIndex) + ? &arena.continuations_[static_cast(current->next)] + : nullptr; } - packedOffset += static_cast(head.size); + packedOffset += head.size; } s.object(compactHeads); s.object(compactData); + s.object(arena.singletonValues_); + s.object(arena.singletonOccupied_); } template @@ -121,10 +127,18 @@ struct ArrayArenaExt CompactHeadsStorage compactHeads; s.object(compactHeads); s.object(arena.data_); + s.object(arena.singletonValues_); + s.object(arena.singletonOccupied_); arena.heads_.clear(); arena.continuations_.clear(); arena.compactHeads_ = std::move(compactHeads); + if (arena.singletonOccupied_.size() < arena.singletonValues_.size()) { + auto const missing = arena.singletonValues_.size() - arena.singletonOccupied_.size(); + for (size_t i = 0; i < missing; ++i) { + arena.singletonOccupied_.emplace_back(static_cast(1)); + } + } } }; @@ -134,7 +148,7 @@ namespace traits { template struct ExtensionTraits { - using TValue = typename T::ElementType; + using TValue = void; static constexpr bool SupportValueOverload = true; static constexpr bool SupportObjectOverload = true; static constexpr bool SupportLambdaOverload = true; diff --git a/include/simfil/model/model.h b/include/simfil/model/model.h index 90e57f20..bc57fc4b 100644 --- a/include/simfil/model/model.h +++ b/include/simfil/model/model.h @@ -12,7 +12,6 @@ #include #include #include -#include #include #include #include @@ -305,7 +304,9 @@ class ModelPool : public Model * so derived ModelPools can create Object/Array-derived nodes. */ Object::Storage& objectMemberStorage(); + [[nodiscard]] Object::Storage const& objectMemberStorage() const; Array::Storage& arrayMemberStorage(); + [[nodiscard]] Array::Storage const& arrayMemberStorage() const; }; } diff --git a/include/simfil/model/nodes.h b/include/simfil/model/nodes.h index 2d79a6eb..af92bbf1 100644 --- a/include/simfil/model/nodes.h +++ b/include/simfil/model/nodes.h @@ -539,7 +539,7 @@ struct BaseArray : public MandatoryDerivedModelNodeBase using MandatoryDerivedModelNodeBase::model; Storage* storage_ = nullptr; - ArrayIndex members_ = 0; + ArrayIndex members_ = InvalidArrayIndex; }; /** Model Node for a mixed-type array. */ @@ -610,7 +610,7 @@ struct BaseObject : public MandatoryDerivedModelNodeBase addFieldInternal(std::string_view const& name, ModelNode::Ptr const& value={}); Storage* storage_ = nullptr; - ArrayIndex members_ = 0; + ArrayIndex members_ = InvalidArrayIndex; }; /** Model Node for an object. */ diff --git a/src/model/model.cpp b/src/model/model.cpp index c8c5a31d..58218456 100644 --- a/src/model/model.cpp +++ b/src/model/model.cpp @@ -121,7 +121,7 @@ std::vector ModelPool::checkForErrors() const std::vector errors; auto validateArrayIndex = [&](auto i, auto arrType, auto const& arena) { - if ((i < 0) || (i >= arena.size())) { + if (!arena.valid(static_cast(i))) { errors.emplace_back(fmt::format("Bad {} array index {}.", arrType, i)); return false; } @@ -447,10 +447,20 @@ Object::Storage& ModelPool::objectMemberStorage() { return impl_->columns_.objectMemberArrays_; } +Object::Storage const& ModelPool::objectMemberStorage() const +{ + return impl_->columns_.objectMemberArrays_; +} + Array::Storage& ModelPool::arrayMemberStorage() { return impl_->columns_.arrayMemberArrays_; } +Array::Storage const& ModelPool::arrayMemberStorage() const +{ + return impl_->columns_.arrayMemberArrays_; +} + tl::expected ModelPool::write(std::ostream& outputStream) { bitsery::Serializer s(outputStream); impl_->readWrite(s); diff --git a/test/arena.cpp b/test/arena.cpp index 12af9abe..f9e70338 100644 --- a/test/arena.cpp +++ b/test/arena.cpp @@ -16,8 +16,8 @@ TEST_CASE("ArrayArena basic functionality", "[ArrayArena]") { ArrayIndex array1 = arena.new_array(2); ArrayIndex array2 = arena.new_array(4); - REQUIRE(array1 == 0); - REQUIRE(array2 == 1); + REQUIRE(array1 == FirstRegularArrayIndex); + REQUIRE(array2 == FirstRegularArrayIndex + 1); } SECTION("size") { @@ -74,7 +74,7 @@ TEST_CASE("ArrayArena clear and shrink_to_fit", "[ArrayArena]") { SECTION("clear") { arena.clear(); ArrayIndex array2 = arena.new_array(2); - REQUIRE(array2 == 0); + REQUIRE(array2 == FirstRegularArrayIndex); REQUIRE(!arena.at(array1, 0)); REQUIRE(arena.at(array1, 0).error().type == Error::IndexOutOfRange); } @@ -98,31 +98,32 @@ TEST_CASE("ArrayArena multiple arrays", "[ArrayArena]") { }; // Interleave pushing array elements for maximum fragmentation + std::vector arrayIndices(expected.size(), InvalidArrayIndex); for (auto j = 0; j < expected[0].size(); j+=2) { for (auto i = 0; i < expected.size(); ++i) { if (j == 0) - arena.new_array(1); - arena.push_back(i, expected[i][j]); - arena.push_back(i, expected[i][j+1]); + arrayIndices[i] = arena.new_array(2); + arena.push_back(arrayIndices[i], expected[i][j]); + arena.push_back(arrayIndices[i], expected[i][j+1]); } } SECTION("accessing elements") { - REQUIRE(arena.at(0, 0) == 10); - REQUIRE(arena.at(0, 1) == 11); - REQUIRE(arena.at(0, 2) == 12); - REQUIRE(arena.at(1, 0) == 20); - REQUIRE(arena.at(1, 1) == 21); - REQUIRE(arena.at(1, 2) == 22); - REQUIRE(arena.at(1, 3) == 23); + REQUIRE(arena.at(arrayIndices[0], 0) == 10); + REQUIRE(arena.at(arrayIndices[0], 1) == 11); + REQUIRE(arena.at(arrayIndices[0], 2) == 12); + REQUIRE(arena.at(arrayIndices[1], 0) == 20); + REQUIRE(arena.at(arrayIndices[1], 1) == 21); + REQUIRE(arena.at(arrayIndices[1], 2) == 22); + REQUIRE(arena.at(arrayIndices[1], 3) == 23); } SECTION("range-based for loop for multiple arrays") { std::vector> result = {{}, {}}; - for (auto value : arena.range(0)) { + for (auto value : arena.range(arrayIndices[0])) { result[0].push_back(value); } - for (auto value : arena.range(1)) { + for (auto value : arena.range(arrayIndices[1])) { result[1].push_back(value); } REQUIRE(result[0] == expected[0]); @@ -144,7 +145,7 @@ TEST_CASE("ArrayArena multiple arrays", "[ArrayArena]") { TEST_CASE("ArrayArena::iterate") { ArrayArena arena; - ArrayIndex a = arena.new_array(1); + ArrayIndex a = arena.new_array(2); for (size_t i = 0; i < 10; ++i) { arena.push_back(a, static_cast(i*2)); } @@ -187,7 +188,7 @@ TEST_CASE("ArrayArena Concurrency", "[ArrayArena]") { auto thread_func = [&]() { // Random delay to increase the chances of concurrency issues std::this_thread::sleep_for(std::chrono::nanoseconds(rand() % 100)); // NOLINT (rand() is safe here) - auto array_index = arena.new_array(1); // Minimal initial capacity for maximal fragmentation + auto array_index = arena.new_array(2); // Minimal regular-array capacity for maximal fragmentation for (size_t i = 0; i < num_iterations; ++i) { arena.push_back(array_index, static_cast(i)); std::this_thread::sleep_for(std::chrono::nanoseconds(rand() % 100)); // NOLINT @@ -255,3 +256,42 @@ TEST_CASE("ArrayArena serialization and deserialization") { REQUIRE(arena.at(array2, i) == deserializedArena.at(array2, i)); } } + +TEST_CASE("ArrayArena singleton-handle storage", "[ArrayArena]") +{ + ArrayArena arena; + + auto emptySingleton = arena.new_array(1); + REQUIRE(ArrayArena::is_singleton_handle(emptySingleton)); + REQUIRE(arena.size(emptySingleton) == 0); + + arena.push_back(emptySingleton, 42); + REQUIRE(arena.size(emptySingleton) == 1); + REQUIRE(arena.at(emptySingleton, 0) == 42); + REQUIRE(!arena.at(emptySingleton, 1)); + + REQUIRE_THROWS(arena.push_back(emptySingleton, 43)); + + auto regular = arena.new_array(4); + REQUIRE(!ArrayArena::is_singleton_handle(regular)); + arena.push_back(regular, 1); + arena.push_back(regular, 2); + REQUIRE(arena.size(regular) == 2); + REQUIRE(arena.at(regular, 1) == 2); + + SECTION("iterating over all arrays includes singleton handles") + { + std::vector valuesByArray; + for (auto const& arr : arena) { + int value = -1; + if (arr.size() > 0) { + value = arr[0].value(); + } + valuesByArray.push_back(value); + } + + REQUIRE(valuesByArray.size() == 2); + REQUIRE(valuesByArray[0] == 1); + REQUIRE(valuesByArray[1] == 42); + } +} From d5313cc8ce6465302a77201a98e4b3c6377041ab Mon Sep 17 00:00:00 2001 From: Joseph Birkner Date: Wed, 4 Mar 2026 15:12:07 +0100 Subject: [PATCH 2/4] Add fixedSize array flag. --- include/simfil/model/arena.h | 5 +++-- include/simfil/model/model.h | 4 ++-- src/model/json.cpp | 4 ++-- src/model/model.cpp | 8 ++++---- test/arena.cpp | 18 ++++++++++++++---- 5 files changed, 25 insertions(+), 14 deletions(-) diff --git a/include/simfil/model/arena.h b/include/simfil/model/arena.h index cf386576..cf09c99d 100644 --- a/include/simfil/model/arena.h +++ b/include/simfil/model/arena.h @@ -104,16 +104,17 @@ class ArrayArena * Creates a new array with the specified initial capacity. * * @param initialCapacity The initial capacity of the new array. + * @param fixedSize If true, allows singleton-handle encoding when initialCapacity is 1. * @return The index of the new array. */ - ArrayIndex new_array(size_t initialCapacity) + ArrayIndex new_array(size_t initialCapacity, bool fixedSize = false) { #ifdef ARRAY_ARENA_THREAD_SAFE std::unique_lock guard(lock_); #endif ensure_runtime_heads_from_compact(); - if (initialCapacity == 1U) { + if (initialCapacity == 1U && fixedSize) { auto singletonIndex = to_array_index(singletonValues_.size()); if (singletonIndex > SingletonArrayHandlePayloadMask) { raise("ArrayArena singleton pool exhausted."); diff --git a/include/simfil/model/model.h b/include/simfil/model/model.h index bc57fc4b..31951867 100644 --- a/include/simfil/model/model.h +++ b/include/simfil/model/model.h @@ -235,13 +235,13 @@ class ModelPool : public Model * Adopt members from the given vector and obtain a new object * model index which has these members. */ - model_ptr newObject(size_t initialFieldCapacity = 2); + model_ptr newObject(size_t initialFieldCapacity = 2, bool fixedSize = false); /** * Adopt members from the given vector and obtain a new array * model index which has these members. */ - model_ptr newArray(size_t initialFieldCapacity = 2); + model_ptr newArray(size_t initialFieldCapacity = 2, bool fixedSize = false); /** Add a scalar value and get its new model node index. */ ModelNode::Ptr newValue(int64_t const& value); diff --git a/src/model/json.cpp b/src/model/json.cpp index 1cbe693a..199fc0c5 100644 --- a/src/model/json.cpp +++ b/src/model/json.cpp @@ -46,7 +46,7 @@ static auto build(const json& j, ModelPool & model) -> tl::expected tl::expectedcolumns_.roots_.emplace_back(rootNode->addr_); } -model_ptr ModelPool::newObject(size_t initialFieldCapacity) +model_ptr ModelPool::newObject(size_t initialFieldCapacity, bool fixedSize) { - auto memberArrId = impl_->columns_.objectMemberArrays_.new_array(initialFieldCapacity); + auto memberArrId = impl_->columns_.objectMemberArrays_.new_array(initialFieldCapacity, fixedSize); return model_ptr::make(shared_from_this(), ModelNodeAddress{Objects, (uint32_t)memberArrId}); } -model_ptr ModelPool::newArray(size_t initialFieldCapacity) +model_ptr ModelPool::newArray(size_t initialFieldCapacity, bool fixedSize) { - auto memberArrId = impl_->columns_.arrayMemberArrays_.new_array(initialFieldCapacity); + auto memberArrId = impl_->columns_.arrayMemberArrays_.new_array(initialFieldCapacity, fixedSize); return model_ptr::make(shared_from_this(), ModelNodeAddress{Arrays, (uint32_t)memberArrId}); } diff --git a/test/arena.cpp b/test/arena.cpp index f9e70338..4e31709e 100644 --- a/test/arena.cpp +++ b/test/arena.cpp @@ -261,7 +261,16 @@ TEST_CASE("ArrayArena singleton-handle storage", "[ArrayArena]") { ArrayArena arena; - auto emptySingleton = arena.new_array(1); + auto regularSingleCapacity = arena.new_array(1); + REQUIRE(!ArrayArena::is_singleton_handle(regularSingleCapacity)); + arena.push_back(regularSingleCapacity, 7); + REQUIRE(arena.size(regularSingleCapacity) == 1); + REQUIRE(arena.at(regularSingleCapacity, 0) == 7); + arena.push_back(regularSingleCapacity, 8); + REQUIRE(arena.size(regularSingleCapacity) == 2); + REQUIRE(arena.at(regularSingleCapacity, 1) == 8); + + auto emptySingleton = arena.new_array(1, true); REQUIRE(ArrayArena::is_singleton_handle(emptySingleton)); REQUIRE(arena.size(emptySingleton) == 0); @@ -290,8 +299,9 @@ TEST_CASE("ArrayArena singleton-handle storage", "[ArrayArena]") valuesByArray.push_back(value); } - REQUIRE(valuesByArray.size() == 2); - REQUIRE(valuesByArray[0] == 1); - REQUIRE(valuesByArray[1] == 42); + REQUIRE(valuesByArray.size() == 3); + REQUIRE(valuesByArray[0] == 7); + REQUIRE(valuesByArray[1] == 1); + REQUIRE(valuesByArray[2] == 42); } } From c324b299be9244a54a815e25aee2af65dd997cd7 Mon Sep 17 00:00:00 2001 From: Joseph Birkner Date: Thu, 5 Mar 2026 11:05:54 +0100 Subject: [PATCH 3/4] Add split TwoPart storage for object fields and array arenas --- include/simfil/model/arena.h | 115 +++++---- include/simfil/model/column.h | 408 ++++++++++++++++++++++++++++++ include/simfil/model/nodes.h | 31 ++- include/simfil/model/nodes.impl.h | 10 +- src/model/model.cpp | 6 +- test/arena.cpp | 76 ++++++ 6 files changed, 575 insertions(+), 71 deletions(-) diff --git a/include/simfil/model/arena.h b/include/simfil/model/arena.h index cf09c99d..78e032c2 100644 --- a/include/simfil/model/arena.h +++ b/include/simfil/model/arena.h @@ -12,6 +12,8 @@ #include #include #include +#include +#include #include #include "simfil/model/column.h" @@ -59,6 +61,12 @@ class ArrayArena public: using ElementType = ElementType_; using SizeType = SizeType_; + using DataStorage = ModelColumn; + using DataWriteRef = decltype(std::declval()[std::declval()]); + using DataReadRef = decltype(std::declval()[std::declval()]); + using AtValue = detail::arena_access_result_t; + using ConstAtValue = detail::arena_access_result_t; + struct SingletonStats { size_t handleCount = 0; @@ -175,7 +183,7 @@ class ArrayArena const auto singletonStorageBytes = singletonValues_.byte_size() + singletonOccupied_.byte_size(); const auto hypotheticalRegularBytes = - handleCount * sizeof(CompactArrayChunk) + occupiedCount * sizeof(ElementType_); + handleCount * sizeof(CompactArrayChunk) + occupiedCount * DataStorage::record_size; return SingletonStats{ .handleCount = handleCount, @@ -246,7 +254,7 @@ class ArrayArena } auto result = heads_.size() * sizeof(CompactArrayChunk); for (auto const& head : heads_) { - result += head.size * sizeof(ElementType_); + result += head.size * DataStorage::record_size; } return result + singletonBytes; } @@ -259,13 +267,13 @@ class ArrayArena * @return A reference to the element at the specified index. * @throws std::out_of_range if the index is out of the array bounds. */ - tl::expected, Error> + tl::expected at(ArrayIndex a, size_t i) { - return at_impl(*this, a, i); + return at_impl(*this, a, i); } - tl::expected, Error> + tl::expected at(ArrayIndex a, size_t i) const { - return at_impl(*this, a, i); + return at_impl(*this, a, i); } /** @@ -275,7 +283,7 @@ class ArrayArena * @param data The element to be appended. * @return A reference to the appended element. */ - ElementType_& push_back(ArrayIndex a, ElementType_ const& data) + DataWriteRef push_back(ArrayIndex a, ElementType_ const& data) { if (is_singleton_handle(a)) { #ifdef ARRAY_ARENA_THREAD_SAFE @@ -301,7 +309,7 @@ class ArrayArena #ifdef ARRAY_ARENA_THREAD_SAFE std::shared_lock guard(lock_); #endif - auto& elem = data_[updatedLast.offset + updatedLast.size]; + auto&& elem = data_[updatedLast.offset + updatedLast.size]; elem = data; ++heads_[a].size; if (&heads_[a] != &updatedLast) @@ -319,7 +327,7 @@ class ArrayArena * @return A reference to the appended element. */ template - ElementType_& emplace_back(ArrayIndex a, Args&&... args) + DataWriteRef emplace_back(ArrayIndex a, Args&&... args) { if (is_singleton_handle(a)) { #ifdef ARRAY_ARENA_THREAD_SAFE @@ -345,8 +353,8 @@ class ArrayArena #ifdef ARRAY_ARENA_THREAD_SAFE std::shared_lock guard(lock_); #endif - auto& elem = data_[updatedLast.offset + updatedLast.size]; - new (&elem) ElementType_(std::forward(args)...); + auto&& elem = data_[updatedLast.offset + updatedLast.size]; + elem = ElementType_(std::forward(args)...); ++heads_[a].size; if (&heads_[a] != &updatedLast) ++updatedLast.size; @@ -415,20 +423,21 @@ class ArrayArena template class ArrayIterator { using ArrayArenaRef = std::conditional_t; - using ElementRef = std::conditional_t; + using AtExpected = decltype(std::declval().at(std::declval(), std::declval())); + using ElementAccess = std::remove_cvref_t().value())>; friend class ArrayRange; public: using iterator_category = std::input_iterator_tag; using value_type = T; using difference_type = std::ptrdiff_t; - using pointer = value_type*; - using reference = ElementRef; + using pointer = void; + using reference = ElementAccess; ArrayIterator(ArrayArenaRef arena, ArrayIndex array_index, size_t elem_index) : arena_(arena), array_index_(array_index), elem_index_(elem_index) {} - ElementRef operator*() noexcept { + reference operator*() noexcept { auto res = arena_.at(array_index_, elem_index_); assert(res); // Unchecked access! @@ -598,14 +607,9 @@ class ArrayArena if (singletonOccupied_.at(singletonIndex) == 0) { return; } - if constexpr (std::is_invocable_r_v) { - (void)lambda(singletonValues_.at(singletonIndex)); - } - else if constexpr (std::is_invocable_v) { - lambda(singletonValues_.at(singletonIndex), 0); - } - else { - lambda(singletonValues_.at(singletonIndex)); + decltype(auto) value = singletonValues_.at(singletonIndex); + if (!invoke_iter_callback(lambda, value, 0)) { + return; } return; } @@ -617,15 +621,9 @@ class ArrayArena auto const& compact = (*compactHeads_)[a]; for (size_t i = 0; i < static_cast(compact.size); ++i) { - if constexpr (std::is_invocable_r_v) { - if (!lambda(data_[static_cast(compact.offset) + i])) - return; - } - else if constexpr (std::is_invocable_v) { - lambda(data_[static_cast(compact.offset) + i], i); - } - else { - lambda(data_[static_cast(compact.offset) + i]); + decltype(auto) value = data_[static_cast(compact.offset) + i]; + if (!invoke_iter_callback(lambda, value, i)) { + return; } } return; @@ -640,17 +638,10 @@ class ArrayArena { for (size_t i = 0; i < current->size && i < current->capacity; ++i) { - if constexpr (std::is_invocable_r_v) { - // If lambda returns bool, break if it returns false - if (!lambda(data_[current->offset + i])) - return; - } - else if constexpr (std::is_invocable_v) { - // If lambda takes two arguments, pass the current index - lambda(data_[current->offset + i], globalIndex); + decltype(auto) value = data_[current->offset + i]; + if (!invoke_iter_callback(lambda, value, globalIndex)) { + return; } - else - lambda(data_[current->offset + i]); ++globalIndex; } current = (current->next != InvalidArrayIndex) ? &continuations_[current->next] : nullptr; @@ -674,8 +665,8 @@ class ArrayArena ModelColumn heads_; // Head chunks of all arrays. ModelColumn continuations_; // Continuation chunks of all arrays. - ModelColumn data_; // Underlying element storage. - ModelColumn singletonValues_; + DataStorage data_; // Underlying element storage. + DataStorage singletonValues_; ModelColumn singletonOccupied_; std::optional compactHeads_; @@ -691,6 +682,26 @@ class ArrayArena return static_cast(value); } + template + static bool invoke_iter_callback(Func&& lambda, Value&& value, size_t index) + { + using Arg = decltype(value); + if constexpr (std::is_invocable_r_v) { + return lambda(std::forward(value)); + } else if constexpr (std::is_invocable_v) { + lambda(std::forward(value), index); + return true; + } else if constexpr (std::is_invocable_v) { + lambda(std::forward(value)); + return true; + } else { + static_assert( + std::is_invocable_v, + "ArrayArena::iterate callback must accept (value) or (value, index), optionally returning bool"); + return false; + } + } + void ensure_regular_head_pool() { if (!heads_.empty()) { @@ -789,8 +800,8 @@ class ArrayArena return continuations_[newIndex]; } - template - static tl::expected, Error> + template + static tl::expected at_impl(Self& self, ArrayIndex a, size_t i) { #ifdef ARRAY_ARENA_THREAD_SAFE @@ -805,7 +816,7 @@ class ArrayArena if (self.singletonOccupied_.at(singletonIndex) == 0 || i > 0) { return tl::unexpected(Error::IndexOutOfRange, "index out of range"); } - return self.singletonValues_.at(singletonIndex); + return detail::arena_access_wrap(self.singletonValues_.at(singletonIndex)); } if (self.heads_.empty() && self.compactHeads_) { @@ -813,8 +824,9 @@ class ArrayArena return tl::unexpected(Error::IndexOutOfRange, "array index out of range"); } auto const& compact = (*self.compactHeads_)[a]; - if (i < static_cast(compact.size)) - return self.data_[static_cast(compact.offset) + i]; + if (i < static_cast(compact.size)) { + return detail::arena_access_wrap(self.data_[static_cast(compact.offset) + i]); + } return tl::unexpected(Error::IndexOutOfRange, "index out of range"); } @@ -825,8 +837,9 @@ class ArrayArena typename Self::Chunk const* current = &self.heads_[a]; size_t remaining = i; while (true) { - if (remaining < current->capacity && remaining < current->size) - return self.data_[current->offset + remaining]; + if (remaining < current->capacity && remaining < current->size) { + return detail::arena_access_wrap(self.data_[current->offset + remaining]); + } if (current->next == InvalidArrayIndex) return tl::unexpected(Error::IndexOutOfRange, "index out of range"); remaining -= current->capacity; diff --git a/include/simfil/model/column.h b/include/simfil/model/column.h index 382ab86e..b64ef970 100644 --- a/include/simfil/model/column.h +++ b/include/simfil/model/column.h @@ -8,6 +8,8 @@ #include #include #include +#include +#include #include #include #include @@ -37,9 +39,47 @@ enum class model_column_io_error static constexpr std::size_t model_column_expected_size = expected_size #endif +template +struct TwoPart +{ + using first_type = std::remove_cv_t; + using second_type = std::remove_cv_t; + + first_type first_{}; + second_type second_{}; + + TwoPart() = default; + TwoPart(first_type const& first, second_type const& second) + : first_(first), second_(second) + { + } + TwoPart(first_type&& first, second_type&& second) + : first_(std::move(first)), second_(std::move(second)) + { + } + + [[nodiscard]] first_type& first() noexcept { return first_; } + [[nodiscard]] first_type const& first() const noexcept { return first_; } + [[nodiscard]] second_type& second() noexcept { return second_; } + [[nodiscard]] second_type const& second() const noexcept { return second_; } + + bool operator==(TwoPart const&) const = default; +}; + namespace detail { +template +struct is_two_part : std::false_type +{}; + +template +struct is_two_part> : std::true_type +{}; + +template +concept two_part_type = is_two_part>::value; + template struct has_model_column_tag_trait : std::false_type {}; @@ -134,6 +174,23 @@ concept vector_storage_policy = typename T_StoragePolicy::type, std::vector>; +template +auto arena_access_wrap(TValue&& value) +{ + if constexpr (std::is_lvalue_reference_v) { + if constexpr (std::is_const_v>) { + return std::cref(value); + } else { + return std::ref(value); + } + } else { + return std::forward(value); + } +} + +template +using arena_access_result_t = decltype(arena_access_wrap(std::declval())); + } // namespace detail template @@ -421,6 +478,357 @@ class ModelColumn storage_type values_; }; +template < + typename TFirst, + typename TSecond, + std::size_t T_RecordsPerPage, + template typename T_StoragePolicy> +class ModelColumn, T_RecordsPerPage, T_StoragePolicy> +{ +public: + using value_type = TwoPart; + using first_type = typename value_type::first_type; + using second_type = typename value_type::second_type; + using first_column_type = + ModelColumn; + using second_column_type = + ModelColumn; + + static constexpr std::size_t record_size = + sizeof(first_type) + sizeof(second_type); + static constexpr std::size_t expected_record_size = record_size; + + static constexpr std::size_t page_bytes = T_RecordsPerPage * record_size; + static constexpr std::size_t records_per_page = T_RecordsPerPage; + static constexpr std::size_t page_size_bytes = page_bytes; + + template + class basic_ref + { + public: + using first_ref = std::conditional_t; + using second_ref = std::conditional_t; + + basic_ref(first_ref first, second_ref second) + : first_(first), second_(second) + { + } + + [[nodiscard]] first_ref first() const noexcept { return first_; } + [[nodiscard]] second_ref second() const noexcept { return second_; } + [[nodiscard]] operator value_type() const { return value_type{first_, second_}; } + + [[nodiscard]] bool operator==(value_type const& other) const + { + return first_ == other.first_ && second_ == other.second_; + } + + template + std::enable_if_t + operator=(value_type const& value) + { + first_ = value.first_; + second_ = value.second_; + return *this; + } + + template + std::enable_if_t + operator=(basic_ref const& other) + { + first_ = other.first_; + second_ = other.second_; + return *this; + } + + private: + first_ref first_; + second_ref second_; + }; + + using ref = basic_ref; + using const_ref = basic_ref; + + template + class basic_iterator + { + friend class basic_iterator; + friend class ModelColumn; + + using owner_type = std::conditional_t; + + public: + using iterator_category = std::forward_iterator_tag; + using value_type = ModelColumn::value_type; + using difference_type = std::ptrdiff_t; + using reference = std::conditional_t; + using pointer = void; + + basic_iterator() = default; + + basic_iterator(owner_type* owner, std::size_t index) + : owner_(owner), index_(index) + { + } + + template + basic_iterator(basic_iterator const& other) + requires(T_Enable) + : owner_(other.owner_), index_(other.index_) + { + } + + reference operator*() const { return (*owner_)[index_]; } + + basic_iterator& operator++() + { + ++index_; + return *this; + } + + basic_iterator operator++(int) + { + auto copy = *this; + ++(*this); + return copy; + } + + bool operator==(basic_iterator const& other) const + { + return owner_ == other.owner_ && index_ == other.index_; + } + + bool operator!=(basic_iterator const& other) const + { + return !(*this == other); + } + + private: + owner_type* owner_ = nullptr; + std::size_t index_ = 0; + }; + + using iterator = basic_iterator; + using const_iterator = basic_iterator; + + ModelColumn() = default; + + [[nodiscard]] std::size_t size() const + { + assert(first_values_.size() == second_values_.size()); + return first_values_.size(); + } + + [[nodiscard]] std::size_t byte_size() const + { + return first_values_.byte_size() + second_values_.byte_size(); + } + + [[nodiscard]] bool empty() const { return size() == 0; } + + void clear() + { + first_values_.clear(); + second_values_.clear(); + } + + void reserve(std::size_t count) + { + first_values_.reserve(count); + second_values_.reserve(count); + } + + void resize(std::size_t count) + { + first_values_.resize(count); + second_values_.resize(count); + } + + void shrink_to_fit() + { + first_values_.shrink_to_fit(); + second_values_.shrink_to_fit(); + } + + template + ref emplace_back(Args&&... args) + { + value_type value(std::forward(args)...); + first_values_.push_back(value.first_); + second_values_.push_back(value.second_); + return back(); + } + + template + ref emplace(Args&&... args) + { + return emplace_back(std::forward(args)...); + } + + void push_back(value_type const& value) + { + first_values_.push_back(value.first_); + second_values_.push_back(value.second_); + } + + void push_back(value_type&& value) + { + first_values_.push_back(std::move(value.first_)); + second_values_.push_back(std::move(value.second_)); + } + + ref operator[](std::size_t index) + { + return ref(first_values_[index], second_values_[index]); + } + + const_ref operator[](std::size_t index) const + { + return const_ref(first_values_[index], second_values_[index]); + } + + ref at(std::size_t index) + { + return ref(first_values_.at(index), second_values_.at(index)); + } + + const_ref at(std::size_t index) const + { + return const_ref(first_values_.at(index), second_values_.at(index)); + } + + ref back() + { + return ref(first_values_.back(), second_values_.back()); + } + + const_ref back() const + { + return const_ref(first_values_.back(), second_values_.back()); + } + + iterator begin() { return iterator(this, 0); } + const_iterator begin() const { return const_iterator(this, 0); } + const_iterator cbegin() const { return const_iterator(this, 0); } + + iterator end() { return iterator(this, size()); } + const_iterator end() const { return const_iterator(this, size()); } + const_iterator cend() const { return const_iterator(this, size()); } + + template + iterator insert(const_iterator pos, InputIt first, InputIt last) + { + const auto insert_index = pos.index_; + std::vector first_parts; + std::vector second_parts; + for (auto it = first; it != last; ++it) { + value_type value = *it; + first_parts.push_back(value.first_); + second_parts.push_back(value.second_); + } + + auto first_pos = first_values_.begin(); + auto second_pos = second_values_.begin(); + std::advance(first_pos, static_cast(insert_index)); + std::advance(second_pos, static_cast(insert_index)); + first_values_.insert(first_pos, first_parts.begin(), first_parts.end()); + second_values_.insert(second_pos, second_parts.begin(), second_parts.end()); + return iterator(this, insert_index); + } + + std::vector bytes() const + { + const auto first_payload = first_values_.bytes(); + const auto second_payload = second_values_.bytes(); + + std::vector out(first_payload.size() + second_payload.size()); + if (!first_payload.empty()) { + std::memcpy(out.data(), first_payload.data(), first_payload.size()); + } + if (!second_payload.empty()) { + std::memcpy( + out.data() + first_payload.size(), + second_payload.data(), + second_payload.size()); + } + return out; + } + + tl::expected + assign_bytes(std::span payload) + { + return assign_bytes_impl(payload); + } + + tl::expected + assign_bytes(std::span payload) + { + return assign_bytes_impl(payload); + } + + template + bool read_payload_from_bitsery( + T_BitseryInputAdapter& adapter, + std::size_t payload_size) + { + std::vector payload(payload_size); + if (payload_size > 0) { + adapter.template readBuffer<1>(payload.data(), payload_size); + if (adapter.error() != bitsery::ReaderError::NoError) { + clear(); + return false; + } + } + + if (!assign_bytes(std::span(payload.data(), payload.size()))) { + clear(); + return false; + } + return true; + } + +private: + template + tl::expected + assign_bytes_impl(std::span payload) + { + static_assert( + sizeof(ByteType) == 1, + "assign_bytes expects 1-byte payload elements"); + + if ((payload.size() % record_size) != 0U) { + return tl::make_unexpected(model_column_io_error::payload_size_mismatch); + } + + const auto record_count = payload.size() / record_size; + const auto first_payload_size = record_count * sizeof(first_type); + const auto second_payload_size = record_count * sizeof(second_type); + + auto const* payload_ptr = reinterpret_cast(payload.data()); + auto const first_payload = std::span(payload_ptr, first_payload_size); + auto const second_payload = std::span( + payload_ptr + first_payload_size, + second_payload_size); + + auto first_assign = first_values_.assign_bytes(first_payload); + if (!first_assign) { + clear(); + return tl::make_unexpected(first_assign.error()); + } + + auto second_assign = second_values_.assign_bytes(second_payload); + if (!second_assign) { + clear(); + return tl::make_unexpected(second_assign.error()); + } + + return {}; + } + + first_column_type first_values_; + second_column_type second_values_; +}; + } // namespace simfil namespace bitsery diff --git a/include/simfil/model/nodes.h b/include/simfil/model/nodes.h index af92bbf1..c655cb75 100644 --- a/include/simfil/model/nodes.h +++ b/include/simfil/model/nodes.h @@ -5,6 +5,7 @@ #include #include #include +#include #include "arena.h" #include "string-pool.h" @@ -215,21 +216,27 @@ namespace detail { // Shared storage entry for object fields across all BaseObject instantiations. // Keeps the underlying ArrayArena type identical regardless of ModelType. -struct ObjectField -{ - MODEL_COLUMN_TYPE(8); +using ObjectField = TwoPart; - ObjectField() = default; - ObjectField(StringId name, ModelNodeAddress a) : name_(name), node_(a) {} - StringId name_ = StringPool::Empty; - ModelNodeAddress node_; +template +decltype(auto) objectFieldName(TField&& field) +{ + if constexpr (requires { std::forward(field).get(); }) { + return objectFieldName(std::forward(field).get()); + } else { + return std::forward(field).first(); + } +} - template - void serialize(S& s) { - s.value2b(name_); - s.object(node_); +template +decltype(auto) objectFieldNode(TField&& field) +{ + if constexpr (requires { std::forward(field).get(); }) { + return objectFieldNode(std::forward(field).get()); + } else { + return std::forward(field).second(); } -}; +} } /** Semantic view onto a particular node in a ModelPool. */ diff --git a/include/simfil/model/nodes.impl.h b/include/simfil/model/nodes.impl.h index ee64b1bb..d8a08e7e 100644 --- a/include/simfil/model/nodes.impl.h +++ b/include/simfil/model/nodes.impl.h @@ -102,7 +102,7 @@ ModelNode::Ptr BaseObject::at(int64_t i) const if (i < 0 || i >= (int64_t)storage_->size(members_)) return {}; if (auto value = storage_->at(members_, i); value) - return ModelNode::Ptr::make(model_, value->get().node_); + return ModelNode::Ptr::make(model_, detail::objectFieldNode(value.value())); return {}; } @@ -112,7 +112,7 @@ StringId BaseObject::keyAt(int64_t i) const if (i < 0 || i >= (int64_t)storage_->size(members_)) return {}; if (auto value = storage_->at(members_, i); value) - return value->get().name_; + return detail::objectFieldName(value.value()); return {}; } @@ -130,8 +130,8 @@ ModelNode::Ptr BaseObject::get(const StringId& field) members_, [&field, &result, this](auto&& member) { - if (member.name_ == field) { - result = ModelNode::Ptr::make(model_, member.node_); + if (detail::objectFieldName(member) == field) { + result = ModelNode::Ptr::make(model_, detail::objectFieldNode(member)); return false; } return true; @@ -148,7 +148,7 @@ bool BaseObject::iterate(const ModelNode::IterCallback members_, [&, this](auto&& member) { - (*model_).resolve(*ModelNode::Ptr::make(model_, member.node_), resolveAndCb); + (*model_).resolve(*ModelNode::Ptr::make(model_, detail::objectFieldNode(member)), resolveAndCb); return cont; }); return cont; diff --git a/src/model/model.cpp b/src/model/model.cpp index 0a0a0da9..6b1ad9a1 100644 --- a/src/model/model.cpp +++ b/src/model/model.cpp @@ -412,11 +412,11 @@ auto ModelPool::setStrings(std::shared_ptr const& strings) -> tl::ex // Translate object field IDs to the new dictionary. for (auto memberArray : impl_->columns_.objectMemberArrays_) { - for (auto& member : memberArray) { - if (auto resolvedName = oldStrings->resolve(member.name_)) { + for (auto member : memberArray) { + if (auto resolvedName = oldStrings->resolve(detail::objectFieldName(member))) { auto stringId = strings->emplace(*resolvedName); TRY_EXPECTED(stringId); - member.name_ = *stringId; + detail::objectFieldName(member) = *stringId; } } } diff --git a/test/arena.cpp b/test/arena.cpp index 4e31709e..457d796d 100644 --- a/test/arena.cpp +++ b/test/arena.cpp @@ -305,3 +305,79 @@ TEST_CASE("ArrayArena singleton-handle storage", "[ArrayArena]") REQUIRE(valuesByArray[2] == 42); } } + +TEST_CASE("ModelColumn supports split TwoPart storage", "[ModelColumn][TwoPart]") +{ + using Pair = TwoPart; + ModelColumn column; + + column.emplace_back(10u, 100u); + column.emplace_back(11u, 200u); + column.push_back(Pair{12u, 300u}); + + REQUIRE(column.size() == 3); + REQUIRE(column.byte_size() == 3 * (sizeof(uint16_t) + sizeof(uint32_t))); + REQUIRE(column.at(0).first() == 10u); + REQUIRE(column.at(0).second() == 100u); + REQUIRE(column.at(2).first() == 12u); + REQUIRE(column.at(2).second() == 300u); + + using Buffer = std::vector; + using OutputAdapter = bitsery::OutputBufferAdapter; + using InputAdapter = bitsery::InputBufferAdapter; + Buffer buffer; + auto writtenSize = bitsery::quickSerialization(buffer, column); + + ModelColumn restored; + auto state = bitsery::quickDeserialization( + {buffer.begin(), writtenSize}, + restored); + + REQUIRE(state.first == bitsery::ReaderError::NoError); + REQUIRE(state.second); + REQUIRE(restored.size() == column.size()); + REQUIRE(restored.at(0).first() == 10u); + REQUIRE(restored.at(0).second() == 100u); + REQUIRE(restored.at(1).first() == 11u); + REQUIRE(restored.at(1).second() == 200u); + REQUIRE(restored.at(2).first() == 12u); + REQUIRE(restored.at(2).second() == 300u); +} + +TEST_CASE("ArrayArena supports TwoPart element storage", "[ArrayArena][TwoPart]") +{ + using Pair = TwoPart; + ArrayArena arena; + + auto regular = arena.new_array(2); + arena.emplace_back(regular, 1u, 10u); + arena.emplace_back(regular, 2u, 20u); + arena.emplace_back(regular, 3u, 30u); + + REQUIRE(arena.size(regular) == 3); + auto first = arena.at(regular, 0); + REQUIRE(first); + REQUIRE(first->first() == 1u); + REQUIRE(first->second() == 10u); + + auto third = arena.at(regular, 2); + REQUIRE(third); + REQUIRE(third->first() == 3u); + REQUIRE(third->second() == 30u); + + auto singleton = arena.new_array(1, true); + arena.push_back(singleton, Pair{7u, 70u}); + auto singletonValue = arena.at(singleton, 0); + REQUIRE(singletonValue); + REQUIRE(singletonValue->first() == 7u); + REQUIRE(singletonValue->second() == 70u); + + size_t visited = 0; + arena.iterate(regular, [&](auto&& value, size_t index) { + REQUIRE(value.first() == static_cast(index + 1)); + REQUIRE(value.second() == static_cast((index + 1) * 10)); + ++visited; + return true; + }); + REQUIRE(visited == 3); +} From 9a3911b8af876abf359f2f49e4461479b23837c4 Mon Sep 17 00:00:00 2001 From: Joseph Birkner Date: Mon, 9 Mar 2026 17:50:37 +0100 Subject: [PATCH 4/4] model: address split storage review comments --- include/simfil/model/arena.h | 46 ++++++--------------------- include/simfil/model/bitsery-traits.h | 2 +- include/simfil/model/column.h | 12 +++---- 3 files changed, 17 insertions(+), 43 deletions(-) diff --git a/include/simfil/model/arena.h b/include/simfil/model/arena.h index 78e032c2..34f7f3cf 100644 --- a/include/simfil/model/arena.h +++ b/include/simfil/model/arena.h @@ -48,7 +48,8 @@ constexpr static ArrayIndex SingletonArrayHandlePayloadMask = 0x007fffffu; * forward-linked array chunks. When an array grows beyond the current capacity c * of its current last chunk, a new chunk of size c*2 is allocated and becomes * the new last chunk. This is then set as linked to the previous last chunk. - * Usually, appending will be lock-free, and only growth needs the lock. + * Without ARRAY_ARENA_THREAD_SAFE, appending is lock-free. With it enabled, + * reads use shared locks while mutations take a write lock. * * @tparam ElementType_ The type of elements stored in the arrays. * @tparam PageSize The number of elements that each storage page can store. @@ -305,11 +306,11 @@ class ArrayArena return singletonValues_.at(singletonIndex); } - Chunk& updatedLast = ensure_capacity_and_get_last_chunk(a); #ifdef ARRAY_ARENA_THREAD_SAFE - std::shared_lock guard(lock_); + std::unique_lock guard(lock_); #endif - auto&& elem = data_[updatedLast.offset + updatedLast.size]; + Chunk& updatedLast = ensure_capacity_and_get_last_chunk_unlocked(a); + DataWriteRef elem = data_[updatedLast.offset + updatedLast.size]; elem = data; ++heads_[a].size; if (&heads_[a] != &updatedLast) @@ -349,11 +350,11 @@ class ArrayArena return singletonValues_.at(singletonIndex); } - Chunk& updatedLast = ensure_capacity_and_get_last_chunk(a); #ifdef ARRAY_ARENA_THREAD_SAFE - std::shared_lock guard(lock_); + std::unique_lock guard(lock_); #endif - auto&& elem = data_[updatedLast.offset + updatedLast.size]; + Chunk& updatedLast = ensure_capacity_and_get_last_chunk_unlocked(a); + DataWriteRef elem = data_[updatedLast.offset + updatedLast.size]; elem = ElementType_(std::forward(args)...); ++heads_[a].size; if (&heads_[a] != &updatedLast) @@ -409,10 +410,6 @@ class ArrayArena return compactHeads_.has_value(); } - [[nodiscard]] bool isCompact() const { - return is_compact(); - } - // Iterator-related types and functions template class ArrayIterator; @@ -746,34 +743,15 @@ class ArrayArena * @param a The index of the array. * @return A reference to the last chunk of the array, after ensuring there's capacity. */ - Chunk& ensure_capacity_and_get_last_chunk(ArrayIndex a) + // Caller must hold the write lock when ARRAY_ARENA_THREAD_SAFE is enabled. + Chunk& ensure_capacity_and_get_last_chunk_unlocked(ArrayIndex a) { if (is_singleton_handle(a)) { raise("Singleton handles do not use chunk growth."); } - #ifndef ARRAY_ARENA_THREAD_SAFE ensure_runtime_heads_from_compact(); ensure_regular_head_pool(); - #endif - - #ifdef ARRAY_ARENA_THREAD_SAFE - std::shared_lock read_guard(lock_); - if (heads_.empty() && compactHeads_) { - read_guard.unlock(); - std::unique_lock write_guard(lock_); - ensure_runtime_heads_from_compact(); - write_guard.unlock(); - read_guard.lock(); - } - if (heads_.empty()) { - read_guard.unlock(); - std::unique_lock write_guard(lock_); - ensure_regular_head_pool(); - write_guard.unlock(); - read_guard.lock(); - } - #endif if (a >= heads_.size()) { raise("ArrayArena head index out of range."); } @@ -781,10 +759,6 @@ class ArrayArena Chunk& last = (head.last == InvalidArrayIndex) ? head : continuations_[head.last]; if (last.size < last.capacity) return last; - #ifdef ARRAY_ARENA_THREAD_SAFE - read_guard.unlock(); - std::unique_lock guard(lock_); - #endif size_t offset = data_.size(); size_t newCapacity = std::max((SizeType_)2, (SizeType_)last.capacity * 2); data_.resize(offset + newCapacity); diff --git a/include/simfil/model/bitsery-traits.h b/include/simfil/model/bitsery-traits.h index ab813330..e3b39e60 100644 --- a/include/simfil/model/bitsery-traits.h +++ b/include/simfil/model/bitsery-traits.h @@ -60,7 +60,7 @@ struct ArrayArenaExt (void)fnc; // If the arena is already compact, we can simply dump out heads and data - if (arena.isCompact()) { + if (arena.is_compact()) { s.object(*arena.compactHeads_); s.object(arena.data_); s.object(arena.singletonValues_); diff --git a/include/simfil/model/column.h b/include/simfil/model/column.h index b64ef970..e08f99b1 100644 --- a/include/simfil/model/column.h +++ b/include/simfil/model/column.h @@ -49,12 +49,12 @@ struct TwoPart second_type second_{}; TwoPart() = default; - TwoPart(first_type const& first, second_type const& second) - : first_(first), second_(second) - { - } - TwoPart(first_type&& first, second_type&& second) - : first_(std::move(first)), second_(std::move(second)) + + template + requires std::constructible_from && + std::constructible_from + TwoPart(A&& first, B&& second) + : first_(std::forward(first)), second_(std::forward(second)) { }