From 79f09941b82f550e5067f2550640aa9cdc1d4383 Mon Sep 17 00:00:00 2001 From: wvpm <24685035+wvpm@users.noreply.github.com> Date: Sun, 17 May 2026 15:09:55 +0200 Subject: [PATCH 1/2] Minimise allocations for ModifierSum --- .../country/CountryInstance.cpp | 4 + .../country/CountryInstance.hpp | 1 + src/openvic-simulation/map/MapInstance.cpp | 4 + .../map/ProvinceInstance.cpp | 6 ++ .../map/ProvinceInstance.hpp | 1 + .../modifier/ModifierSum.cpp | 41 +++---- .../modifier/ModifierSum.hpp | 100 ++++++++++++++++-- 7 files changed, 122 insertions(+), 35 deletions(-) diff --git a/src/openvic-simulation/country/CountryInstance.cpp b/src/openvic-simulation/country/CountryInstance.cpp index 818610c0f..2f2105903 100644 --- a/src/openvic-simulation/country/CountryInstance.cpp +++ b/src/openvic-simulation/country/CountryInstance.cpp @@ -1841,6 +1841,10 @@ void CountryInstance::update_modifier_sum(Date today, StaticModifierCache const& // TODO - calculate stats for each unit type (locked and unlocked) } +void CountryInstance::make_room_for_province_modifier_sum(ModifierSum const& province_modifier_sum) { + modifier_sum.make_room_for(province_modifier_sum); +} + void CountryInstance::contribute_province_modifier_sum(ModifierSum const& province_modifier_sum) { modifier_sum.add_modifier_sum(province_modifier_sum); } diff --git a/src/openvic-simulation/country/CountryInstance.hpp b/src/openvic-simulation/country/CountryInstance.hpp index 655e73138..0517e2f47 100644 --- a/src/openvic-simulation/country/CountryInstance.hpp +++ b/src/openvic-simulation/country/CountryInstance.hpp @@ -655,6 +655,7 @@ namespace OpenVic { public: void update_modifier_sum(Date today, StaticModifierCache const& static_modifier_cache); + void make_room_for_province_modifier_sum(ModifierSum const& province_modifier_sum); void contribute_province_modifier_sum(ModifierSum const& province_modifier_sum); fixed_point_t get_modifier_effect_value(ModifierEffect const& effect) const; constexpr void for_each_contributing_modifier( diff --git a/src/openvic-simulation/map/MapInstance.cpp b/src/openvic-simulation/map/MapInstance.cpp index 4b864e85b..bb126c24d 100644 --- a/src/openvic-simulation/map/MapInstance.cpp +++ b/src/openvic-simulation/map/MapInstance.cpp @@ -143,6 +143,10 @@ void MapInstance::update_modifier_sums(const Date today, StaticModifierCache con for (ProvinceInstance& province : get_province_instances()) { province.update_modifier_sum(today, static_modifier_cache); } + + for (ProvinceInstance& province : get_province_instances()) { + province.update_country_modifier_sum(); + } } void MapInstance::update_gamestate(InstanceManager const& instance_manager) { diff --git a/src/openvic-simulation/map/ProvinceInstance.cpp b/src/openvic-simulation/map/ProvinceInstance.cpp index e7bc0bd60..1f4c93c12 100644 --- a/src/openvic-simulation/map/ProvinceInstance.cpp +++ b/src/openvic-simulation/map/ProvinceInstance.cpp @@ -288,6 +288,12 @@ void ProvinceInstance::update_modifier_sum(Date today, StaticModifierCache const modifier_sum.add_modifier(*crime); } + if (controller != nullptr) { + controller->make_room_for_province_modifier_sum(modifier_sum); + } +} + +void ProvinceInstance::update_country_modifier_sum() { if (controller != nullptr) { controller->contribute_province_modifier_sum(modifier_sum); } diff --git a/src/openvic-simulation/map/ProvinceInstance.hpp b/src/openvic-simulation/map/ProvinceInstance.hpp index f280155c7..b3ffc3dfa 100644 --- a/src/openvic-simulation/map/ProvinceInstance.hpp +++ b/src/openvic-simulation/map/ProvinceInstance.hpp @@ -184,6 +184,7 @@ namespace OpenVic { size_t get_pop_count() const; void update_modifier_sum(Date today, StaticModifierCache const& static_modifier_cache); + void update_country_modifier_sum(); fixed_point_t get_modifier_effect_value(ModifierEffect const& effect) const; void for_each_contributing_modifier(ModifierEffect const& effect, ContributingModifierCallback auto callback) const { diff --git a/src/openvic-simulation/modifier/ModifierSum.cpp b/src/openvic-simulation/modifier/ModifierSum.cpp index 1c4daba47..c0c6518ae 100644 --- a/src/openvic-simulation/modifier/ModifierSum.cpp +++ b/src/openvic-simulation/modifier/ModifierSum.cpp @@ -1,17 +1,20 @@ #include "ModifierSum.hpp" -#include "openvic-simulation/modifier/Modifier.hpp" +#include -#include "openvic-simulation/country/CountryInstance.hpp" -#include "openvic-simulation/map/ProvinceInstance.hpp" #include "openvic-simulation/core/template/Concepts.hpp" +#include "openvic-simulation/country/CountryInstance.hpp" // IWYU pragma: keep for modifier_source_t +#include "openvic-simulation/map/ProvinceInstance.hpp" // IWYU pragma: keep for modifier_source_t +#include "openvic-simulation/modifier/Modifier.hpp" using namespace OpenVic; std::string_view modifier_entry_t::source_to_string(modifier_source_t const& source) { return std::visit( [](has_get_identifier auto const* has_identifier) -> std::string_view { - return has_identifier->get_identifier(); + return has_identifier == nullptr + ? "" + : has_identifier->get_identifier(); }, source ); @@ -20,20 +23,20 @@ std::string_view modifier_entry_t::source_to_string(modifier_source_t const& sou memory::string modifier_entry_t::to_string() const { return memory::fmt::format( "[{}, {}, {}, {}]", - modifier, multiplier, source_to_string(source), + ovfmt::validate(modifier), + multiplier, + source_to_string(source), ModifierEffect::target_to_string(excluded_targets) ); } void ModifierSum::clear() { + extra_size = 0; + valid_size = 0; modifiers.clear(); value_sum.clear(); } -bool ModifierSum::empty() { - return modifiers.empty(); -} - fixed_point_t ModifierSum::get_modifier_effect_value(ModifierEffect const& effect, bool* effect_found) const { return value_sum.get_effect(effect, effect_found); } @@ -55,21 +58,11 @@ void ModifierSum::add_modifier( modifier_entry_t::source_or_null_fallback(source, this_source), excluded_targets | this_excluded_targets ); - value_sum.multiply_add_exclude_targets(new_entry.modifier, new_entry.multiplier, new_entry.excluded_targets); - } -} - -void ModifierSum::add_modifier_sum(ModifierSum const& modifier_sum) { - reserve_more(modifiers, modifier_sum.modifiers.size()); - - // We could test that excluded_targets != ALL_TARGETS, but in practice it's always - // called with an explcit/hardcoded value and so won't ever exclude everything. - for (modifier_entry_t const& modifier_entry : modifier_sum.modifiers) { - add_modifier( - modifier_entry.modifier, - modifier_entry.multiplier, - modifier_entry.source, - modifier_entry.excluded_targets + value_sum.multiply_add_exclude_targets( + *new_entry.modifier, + new_entry.multiplier, + new_entry.excluded_targets ); + ++valid_size; } } diff --git a/src/openvic-simulation/modifier/ModifierSum.hpp b/src/openvic-simulation/modifier/ModifierSum.hpp index 5e5c78810..a24cf2dbc 100644 --- a/src/openvic-simulation/modifier/ModifierSum.hpp +++ b/src/openvic-simulation/modifier/ModifierSum.hpp @@ -1,8 +1,11 @@ #pragma once -#include +#include #include +#include + +#include "openvic-simulation/core/portable/ForwardableSpan.hpp" #include "openvic-simulation/dataloader/NodeTools.hpp" #include "openvic-simulation/modifier/ModifierValue.hpp" #include "openvic-simulation/modifier/Modifier.hpp" @@ -32,23 +35,40 @@ namespace OpenVic { ); } - Modifier const& modifier; + Modifier const* modifier; fixed_point_t multiplier; modifier_source_t source; ModifierEffect::target_t excluded_targets; + //invalid but required fore resizing to work + constexpr modifier_entry_t() + : modifier {nullptr}, + multiplier {fixed_point_t::_0}, + source {static_cast>(nullptr)}, + excluded_targets {} {} + + // constexpr modifier_entry_t( Modifier const& new_modifier, fixed_point_t new_multiplier, modifier_source_t const& new_source, ModifierEffect::target_t new_excluded_targets - ) : modifier { new_modifier }, + ) : modifier { &new_modifier }, multiplier { new_multiplier }, source { new_source }, excluded_targets { new_excluded_targets } {} + constexpr bool is_valid() const { + return multiplier != fixed_point_t::_0 + && modifier != nullptr + && ( + get_source_country() != nullptr + || get_source_province() != nullptr + ); + } + constexpr bool operator==(modifier_entry_t const& other) const { - return &modifier == &other.modifier + return modifier == other.modifier && multiplier == other.multiplier && source == other.source && excluded_targets == other.excluded_targets; @@ -68,14 +88,18 @@ namespace OpenVic { constexpr fixed_point_t get_modifier_effect_value( ModifierEffect const& effect, bool* effect_found = nullptr ) const { + if (!is_valid()) { + return fixed_point_t::_0; + } + if (ModifierEffect::excludes_targets(effect.targets, excluded_targets)) { - return modifier.get_effect(effect, effect_found) * multiplier; + return modifier->get_effect(effect, effect_found) * multiplier; } if (effect_found != nullptr) { *effect_found = false; } - return 0; + return fixed_point_t::_0; } }; @@ -89,15 +113,26 @@ namespace OpenVic { struct ModifierSum { private: + // to optimise adding other modifier sums + std::size_t extra_size { 0 }; + std::size_t valid_size { 0 }; //everything beyond this is empty + // Default source used for any modifier added to the sum without an explicit non-null source, // representing the object the sum is attached to. modifier_entry_t::modifier_source_t PROPERTY_RW(this_source); // Targets to be excluded from all modifiers added to the sum, combined with any explicit exclusions. ModifierEffect::target_t PROPERTY_RW(this_excluded_targets, ModifierEffect::target_t::NO_TARGETS); - memory::vector SPAN_PROPERTY(modifiers); + memory::vector modifiers; ModifierValue PROPERTY(value_sum); + constexpr void reserve(const std::size_t new_capacity) { + modifiers.reserve(new_capacity); + } + constexpr void resize(const std::size_t new_size) { + modifiers.resize(new_size); + } + public: ModifierSum() {}; ModifierSum(ModifierSum const&) = default; @@ -105,8 +140,17 @@ namespace OpenVic { ModifierSum& operator=(ModifierSum const&) = default; ModifierSum& operator=(ModifierSum&&) = default; + constexpr forwardable_span get_modifiers() const { + return { modifiers.data(), size() }; + } + + constexpr std::size_t size() const { + return valid_size; + } + constexpr bool empty() const { + return valid_size == 0; + } void clear(); - bool empty(); fixed_point_t get_modifier_effect_value(ModifierEffect const& effect, bool* effect_found = nullptr) const; bool has_modifier_effect(ModifierEffect const& effect) const; @@ -117,17 +161,51 @@ namespace OpenVic { modifier_entry_t::modifier_source_t const& source = {}, ModifierEffect::target_t excluded_targets = ModifierEffect::target_t::NO_TARGETS ); - // Reserves space for the number of modifier entries in the given sum and adds each of them using add_modifier + + constexpr void make_room_for(ModifierSum const& modifier_sum) { + extra_size += modifier_sum.valid_size; + } + + // Inserts modifiers directly via std::ranges::copy. Requires resizing beforehand! // with the modifier entries' attributes as arguments. This means non-null sources are preserved (null ones are // replaced with this_source, but in practice the other sum should've set them itself already) and exclusion targets // are combined with this_excluded_targets. - void add_modifier_sum(ModifierSum const& modifier_sum); + constexpr void add_modifier_sum(ModifierSum const& modifier_sum) { + if (extra_size > 0) { + resize( + valid_size + + extra_size + ); + extra_size = 0; + } + + const std::size_t new_valid_size = valid_size + modifier_sum.valid_size; + assert(new_valid_size <= modifiers.capacity()); + + // We could test that excluded_targets != ALL_TARGETS, but in practice it's always + // called with an explcit/hardcoded value and so won't ever exclude everything. + ranges::copy( + modifier_sum.modifiers.begin(), + modifier_sum.modifiers.begin() + modifier_sum.valid_size, + modifiers.begin() + valid_size + ); + + valid_size = new_valid_size; + + for (modifier_entry_t const& m : modifier_sum.get_modifiers()) { + value_sum.multiply_add_exclude_targets( + *m.modifier, + m.multiplier, + m.excluded_targets + ); + } + } // TODO - help calculate value_sum[effect]? Early return if lookup in value_sum fails? constexpr void for_each_contributing_modifier( ModifierEffect const& effect, ContributingModifierCallback auto callback ) const { - for (modifier_entry_t const& modifier_entry : modifiers) { + for (modifier_entry_t const& modifier_entry : get_modifiers()) { const fixed_point_t contribution = modifier_entry.get_modifier_effect_value(effect); if (contribution != 0) { From 3435c92b033c28588491f32c338c31af0d4e8559 Mon Sep 17 00:00:00 2001 From: wvpm <24685035+wvpm@users.noreply.github.com> Date: Mon, 18 May 2026 15:33:04 +0200 Subject: [PATCH 2/2] BulkInsertWrapper --- .../core/BulkInsertWrapper.hpp | 229 ++++++++++++++++++ .../modifier/ModifierSum.cpp | 3 - .../modifier/ModifierSum.hpp | 47 +--- 3 files changed, 237 insertions(+), 42 deletions(-) create mode 100644 src/openvic-simulation/core/BulkInsertWrapper.hpp diff --git a/src/openvic-simulation/core/BulkInsertWrapper.hpp b/src/openvic-simulation/core/BulkInsertWrapper.hpp new file mode 100644 index 000000000..c5de2f34b --- /dev/null +++ b/src/openvic-simulation/core/BulkInsertWrapper.hpp @@ -0,0 +1,229 @@ +#pragma once + +#include +#include +#include + +#include + +#include "openvic-simulation/core/Assert.hpp" + +namespace OpenVic { + template + struct bulk_insert_wrapper { + public: + // Member types based on std::vector + using container_type = Container; + using value_type = typename container_type::value_type; + using allocator_type = typename container_type::allocator_type; + using size_type = typename container_type::size_type; + using difference_type = typename container_type::difference_type; + using reference = typename container_type::reference; + using const_reference = typename container_type::const_reference; + using pointer = typename container_type::pointer; + using const_pointer = typename container_type::const_pointer; + using iterator = typename container_type::iterator; + using const_iterator = typename container_type::const_iterator; + using reverse_iterator = typename container_type::reverse_iterator; + using const_reverse_iterator = typename container_type::const_reverse_iterator; + + private: + Container container; + size_type valid_size {}; + size_type pending_extra_size {}; + + constexpr void flush_pending_room() { + if (pending_extra_size > size_type{}) { + container.resize(valid_size + pending_extra_size); + pending_extra_size = size_type{}; + } + } + + constexpr size_type get_invalid_element_count() const { + return container.size() - valid_size; + } + constexpr bool has_invalid_element() const { + return container.size() > valid_size; + } + + public: + constexpr allocator_type get_allocator() const { + return container.get_allocator(); + } + + constexpr bulk_insert_wrapper() noexcept {}; + + // Forwarding constructor for custom allocators or initial capacities + template + explicit bulk_insert_wrapper(Args&&... args) + : container(std::forward(args)...) {} + + constexpr void make_room_for(const size_type count) noexcept { + pending_extra_size += count; + } + + // Element access based on std::vector + constexpr reference operator[](const size_type pos) { + OV_HARDEN_ASSERT_ACCESS(pos, "operator[]"); + return container[pos]; + } + constexpr const_reference operator[](const size_type pos) const { + OV_HARDEN_ASSERT_ACCESS(pos, "operator[]"); + return container[pos]; + } + + constexpr reference front() { + OV_HARDEN_ASSERT_NONEMPTY("front"); + return container.front(); + } + constexpr const_reference front() const { + OV_HARDEN_ASSERT_NONEMPTY("front"); + return container.front(); + } + + constexpr reference back() { + OV_HARDEN_ASSERT_NONEMPTY("back"); + return container[size()-1]; + } + constexpr const_reference back() const { + OV_HARDEN_ASSERT_NONEMPTY("back"); + return container[size()-1]; + } + + constexpr value_type* data() noexcept { return container.data(); } + constexpr value_type const* data() const noexcept { return container.data(); } + + // Iterators based on std::vector + constexpr iterator begin() noexcept { + return container.begin(); + } + constexpr const_iterator begin() const noexcept { + return container.begin(); + } + constexpr const_iterator cbegin() const noexcept { + return container.cbegin(); + } + + constexpr iterator end() noexcept { + return std::next(begin(), valid_size); + } + constexpr const_iterator end() const noexcept { + return std::next(begin(), valid_size); + } + constexpr const_iterator cend() const noexcept { + return std::next(cbegin(), valid_size); + } + + constexpr reverse_iterator rbegin() noexcept { + return std::next(container.rbegin(), get_invalid_element_count()); + } + constexpr const_reverse_iterator rbegin() const noexcept { + return std::next(container.rbegin(), get_invalid_element_count()); + } + constexpr const_reverse_iterator crbegin() const noexcept { + return std::next(container.crbegin(), get_invalid_element_count()); + } + + constexpr reverse_iterator rend() noexcept { + return container.rend(); + } + constexpr const_reverse_iterator rend() const noexcept { + return container.rend(); + } + constexpr const_reverse_iterator crend() const noexcept { + return container.crend(); + } + + // Capacity based on std::vector + constexpr bool empty() const noexcept { return valid_size <= size_type{}; } + constexpr size_type size() const noexcept { return valid_size; } + constexpr size_type max_size() const noexcept { return container.max_size(); } + // reserve() is omitted as we manage that via make_room_for + constexpr size_type capacity() const noexcept { return container.capacity(); } + constexpr void shrink_to_fit() { + pending_extra_size = size_type{}; + container.resize(valid_size); + container.shrink_to_fit(); + } + + // Modifiers based on std::vector + constexpr void clear() noexcept { + valid_size = size_type{}; + pending_extra_size = size_type{}; + container.clear(); + } + + // the following could be implemented: + // - insert + // - insert_range + // - emplace + // - erase + // - append_range (C++23) + // - pop_back + // - swap + + constexpr void push_back(value_type const& value) { + flush_pending_room(); + if (get_invalid_element_count() > size_type{}) { + container[valid_size] = value; + } else { + container.push_back(value); + } + ++valid_size; + + } + constexpr void push_back(value_type&& value) { + flush_pending_room(); + if (has_invalid_element()) { + container[valid_size] = std::move(value); + } else { + container.push_back(std::forward(value)); + } + ++valid_size; + } + + template + constexpr reference emplace_back(Args&&... args) { + flush_pending_room(); + if (has_invalid_element()) { + auto* target_ptr = std::addressof(container[valid_size]); + + if constexpr (std::is_trivially_destructible_v) { + std::construct_at(target_ptr, std::forward(args)...); + } else { + std::destroy_at(target_ptr); + std::construct_at(target_ptr, std::forward(args)...); + } + + ++valid_size; + return *target_ptr; + } else { + reference result = container.emplace_back(std::forward(args)...); + ++valid_size; + return result; + } + } + + template + constexpr void append_range(OtherContainerT const& other) { + append_range(other.begin(), other.end()); + } + + template + constexpr void append_range(const InputIt first, const InputIt last) { + flush_pending_room(); + + const size_type new_valid_size = valid_size + std::distance(first, last); + assert(new_valid_size <= container.capacity()); + + ranges::copy( + first, + last, + end() + ); + valid_size = new_valid_size; + } + + // resize() is omitted as we manage that via make_room_for + }; +} \ No newline at end of file diff --git a/src/openvic-simulation/modifier/ModifierSum.cpp b/src/openvic-simulation/modifier/ModifierSum.cpp index c0c6518ae..442a4e8e7 100644 --- a/src/openvic-simulation/modifier/ModifierSum.cpp +++ b/src/openvic-simulation/modifier/ModifierSum.cpp @@ -31,8 +31,6 @@ memory::string modifier_entry_t::to_string() const { } void ModifierSum::clear() { - extra_size = 0; - valid_size = 0; modifiers.clear(); value_sum.clear(); } @@ -63,6 +61,5 @@ void ModifierSum::add_modifier( new_entry.multiplier, new_entry.excluded_targets ); - ++valid_size; } } diff --git a/src/openvic-simulation/modifier/ModifierSum.hpp b/src/openvic-simulation/modifier/ModifierSum.hpp index a24cf2dbc..97de16cca 100644 --- a/src/openvic-simulation/modifier/ModifierSum.hpp +++ b/src/openvic-simulation/modifier/ModifierSum.hpp @@ -3,8 +3,7 @@ #include #include -#include - +#include "openvic-simulation/core/BulkInsertWrapper.hpp" #include "openvic-simulation/core/portable/ForwardableSpan.hpp" #include "openvic-simulation/dataloader/NodeTools.hpp" #include "openvic-simulation/modifier/ModifierValue.hpp" @@ -113,26 +112,17 @@ namespace OpenVic { struct ModifierSum { private: - // to optimise adding other modifier sums - std::size_t extra_size { 0 }; - std::size_t valid_size { 0 }; //everything beyond this is empty - // Default source used for any modifier added to the sum without an explicit non-null source, // representing the object the sum is attached to. modifier_entry_t::modifier_source_t PROPERTY_RW(this_source); // Targets to be excluded from all modifiers added to the sum, combined with any explicit exclusions. ModifierEffect::target_t PROPERTY_RW(this_excluded_targets, ModifierEffect::target_t::NO_TARGETS); - memory::vector modifiers; + bulk_insert_wrapper< + memory::vector + > SPAN_PROPERTY(modifiers); ModifierValue PROPERTY(value_sum); - constexpr void reserve(const std::size_t new_capacity) { - modifiers.reserve(new_capacity); - } - constexpr void resize(const std::size_t new_size) { - modifiers.resize(new_size); - } - public: ModifierSum() {}; ModifierSum(ModifierSum const&) = default; @@ -140,15 +130,11 @@ namespace OpenVic { ModifierSum& operator=(ModifierSum const&) = default; ModifierSum& operator=(ModifierSum&&) = default; - constexpr forwardable_span get_modifiers() const { - return { modifiers.data(), size() }; - } - constexpr std::size_t size() const { - return valid_size; + return modifiers.size(); } constexpr bool empty() const { - return valid_size == 0; + return modifiers.empty(); } void clear(); @@ -163,7 +149,7 @@ namespace OpenVic { ); constexpr void make_room_for(ModifierSum const& modifier_sum) { - extra_size += modifier_sum.valid_size; + modifiers.make_room_for(modifier_sum.size()); } // Inserts modifiers directly via std::ranges::copy. Requires resizing beforehand! @@ -171,26 +157,9 @@ namespace OpenVic { // replaced with this_source, but in practice the other sum should've set them itself already) and exclusion targets // are combined with this_excluded_targets. constexpr void add_modifier_sum(ModifierSum const& modifier_sum) { - if (extra_size > 0) { - resize( - valid_size - + extra_size - ); - extra_size = 0; - } - - const std::size_t new_valid_size = valid_size + modifier_sum.valid_size; - assert(new_valid_size <= modifiers.capacity()); - // We could test that excluded_targets != ALL_TARGETS, but in practice it's always // called with an explcit/hardcoded value and so won't ever exclude everything. - ranges::copy( - modifier_sum.modifiers.begin(), - modifier_sum.modifiers.begin() + modifier_sum.valid_size, - modifiers.begin() + valid_size - ); - - valid_size = new_valid_size; + modifiers.append_range(modifier_sum.modifiers); for (modifier_entry_t const& m : modifier_sum.get_modifiers()) { value_sum.multiply_add_exclude_targets(