From 8ef822c02d17875d6909700d333f459324b9a7a4 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 --- .../core/BulkInsertWrapper.hpp | 201 ++++++++++++++++++ .../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 | 38 ++-- .../modifier/ModifierSum.hpp | 71 +++++-- tests/src/SpyAllocator.hpp | 61 ++++++ tests/src/core/BulkInsertWrapper.cpp | 162 ++++++++++++++ 10 files changed, 512 insertions(+), 37 deletions(-) create mode 100644 src/openvic-simulation/core/BulkInsertWrapper.hpp create mode 100644 tests/src/SpyAllocator.hpp create mode 100644 tests/src/core/BulkInsertWrapper.cpp diff --git a/src/openvic-simulation/core/BulkInsertWrapper.hpp b/src/openvic-simulation/core/BulkInsertWrapper.hpp new file mode 100644 index 000000000..cd31e8bdf --- /dev/null +++ b/src/openvic-simulation/core/BulkInsertWrapper.hpp @@ -0,0 +1,201 @@ +#pragma once + +#include +#include +#include +#include +#include +#include +#include + +#include "openvic-simulation/core/Assert.hpp" + +namespace OpenVic { + // not thread safe + 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; + + static_assert(std::is_default_constructible_v); + static_assert(std::is_trivially_destructible_v); + + private: + Container container; + std::atomic pending_extra_size {}; + + constexpr void flush_pending_room() { + if (pending_extra_size > size_type{}) { + size_type valid_size { size() }; + container.resize(valid_size + pending_extra_size); + container.resize(valid_size); + pending_extra_size = size_type{}; + } + } + + 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 + constexpr explicit bulk_insert_wrapper(Args&&... args) + : container(std::forward(args)...) {} + + // thread safe + 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[0]; + } + constexpr const_reference front() const { + OV_HARDEN_ASSERT_NONEMPTY("front"); + return container[0]; + } + + 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 container.end(); + } + constexpr const_iterator end() const noexcept { + return container.end(); + } + constexpr const_iterator cend() const noexcept { + return container.cend(); + } + + constexpr reverse_iterator rbegin() noexcept { + return container.rbegin(); + } + constexpr const_reverse_iterator rbegin() const noexcept { + return container.rbegin(); + } + constexpr const_reverse_iterator crbegin() const noexcept { + return container.crbegin(); + } + + 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 size() <= size_type{}; } + constexpr size_type size() const noexcept { return container.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.shrink_to_fit(); + } + + // Modifiers based on std::vector + constexpr void clear() noexcept { + 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(); + container.push_back(value); + + } + constexpr void push_back(value_type&& value) { + flush_pending_room(); + container.push_back(std::move(value)); + } + + template + requires std::is_trivially_destructible_v + constexpr reference emplace_back(Args&&... args) { + return container.emplace_back(std::forward(args)...); + } + + 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 = size() + std::distance(first, last); + if (new_valid_size > container.capacity()) { + assert(!"append_range called without make_room_for"); + container.reserve(new_valid_size); + } + + std::uninitialized_copy(first, last, end()); + container.resize(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/country/CountryInstance.cpp b/src/openvic-simulation/country/CountryInstance.cpp index 14bfe7a3d..3978b10f7 100644 --- a/src/openvic-simulation/country/CountryInstance.cpp +++ b/src/openvic-simulation/country/CountryInstance.cpp @@ -1856,6 +1856,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 cfe3c3b0a..ed9f5958f 100644 --- a/src/openvic-simulation/country/CountryInstance.hpp +++ b/src/openvic-simulation/country/CountryInstance.hpp @@ -686,6 +686,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 4b8cc6286..5a9b50270 100644 --- a/src/openvic-simulation/map/MapInstance.cpp +++ b/src/openvic-simulation/map/MapInstance.cpp @@ -149,6 +149,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 b10a92072..aa058883b 100644 --- a/src/openvic-simulation/map/ProvinceInstance.cpp +++ b/src/openvic-simulation/map/ProvinceInstance.cpp @@ -283,6 +283,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 68b252a35..c21510e46 100644 --- a/src/openvic-simulation/map/ProvinceInstance.hpp +++ b/src/openvic-simulation/map/ProvinceInstance.hpp @@ -190,6 +190,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..442a4e8e7 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,7 +23,9 @@ 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) ); } @@ -30,10 +35,6 @@ void ModifierSum::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 +56,10 @@ 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 ); } } diff --git a/src/openvic-simulation/modifier/ModifierSum.hpp b/src/openvic-simulation/modifier/ModifierSum.hpp index 72b2459c2..4be964469 100644 --- a/src/openvic-simulation/modifier/ModifierSum.hpp +++ b/src/openvic-simulation/modifier/ModifierSum.hpp @@ -1,8 +1,11 @@ #pragma once +#include #include +#include "openvic-simulation/core/BulkInsertWrapper.hpp" #include "openvic-simulation/core/memory/Vector.hpp" +#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" @@ -31,23 +34,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; @@ -67,14 +87,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; } }; @@ -94,18 +118,22 @@ namespace OpenVic { // 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); + bulk_insert_wrapper< + memory::vector + > SPAN_PROPERTY(modifiers); ModifierValue PROPERTY(value_sum); public: ModifierSum() {}; - ModifierSum(ModifierSum const&) = default; ModifierSum(ModifierSum&&) = default; - ModifierSum& operator=(ModifierSum const&) = default; - ModifierSum& operator=(ModifierSum&&) = default; + constexpr std::size_t size() const { + return modifiers.size(); + } + constexpr bool empty() const { + return modifiers.empty(); + } 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; @@ -116,17 +144,34 @@ 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) { + modifiers.make_room_for(modifier_sum.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) { + // 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. + modifiers.append_range(modifier_sum.modifiers); + + 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) { diff --git a/tests/src/SpyAllocator.hpp b/tests/src/SpyAllocator.hpp new file mode 100644 index 000000000..22a1d21e0 --- /dev/null +++ b/tests/src/SpyAllocator.hpp @@ -0,0 +1,61 @@ +#include +#include + +// Telemetry structure remains the same +struct AllocationMetrics { + std::size_t total_allocated_bytes = 0; + std::size_t total_deallocated_bytes = 0; + std::size_t allocation_count = 0; + std::size_t deallocation_count = 0; + + void reset() { + total_allocated_bytes = 0; + total_deallocated_bytes = 0; + allocation_count = 0; + deallocation_count = 0; + } + + std::size_t active_bytes() const { + return total_allocated_bytes - total_deallocated_bytes; + } +}; + +// SpyAllocator now wraps an underlying Allocator type (defaults to std::allocator) +template > +class SpyAllocator { +public: + using value_type = T; + + // Shared telemetry state across allocator copies + std::shared_ptr metrics; + // The actual allocator doing the heavy lifting + Allocator upstream; + + // Default Constructor + SpyAllocator() + : metrics(std::make_shared()), upstream() {} + + // Explicitly pass an existing upstream allocator instance if needed + explicit SpyAllocator(const Allocator& alloc) + : metrics(std::make_shared()), upstream(alloc) {} + + value_type* allocate(const std::size_t n) { + // Delegate allocation to the upstream allocator + value_type* ptr = std::allocator_traits::allocate(upstream, n); + + if (metrics && ptr) { + metrics->total_allocated_bytes += (n * sizeof(value_type)); + metrics->allocation_count++; + } + return ptr; + } + + void deallocate(value_type* const ptr, const std::size_t n) noexcept { + if (metrics) { + metrics->total_deallocated_bytes += (n * sizeof(value_type)); + metrics->deallocation_count++; + } + // Delegate deallocation to the upstream allocator + std::allocator_traits::deallocate(upstream, ptr, n); + } +}; \ No newline at end of file diff --git a/tests/src/core/BulkInsertWrapper.cpp b/tests/src/core/BulkInsertWrapper.cpp new file mode 100644 index 000000000..7b2301ce6 --- /dev/null +++ b/tests/src/core/BulkInsertWrapper.cpp @@ -0,0 +1,162 @@ +#include +#include +#include + +#include "openvic-simulation/core/BulkInsertWrapper.hpp" + +#include + +#include "SpyAllocator.hpp" + +// A simple non-trivially destructible type to test constraint violations if needed, +// and a trivial one to satisfy emplace_back's requires clause. +struct TrivialPoint { + int x = 0; + int y = 0; +}; + +using namespace OpenVic; + +TEST_CASE("bulk_insert_wrapper Constructors", "[bulk_insert_wrapper][bulk_insert_wrapper-constructor]") { + constexpr bulk_insert_wrapper> empty; + CONSTEXPR_CHECK(empty.empty()); + CONSTEXPR_CHECK(empty.size() == 0); + CONSTEXPR_CHECK(empty.capacity() == 0); + CONSTEXPR_CHECK(empty.begin() == empty.end()); + CONSTEXPR_CHECK(empty.cbegin() == empty.cend()); + CONSTEXPR_CHECK(empty.rbegin() == empty.rend()); + + constexpr std::size_t expected_size = 3; + bulk_insert_wrapper> filled { + std::vector { 1, 2, 3 } + }; + CHECK(!filled.empty()); + CHECK(filled.size() == expected_size); + CHECK(filled.capacity() >= expected_size); + CHECK(std::distance(filled.begin(), filled.end()) == expected_size); + CHECK(std::distance(filled.cbegin(), filled.cend()) == expected_size); + CHECK(std::distance(filled.rbegin(), filled.rend()) == expected_size); + CHECK(filled[0] == 1); +} + +TEST_CASE("bulk_insert_wrapper make_room does not allocate", "[bulk_insert_wrapper][bulk_insert_wrapper-make_room]") { + SpyAllocator spy_allocator{}; + bulk_insert_wrapper< + std::vector< + int, + SpyAllocator + > + > empty { spy_allocator }; + empty.make_room_for(10); + + CHECK(empty.empty()); + CHECK(empty.size() == 0); + CHECK(empty.capacity() == 0); + CHECK(empty.begin() == empty.end()); + CHECK(empty.cbegin() == empty.cend()); + CHECK(empty.rbegin() == empty.rend()); + CHECK(spy_allocator.metrics->allocation_count == 0); +} + +// correct usage +TEST_CASE("bulk_insert_wrapper make_room + append_range", "[bulk_insert_wrapper][bulk_insert_wrapper-append_range]") { + SpyAllocator spy_allocator{}; + bulk_insert_wrapper< + std::vector< + int, + SpyAllocator + > + > wrapper { spy_allocator }; + + std::vector a { 1, 2 }; + std::vector b { 3, 4, 5 }; + + wrapper.make_room_for(a.size()); + wrapper.make_room_for(b.size()); + + wrapper.append_range(a); + CHECK(wrapper.size() == a.size()); + CHECK(wrapper.capacity() >= a.size() + b.size()); + + wrapper.append_range(b); + + CHECK(wrapper.size() == a.size() + b.size()); + CHECK(spy_allocator.metrics->allocation_count == 1); +} + +// cassert prevents testing in debug +#ifdef NDEBUG +// incorrect usage may not crash +TEST_CASE("bulk_insert_wrapper append_range without make_room", "[bulk_insert_wrapper][bulk_insert_wrapper-append_range]") { + SpyAllocator spy_allocator{}; + bulk_insert_wrapper< + std::vector< + int, + SpyAllocator + > + > wrapper { spy_allocator }; + + std::vector a { 1, 2 }; + std::vector b { 3, 4, 5 }; + + wrapper.append_range(a); + wrapper.append_range(b); + + CHECK(wrapper.size() == a.size() + b.size()); + CHECK(wrapper.capacity() >= a.size() + b.size()); + CHECK(spy_allocator.metrics->allocation_count <= 2); +} +#endif + +TEST_CASE("bulk_insert_wrapper clear", "[bulk_insert_wrapper][bulk_insert_wrapper-clear]") { + SpyAllocator spy_allocator{}; + bulk_insert_wrapper< + std::vector< + int, + SpyAllocator + > + > wrapper { spy_allocator }; + + std::vector a { 1, 2 }; + constexpr std::size_t extra_room = 1; + + wrapper.make_room_for(a.size()); + wrapper.make_room_for(extra_room); + + wrapper.append_range(a); + CHECK(wrapper.size() == a.size()); + CHECK(wrapper.capacity() >= a.size() + extra_room); + + wrapper.clear(); + CHECK(wrapper.empty()); + CHECK(spy_allocator.metrics->allocation_count == 1); +} + +TEST_CASE("bulk_insert_wrapper shrink_to_fit", "[bulk_insert_wrapper][bulk_insert_wrapper-shrink_to_fit]") { + SpyAllocator spy_allocator{}; + bulk_insert_wrapper< + std::vector< + int, + SpyAllocator + > + > wrapper { spy_allocator }; + + std::vector a { 1, 2 }; + constexpr std::size_t extra_room = 1; + + wrapper.make_room_for(a.size()); + wrapper.make_room_for(extra_room); + + wrapper.append_range(a); + CHECK(wrapper.size() == a.size()); + const std::size_t capacity_before_shrink = wrapper.capacity(); + CHECK(capacity_before_shrink >= a.size() + extra_room); + + wrapper.shrink_to_fit(); + CHECK(wrapper.size() == a.size()); + // shrink_to_fit() is non-binding, it may or may not actually shrink depending on underlying container implementation. + // just ensure we don't expand or allocate too often somehow + CHECK(wrapper.capacity() <= capacity_before_shrink); + CHECK(spy_allocator.metrics->allocation_count >= 1); + CHECK(spy_allocator.metrics->allocation_count <= 2); +} \ No newline at end of file From 543e5e03048dfe7f2e89388c63b19616c417bd7e Mon Sep 17 00:00:00 2001 From: wvpm <24685035+wvpm@users.noreply.github.com> Date: Tue, 19 May 2026 12:56:57 +0200 Subject: [PATCH 2/2] Parallellise update gamestate and modifiers --- src/openvic-simulation/InstanceManager.cpp | 37 +++-- .../country/CountryInstance.cpp | 56 ++++--- .../country/CountryInstance.hpp | 6 +- .../country/CountryInstanceManager.cpp | 20 +-- .../country/CountryInstanceManager.hpp | 7 +- .../economy/trading/MarketInstance.cpp | 2 +- src/openvic-simulation/map/MapInstance.cpp | 24 ++- src/openvic-simulation/map/MapInstance.hpp | 6 +- .../map/ProvinceInstance.cpp | 62 ++++---- .../map/ProvinceInstance.hpp | 8 +- src/openvic-simulation/utility/ThreadDeps.hpp | 28 ++++ src/openvic-simulation/utility/ThreadPool.cpp | 149 ++++++++---------- src/openvic-simulation/utility/ThreadPool.hpp | 58 +++---- 13 files changed, 233 insertions(+), 230 deletions(-) create mode 100644 src/openvic-simulation/utility/ThreadDeps.hpp diff --git a/src/openvic-simulation/InstanceManager.cpp b/src/openvic-simulation/InstanceManager.cpp index cf628e40c..022406351 100644 --- a/src/openvic-simulation/InstanceManager.cpp +++ b/src/openvic-simulation/InstanceManager.cpp @@ -5,6 +5,7 @@ #include "openvic-simulation/core/stl/containers/TypedSpan.hpp" #include "openvic-simulation/misc/GameAction.hpp" #include "openvic-simulation/utility/Logger.hpp" +#include "openvic-simulation/utility/ThreadDeps.hpp" using namespace OpenVic; @@ -168,8 +169,8 @@ void InstanceManager::update_gamestate() { update_modifier_sums(); // Update gamestate... - map_instance.update_gamestate(*this); - country_instance_manager.update_gamestate(today, map_instance); + map_instance.update_gamestate(); + country_instance_manager.update_gamestate_after_map(today); unit_instance_manager.update_gamestate(); gamestate_updated(); @@ -234,12 +235,19 @@ bool InstanceManager::setup() { } thread_pool.initialise_threadpool( - game_rules_manager, - good_instance_manager, - definition_manager.get_modifier_manager().get_modifier_effect_cache(), - definition_manager.get_define_manager().get_pops_defines(), - definition_manager.get_economy_manager().get_production_type_manager(), - strata_index_t(definition_manager.get_pop_manager().get_strata_count()), + ThreadDeps{ + game_rules_manager, + good_instance_manager, + map_instance, + definition_manager.get_define_manager().get_military_defines(), + definition_manager.get_modifier_manager().get_modifier_effect_cache(), + definition_manager.get_define_manager().get_pops_defines(), + definition_manager.get_economy_manager().get_production_type_manager(), + definition_manager.get_modifier_manager().get_static_modifier_cache(), + country_index_t(definition_manager.get_country_definition_manager().get_country_definition_count()), + good_index_t(definition_manager.get_economy_manager().get_good_definition_manager().get_good_definition_count()), + strata_index_t(definition_manager.get_pop_manager().get_strata_count()) + }, good_instance_manager.get_good_instances(), country_instance_manager.get_country_instances(), map_instance.get_province_instances() @@ -305,8 +313,8 @@ bool InstanceManager::load_bookmark(Bookmark const& new_bookmark) { OV_ERR_FAIL_COND_V_MSG(!all_has_state, false, "At least one land province has no state"); update_modifier_sums(); - map_instance.initialise_for_new_game(*this); - country_instance_manager.update_gamestate(today, map_instance); + map_instance.initialise_for_new_game(); + country_instance_manager.update_gamestate_after_map(today); market_instance.execute_orders(); return ret; @@ -359,12 +367,9 @@ void InstanceManager::update_modifier_sums() { // full copy of all the modifiers affecting them in their modifier sum, but provinces only having their directly/locally // applied modifiers in their modifier sum, hence requiring owner country modifier effect values to be looked up when // determining the value of a global effect on the province. - country_instance_manager.update_modifier_sums( - today, definition_manager.get_modifier_manager().get_static_modifier_cache() - ); - map_instance.update_modifier_sums( - today, definition_manager.get_modifier_manager().get_static_modifier_cache() - ); + country_instance_manager.update_modifier_sums_before_map(); + map_instance.update_modifier_sums(today); + country_instance_manager.update_modifier_sums_after_map(); } bool InstanceManager::queue_game_action(game_action_t&& game_action) { diff --git a/src/openvic-simulation/country/CountryInstance.cpp b/src/openvic-simulation/country/CountryInstance.cpp index 3978b10f7..122504ee5 100644 --- a/src/openvic-simulation/country/CountryInstance.cpp +++ b/src/openvic-simulation/country/CountryInstance.cpp @@ -1782,7 +1782,7 @@ static constexpr Modifier const& get_country_status_static_effect( } } -void CountryInstance::update_modifier_sum(Date today, StaticModifierCache const& static_modifier_cache) { +void CountryInstance::update_modifier_sum_before_map(Date today, StaticModifierCache const& static_modifier_cache) { // Update sum of national modifiers modifier_sum.clear(); @@ -1856,19 +1856,23 @@ 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::update_modifier_sum_after_map(Date today) { + for (ProvinceInstance const* const province_ptr : controlled_provinces) { + if (OV_likely(province_ptr != nullptr)) { + modifier_sum.add_modifier_sum(province_ptr->get_modifier_sum()); + } + } } -void CountryInstance::contribute_province_modifier_sum(ModifierSum const& province_modifier_sum) { - modifier_sum.add_modifier_sum(province_modifier_sum); +void CountryInstance::make_room_for_province_modifier_sum(ModifierSum const& province_modifier_sum) { + modifier_sum.make_room_for(province_modifier_sum); } fixed_point_t CountryInstance::get_modifier_effect_value(ModifierEffect const& effect) const { return modifier_sum.get_modifier_effect_value(effect); } -void CountryInstance::update_gamestate(const Date today, MapInstance& map_instance) { +void CountryInstance::update_gamestate_after_map(const Date today) { if (is_civilised()) { civilisation_progress = 0; } else { @@ -1932,14 +1936,14 @@ void CountryInstance::update_gamestate(const Date today, MapInstance& map_instan province->set_connected_to_capital(false); province->set_is_overseas(province_definition.get_continent() != capital_continent); - for (ProvinceDefinition::adjacency_t const& adjacency : province_definition.get_adjacencies()) { - // TODO - should we limit based on adjacency type? Straits and impassable still work in game, - // and water provinces don't have an owner so they'll get caught by the later checks anyway. - CountryInstance* neighbour = map_instance.get_province_instance_by_definition(adjacency.get_to()).get_owner(); - if (neighbour != nullptr && neighbour != this) { - neighbouring_countries.insert(neighbour); - } - } + // for (ProvinceDefinition::adjacency_t const& adjacency : province_definition.get_adjacencies()) { + // // TODO - should we limit based on adjacency type? Straits and impassable still work in game, + // // and water provinces don't have an owner so they'll get caught by the later checks anyway. + // CountryInstance* neighbour = map_instance.get_province_instance_by_definition(adjacency.get_to()).get_owner(); + // if (neighbour != nullptr && neighbour != this) { + // neighbouring_countries.insert(neighbour); + // } + // } } if (occupied_provinces_proportion != 0) { @@ -1949,21 +1953,21 @@ void CountryInstance::update_gamestate(const Date today, MapInstance& map_instan if (capital != nullptr) { capital->set_connected_to_capital(true); - memory::vector> province_checklist { *capital }; + // memory::vector> province_checklist { *capital }; - for (size_t index = 0; index < province_checklist.size(); ++index) { - ProvinceInstance const& province = province_checklist[index]; + // for (size_t index = 0; index < province_checklist.size(); ++index) { + // ProvinceInstance const& province = province_checklist[index]; - for (ProvinceDefinition::adjacency_t const& adjacency : province.province_definition.get_adjacencies()) { - ProvinceInstance& adjacent_province = map_instance.get_province_instance_by_definition(adjacency.get_to()); + // for (ProvinceDefinition::adjacency_t const& adjacency : province.province_definition.get_adjacencies()) { + // ProvinceInstance& adjacent_province = map_instance.get_province_instance_by_definition(adjacency.get_to()); - if (adjacent_province.get_owner() == this && !adjacent_province.get_connected_to_capital()) { - adjacent_province.set_connected_to_capital(true); - adjacent_province.set_is_overseas(false); - province_checklist.emplace_back(adjacent_province); - } - } - } + // if (adjacent_province.get_owner() == this && !adjacent_province.get_connected_to_capital()) { + // adjacent_province.set_connected_to_capital(true); + // adjacent_province.set_is_overseas(false); + // province_checklist.emplace_back(adjacent_province); + // } + // } + // } } // Order of updates might need to be changed/functions split up to account for dependencies diff --git a/src/openvic-simulation/country/CountryInstance.hpp b/src/openvic-simulation/country/CountryInstance.hpp index ed9f5958f..db871e643 100644 --- a/src/openvic-simulation/country/CountryInstance.hpp +++ b/src/openvic-simulation/country/CountryInstance.hpp @@ -685,9 +685,9 @@ namespace OpenVic { bool update_rule_set(); public: - void update_modifier_sum(Date today, StaticModifierCache const& static_modifier_cache); + void update_modifier_sum_before_map(Date today, StaticModifierCache const& static_modifier_cache); + void update_modifier_sum_after_map(Date today); 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( ModifierEffect const& effect, ContributingModifierCallback auto callback @@ -695,7 +695,7 @@ namespace OpenVic { return modifier_sum.for_each_contributing_modifier(effect, std::move(callback)); } - void update_gamestate(const Date today, MapInstance& map_instance); + void update_gamestate_after_map(const Date today); void country_tick_before_map( TypedSpan reusable_goods_mask, forwardable_span< diff --git a/src/openvic-simulation/country/CountryInstanceManager.cpp b/src/openvic-simulation/country/CountryInstanceManager.cpp index 090745d07..f3ee82992 100644 --- a/src/openvic-simulation/country/CountryInstanceManager.cpp +++ b/src/openvic-simulation/country/CountryInstanceManager.cpp @@ -292,16 +292,16 @@ bool CountryInstanceManager::apply_history_to_countries(InstanceManager& instanc return ret; } -void CountryInstanceManager::update_modifier_sums(const Date today, StaticModifierCache const& static_modifier_cache) { - for (CountryInstance& country : country_instances) { - country.update_modifier_sum(today, static_modifier_cache); - } +void CountryInstanceManager::update_modifier_sums_before_map() { + thread_pool.process(work_t::COUNTRY_UPDATE_MODIFIER_SUMS_BEFORE_MAP); } -void CountryInstanceManager::update_gamestate(const Date today, MapInstance& map_instance) { - for (CountryInstance& country : country_instances) { - country.update_gamestate(today, map_instance); - } +void CountryInstanceManager::update_modifier_sums_after_map() { + thread_pool.process(work_t::COUNTRY_UPDATE_MODIFIER_SUMS_AFTER_MAP); +} + +void CountryInstanceManager::update_gamestate_after_map(const Date today) { + thread_pool.process(work_t::COUNTRY_UPDATE_GAMESTATE_AFTER_MAP); // TODO - work out how to have ranking effects applied (e.g. static modifiers) applied at game start // we can't just move update_rankings to the top of this function as it will choose initial GPs based on @@ -311,10 +311,10 @@ void CountryInstanceManager::update_gamestate(const Date today, MapInstance& map } void CountryInstanceManager::country_manager_tick_before_map() { - thread_pool.process_country_ticks_before_map(); + thread_pool.process(work_t::COUNTRY_TICK_BEFORE_MAP); } void CountryInstanceManager::country_manager_tick_after_map() { - thread_pool.process_country_ticks_after_map(); + thread_pool.process(work_t::COUNTRY_TICK_AFTER_MAP); shared_country_values.update_costs(); } diff --git a/src/openvic-simulation/country/CountryInstanceManager.hpp b/src/openvic-simulation/country/CountryInstanceManager.hpp index f611c51b9..fff69e4f1 100644 --- a/src/openvic-simulation/country/CountryInstanceManager.hpp +++ b/src/openvic-simulation/country/CountryInstanceManager.hpp @@ -18,10 +18,8 @@ namespace OpenVic { struct CountryHistoryManager; struct GoodInstanceManager; struct InstanceManager; - struct MapInstance; struct PopsDefines; struct PopType; - struct StaticModifierCache; struct ThreadPool; struct CountryInstanceManager { @@ -71,8 +69,9 @@ namespace OpenVic { bool apply_history_to_countries(InstanceManager& instance_manager); - void update_modifier_sums(const Date today, StaticModifierCache const& static_modifier_cache); - void update_gamestate(const Date today, MapInstance& map_instance); + void update_modifier_sums_before_map(); + void update_modifier_sums_after_map(); + void update_gamestate_after_map(const Date today); void country_manager_tick_before_map(); void country_manager_tick_after_map(); }; diff --git a/src/openvic-simulation/economy/trading/MarketInstance.cpp b/src/openvic-simulation/economy/trading/MarketInstance.cpp index b39cea789..e18b9d52b 100644 --- a/src/openvic-simulation/economy/trading/MarketInstance.cpp +++ b/src/openvic-simulation/economy/trading/MarketInstance.cpp @@ -78,7 +78,7 @@ void MarketInstance::place_market_sell_order(MarketSellOrder&& market_sell_order } void MarketInstance::execute_orders() { - thread_pool.process_good_execute_orders(); + thread_pool.process(work_t::GOOD_EXECUTE_ORDERS); } void MarketInstance::record_price_history() { diff --git a/src/openvic-simulation/map/MapInstance.cpp b/src/openvic-simulation/map/MapInstance.cpp index 5a9b50270..00e53531d 100644 --- a/src/openvic-simulation/map/MapInstance.cpp +++ b/src/openvic-simulation/map/MapInstance.cpp @@ -145,23 +145,17 @@ bool MapInstance::apply_history_to_provinces( return ret; } -void MapInstance::update_modifier_sums(const Date today, StaticModifierCache const& static_modifier_cache) { - 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_modifier_sums(const Date today) { + thread_pool.process(work_t::PROVINCE_UPDATE_MODIFIER_SUMS); } -void MapInstance::update_gamestate(InstanceManager const& instance_manager) { +void MapInstance::update_gamestate() { highest_province_population = 0; total_map_population = 0; - for (ProvinceInstance& province : get_province_instances()) { - province.update_gamestate(instance_manager); + thread_pool.process(work_t::PROVINCE_UPDATE_GAMESTATE); + for (ProvinceInstance& province : get_province_instances()) { // Update population stats const pop_sum_t province_population = province.get_total_population(); if (highest_province_population < province_population) { @@ -174,13 +168,13 @@ void MapInstance::update_gamestate(InstanceManager const& instance_manager) { } void MapInstance::map_tick() { - thread_pool.process_province_ticks(); + thread_pool.process(work_t::PROVINCE_TICK); //state tick //after province tick as province tick sets pop employment to 0 //state tick will update pop employment via factories } -void MapInstance::initialise_for_new_game(InstanceManager const& instance_manager) { - update_gamestate(instance_manager); - thread_pool.process_province_initialise_for_new_game(); +void MapInstance::initialise_for_new_game() { + update_gamestate(); + thread_pool.process(work_t::PROVINCE_INITIALISE_FOR_NEW_GAME); } \ No newline at end of file diff --git a/src/openvic-simulation/map/MapInstance.hpp b/src/openvic-simulation/map/MapInstance.hpp index 0c37d5589..3c42cbb3b 100644 --- a/src/openvic-simulation/map/MapInstance.hpp +++ b/src/openvic-simulation/map/MapInstance.hpp @@ -85,9 +85,9 @@ namespace OpenVic { TypedSpan reforms ); - void update_modifier_sums(const Date today, StaticModifierCache const& static_modifier_cache); - void update_gamestate(InstanceManager const& instance_manager); + void update_modifier_sums(const Date today); + void update_gamestate(); void map_tick(); - void initialise_for_new_game(InstanceManager const& instance_manager); + void initialise_for_new_game(); }; } diff --git a/src/openvic-simulation/map/ProvinceInstance.cpp b/src/openvic-simulation/map/ProvinceInstance.cpp index aa058883b..0da0f8fa3 100644 --- a/src/openvic-simulation/map/ProvinceInstance.cpp +++ b/src/openvic-simulation/map/ProvinceInstance.cpp @@ -4,18 +4,26 @@ #include -#include "openvic-simulation/country/CountryDefinition.hpp" #include "openvic-simulation/country/CountryInstance.hpp" +#include "openvic-simulation/country/CountryInstanceManager.hpp" #include "openvic-simulation/defines/MilitaryDefines.hpp" -#include "openvic-simulation/DefinitionManager.hpp" #include "openvic-simulation/economy/BuildingInstance.hpp" #include "openvic-simulation/economy/BuildingType.hpp" #include "openvic-simulation/economy/production/Employee.hpp" #include "openvic-simulation/economy/production/ProductionType.hpp" -#include "openvic-simulation/InstanceManager.hpp" +#include "openvic-simulation/history/ProvinceHistory.hpp" +#include "openvic-simulation/map/Crime.hpp" +#include "openvic-simulation/map/MapInstance.hpp" #include "openvic-simulation/map/ProvinceDefinition.hpp" +#include "openvic-simulation/map/Region.hpp" +#include "openvic-simulation/map/TerrainType.hpp" +#include "openvic-simulation/military/UnitInstanceGroup.hpp" +#include "openvic-simulation/military/UnitType.hpp" #include "openvic-simulation/misc/GameRulesManager.hpp" #include "openvic-simulation/modifier/StaticModifierCache.hpp" +#include "openvic-simulation/politics/Reform.hpp" +#include "openvic-simulation/population/PopType.hpp" +#include "openvic-simulation/population/PopValuesFromProvince.hpp" #include "openvic-simulation/types/ConstructorTags.hpp" #include "openvic-simulation/types/TypedIndices.hpp" @@ -42,6 +50,8 @@ ProvinceInstance::ProvinceInstance( } } { + adjacencies.reserve(new_province_definition.get_adjacencies().size()); + adjacent_nonempty_land_provinces.reserve(new_province_definition.get_adjacencies().size()); modifier_sum.set_this_source(this); rgo.setup_location_ptr(*this); } @@ -233,7 +243,7 @@ void ProvinceInstance::_update_pops(MilitaryDefines const& military_defines) { normalise_pops_aggregate(); } -void ProvinceInstance::update_modifier_sum(Date today, StaticModifierCache const& static_modifier_cache) { +void ProvinceInstance::update_modifier_sum(const Date today, StaticModifierCache const& static_modifier_cache) { // Update sum of direct province modifiers modifier_sum.clear(); @@ -288,9 +298,17 @@ void ProvinceInstance::update_modifier_sum(Date today, StaticModifierCache const } } -void ProvinceInstance::update_country_modifier_sum() { - if (controller != nullptr) { - controller->contribute_province_modifier_sum(modifier_sum); +void ProvinceInstance::update_adjecencies() { + has_empty_adjacent_province = false; + // We assume there are no duplicate province adjacencies, so each adjacency.get_to() is unique in the loop below + adjacent_nonempty_land_provinces.clear(); + for (const std::reference_wrapper adjacency_wrapper : get_adjacencies()) { + ProvinceInstance const& adjacency = adjacency_wrapper.get(); + if (adjacency.is_empty()) { + has_empty_adjacent_province = true; + } else if (!adjacency.province_definition.is_water()) { + adjacent_nonempty_land_provinces.emplace_back(adjacency); + } } } @@ -332,23 +350,8 @@ bool ProvinceInstance::convert_rgo_worker_pops_to_equivalent( return is_valid_operation; } -void ProvinceInstance::update_gamestate(InstanceManager const& instance_manager) { - has_empty_adjacent_province = false; - // We assume there are no duplicate province adjacencies, so each adjacency.get_to() is unique in the loop below - adjacent_nonempty_land_provinces.clear(); - - MapInstance const& map_instance = instance_manager.get_map_instance(); - for (ProvinceDefinition::adjacency_t const& adjacency : province_definition.get_adjacencies()) { - ProvinceDefinition const& adjacent_to_definition = adjacency.get_to(); - ProvinceInstance const& adjacent_to_instance = map_instance.get_province_instance_by_definition(adjacent_to_definition); - - if (adjacent_to_instance.is_empty()) { - has_empty_adjacent_province = true; - } else if (!adjacent_to_definition.is_water()) { - adjacent_nonempty_land_provinces.emplace_back(adjacent_to_instance); - } - } - +void ProvinceInstance::update_gamestate(const Date today, MilitaryDefines const& military_defines) { + update_adjecencies(); land_regiment_count = 0; for (ArmyInstance const& army : armies) { land_regiment_count += army.get_unit_count(); @@ -363,12 +366,10 @@ void ProvinceInstance::update_gamestate(InstanceManager const& instance_manager) occupation_duration = 0; } - const Date today = instance_manager.get_today(); - for (BuildingInstance& building : buildings) { building.update_gamestate(today); } - _update_pops(instance_manager.definition_manager.get_define_manager().get_military_defines()); + _update_pops(military_defines); } void ProvinceInstance::province_tick( @@ -504,6 +505,7 @@ bool ProvinceInstance::apply_history_to_province(ProvinceHistoryEntry const& ent void ProvinceInstance::initialise_for_new_game( const Date today, + MapInstance const& map_instance, PopValuesFromProvince& reusable_pop_values, RandomU32& random_number_generator, TypedSpan reusable_goods_mask, @@ -512,6 +514,12 @@ void ProvinceInstance::initialise_for_new_game( VECTORS_FOR_PROVINCE_TICK > reusable_vectors ) { + for (ProvinceDefinition::adjacency_t const& adjacency : province_definition.get_adjacencies()) { + ProvinceDefinition const& adjacent_to_definition = adjacency.get_to(); + ProvinceInstance const& adjacent_to_instance = map_instance.get_province_instance_by_definition(adjacent_to_definition); + adjacencies.emplace_back(adjacent_to_instance); + } + update_adjecencies(); initialise_rgo(); province_tick( today, diff --git a/src/openvic-simulation/map/ProvinceInstance.hpp b/src/openvic-simulation/map/ProvinceInstance.hpp index c21510e46..8870ff930 100644 --- a/src/openvic-simulation/map/ProvinceInstance.hpp +++ b/src/openvic-simulation/map/ProvinceInstance.hpp @@ -95,6 +95,7 @@ namespace OpenVic { bool PROPERTY_RW(connected_to_capital, false); bool PROPERTY_RW(is_overseas, false); bool PROPERTY(has_empty_adjacent_province, false); + memory::vector> SPAN_PROPERTY(adjacencies); memory::vector> SPAN_PROPERTY(adjacent_nonempty_land_provinces); Crime const* PROPERTY_RW(crime, nullptr); ResourceGatheringOperation PROPERTY(rgo); @@ -123,6 +124,7 @@ namespace OpenVic { ProductionType const& production_type ); void initialise_rgo(); + void update_adjecencies(); memory::FixedVector< memory::vector>, @@ -188,9 +190,8 @@ namespace OpenVic { PopDeps const& pop_deps ); size_t get_pop_count() const; + void update_modifier_sum(const Date today, StaticModifierCache const& static_modifier_cache); - 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 { @@ -205,7 +206,7 @@ namespace OpenVic { get_owner_modifier_sum().for_each_contributing_modifier(effect, std::move(callback)); } } - void update_gamestate(InstanceManager const& instance_manager); + void update_gamestate(const Date today, MilitaryDefines const& military_defines); static constexpr size_t VECTORS_FOR_PROVINCE_TICK = std::max( ResourceGatheringOperation::VECTORS_FOR_RGO_TICK, Pop::VECTORS_FOR_POP_TICK @@ -222,6 +223,7 @@ namespace OpenVic { ); void initialise_for_new_game( const Date today, + MapInstance const& map_instance, PopValuesFromProvince& reusable_pop_values, RandomU32& random_number_generator, TypedSpan reusable_goods_mask, diff --git a/src/openvic-simulation/utility/ThreadDeps.hpp b/src/openvic-simulation/utility/ThreadDeps.hpp new file mode 100644 index 000000000..49eb2bfd7 --- /dev/null +++ b/src/openvic-simulation/utility/ThreadDeps.hpp @@ -0,0 +1,28 @@ +#pragma once + +#include "openvic-simulation/types/TypedIndices.hpp" + +namespace OpenVic { + struct GameRulesManager; + struct GoodInstanceManager; + struct MapInstance; + struct MilitaryDefines; + struct ModifierEffectCache; + struct PopsDefines; + struct ProductionTypeManager; + struct StaticModifierCache; + + struct ThreadDeps { + GameRulesManager const& game_rules_manager; + GoodInstanceManager const& good_instance_manager; + MapInstance const& map_instance; + MilitaryDefines const& military_defines; + ModifierEffectCache const& modifier_effect_cache; + PopsDefines const& pop_defines; + ProductionTypeManager const& production_type_manager; + StaticModifierCache const& static_modifier_cache; + country_index_t country_count; + good_index_t good_count; + strata_index_t strata_count; + }; +} \ No newline at end of file diff --git a/src/openvic-simulation/utility/ThreadPool.cpp b/src/openvic-simulation/utility/ThreadPool.cpp index 77c02f9b3..6dfb33875 100644 --- a/src/openvic-simulation/utility/ThreadPool.cpp +++ b/src/openvic-simulation/utility/ThreadPool.cpp @@ -1,4 +1,5 @@ #include "ThreadPool.hpp" +#include "ThreadDeps.hpp" #include #include @@ -11,6 +12,7 @@ #include "openvic-simulation/economy/GoodInstance.hpp" #include "openvic-simulation/economy/trading/GoodMarket.hpp" #include "openvic-simulation/map/ProvinceInstance.hpp" +#include "openvic-simulation/population/PopValuesFromProvince.hpp" #include "openvic-simulation/types/fixed_point/FixedPoint.hpp" #include "openvic-simulation/types/TypedIndices.hpp" @@ -18,20 +20,13 @@ using namespace OpenVic; void ThreadPool::loop_until_cancelled( work_t& work_type, - GameRulesManager const& game_rules_manager, - GoodInstanceManager const& good_instance_manager, - ModifierEffectCache const& modifier_effect_cache, - PopsDefines const& pop_defines, - ProductionTypeManager const& production_type_manager, - forwardable_span country_keys, - const good_index_t good_count, - const strata_index_t strata_count, + const ThreadDeps deps, forwardable_span work_bundles ) { - memory::FixedVector reusable_goods_mask { good_count, {} }; + memory::FixedVector reusable_goods_mask { deps.good_count, {} }; - memory::FixedVector reusable_country_map_0 { country_index_t(country_keys.size()), fixed_point_t::_0 }; - memory::FixedVector reusable_country_map_1 { country_index_t(country_keys.size()), fixed_point_t::_0 }; + memory::FixedVector reusable_country_map_0 { deps.country_count, fixed_point_t::_0 }; + memory::FixedVector reusable_country_map_1 { deps.country_count, fixed_point_t::_0 }; static constexpr std::size_t VECTOR_COUNT = std::max( GoodMarket::VECTORS_FOR_EXECUTE_ORDERS, @@ -44,12 +39,12 @@ void ThreadPool::loop_until_cancelled( std::span, VECTOR_COUNT> reusable_vectors_span = std::span(reusable_vectors); memory::vector reusable_good_index_vector; PopValuesFromProvince reusable_pop_values { - game_rules_manager, - good_instance_manager, - modifier_effect_cache, - production_type_manager, - pop_defines, - strata_count + deps.game_rules_manager, + deps.good_instance_manager, + deps.modifier_effect_cache, + deps.production_type_manager, + deps.pop_defines, + deps.strata_count }; while (!is_cancellation_requested) { @@ -70,11 +65,11 @@ void ThreadPool::loop_until_cancelled( work_type = work_t::NONE; } - switch (work_type_copy) { - case work_t::NONE: - break; - case work_t::GOOD_EXECUTE_ORDERS: - for (WorkBundle& work_bundle : work_bundles) { + for (WorkBundle& work_bundle : work_bundles) { + switch (work_type_copy) { + case work_t::NONE: + break; + case work_t::GOOD_EXECUTE_ORDERS: for (GoodMarket& good : work_bundle.goods_chunk) { good.execute_orders( reusable_country_map_0, @@ -82,10 +77,8 @@ void ThreadPool::loop_until_cancelled( reusable_vectors_span.first() ); } - } - break; - case work_t::PROVINCE_TICK: - for (WorkBundle& work_bundle : work_bundles) { + break; + case work_t::PROVINCE_TICK: for (ProvinceInstance& province : work_bundle.provinces_chunk) { province.province_tick( current_date, @@ -95,23 +88,20 @@ void ThreadPool::loop_until_cancelled( reusable_vectors_span.first() ); } - } - break; - case work_t::PROVINCE_INITIALISE_FOR_NEW_GAME: - for (WorkBundle& work_bundle : work_bundles) { + break; + case work_t::PROVINCE_INITIALISE_FOR_NEW_GAME: for (ProvinceInstance& province : work_bundle.provinces_chunk) { province.initialise_for_new_game( current_date, + deps.map_instance, reusable_pop_values, work_bundle.random_number_generator, reusable_goods_mask, reusable_vectors_span.first() ); } - } - break; - case work_t::COUNTRY_TICK_BEFORE_MAP: - for (WorkBundle& work_bundle : work_bundles) { + break; + case work_t::COUNTRY_TICK_BEFORE_MAP: for (CountryInstance& country : work_bundle.countries_chunk) { country.country_tick_before_map( reusable_goods_mask, @@ -119,15 +109,47 @@ void ThreadPool::loop_until_cancelled( reusable_good_index_vector ); } - } - break; - case work_t::COUNTRY_TICK_AFTER_MAP: - for (WorkBundle& work_bundle : work_bundles) { + break; + case work_t::COUNTRY_TICK_AFTER_MAP: for (CountryInstance& country : work_bundle.countries_chunk) { country.country_tick_after_map(current_date); } - } - break; + break; + case work_t::PROVINCE_UPDATE_GAMESTATE: + for (ProvinceInstance& province : work_bundle.provinces_chunk) { + province.update_gamestate( + current_date, + deps.military_defines + ); + } + break; + case work_t::COUNTRY_UPDATE_GAMESTATE_AFTER_MAP: + for (CountryInstance& country : work_bundle.countries_chunk) { + country.update_gamestate_after_map(current_date); + } + break; + case work_t::PROVINCE_UPDATE_MODIFIER_SUMS: + for (ProvinceInstance& province : work_bundle.provinces_chunk) { + province.update_modifier_sum( + current_date, + deps.static_modifier_cache + ); + } + break; + case work_t::COUNTRY_UPDATE_MODIFIER_SUMS_BEFORE_MAP: + for (CountryInstance& country : work_bundle.countries_chunk) { + country.update_modifier_sum_before_map( + current_date, + deps.static_modifier_cache + ); + } + break; + case work_t::COUNTRY_UPDATE_MODIFIER_SUMS_AFTER_MAP: + for (CountryInstance& country : work_bundle.countries_chunk) { + country.update_modifier_sum_after_map(current_date); + } + break; + } } { @@ -139,7 +161,7 @@ void ThreadPool::loop_until_cancelled( } } -void ThreadPool::process_work(const work_t work_type) { +void ThreadPool::process(const work_t work_type) { { std::unique_lock thread_lock { thread_mutex }; if (is_cancellation_requested) { @@ -184,12 +206,7 @@ ThreadPool::~ThreadPool() { } void ThreadPool::initialise_threadpool( - GameRulesManager const& game_rules_manager, - GoodInstanceManager const& good_instance_manager, - ModifierEffectCache const& modifier_effect_cache, - PopsDefines const& pop_defines, - ProductionTypeManager const& production_type_manager, - const strata_index_t strata_count, + ThreadDeps const& deps, forwardable_span goods, forwardable_span countries, forwardable_span provinces @@ -261,27 +278,13 @@ void ThreadPool::initialise_threadpool( [ this, &work_for_thread = work_per_thread[i], - &game_rules_manager, - &good_instance_manager, - &modifier_effect_cache, - &pop_defines, - &production_type_manager, - countries, - good_count = good_index_t(goods.size()), - strata_count, + deps, work_bundles_begin, work_bundles_end ]() -> void { loop_until_cancelled( work_for_thread, - game_rules_manager, - good_instance_manager, - modifier_effect_cache, - pop_defines, - production_type_manager, - countries, - good_count, - strata_count, + deps, std::span{ work_bundles_begin, work_bundles_end } ); } @@ -289,24 +292,4 @@ void ThreadPool::initialise_threadpool( work_bundles_begin = work_bundles_end; } -} - -void ThreadPool::process_good_execute_orders() { - process_work(work_t::GOOD_EXECUTE_ORDERS); -} - -void ThreadPool::process_province_ticks() { - process_work(work_t::PROVINCE_TICK); -} - -void ThreadPool::process_province_initialise_for_new_game() { - process_work(work_t::PROVINCE_INITIALISE_FOR_NEW_GAME); -} - -void ThreadPool::process_country_ticks_before_map() { - process_work(work_t::COUNTRY_TICK_BEFORE_MAP); -} - -void ThreadPool::process_country_ticks_after_map(){ - process_work(work_t::COUNTRY_TICK_AFTER_MAP); } \ No newline at end of file diff --git a/src/openvic-simulation/utility/ThreadPool.hpp b/src/openvic-simulation/utility/ThreadPool.hpp index 8251857ef..75a05fdf9 100644 --- a/src/openvic-simulation/utility/ThreadPool.hpp +++ b/src/openvic-simulation/utility/ThreadPool.hpp @@ -7,24 +7,17 @@ #include #include -#include "openvic-simulation/population/PopValuesFromProvince.hpp" #include "openvic-simulation/core/memory/Vector.hpp" #include "openvic-simulation/core/portable/ForwardableSpan.hpp" #include "openvic-simulation/core/random/RandomGenerator.hpp" #include "openvic-simulation/population/PopValuesFromProvince.hpp" #include "openvic-simulation/types/Date.hpp" -#include "openvic-simulation/types/TypedIndices.hpp" namespace OpenVic { - struct GameRulesManager; - struct GoodDefinition; - struct GoodInstanceManager; struct CountryInstance; struct GoodInstance; - struct ModifierEffectCache; - struct PopsDefines; - struct ProductionTypeManager; - struct Strata; + struct ProvinceInstance; + struct ThreadDeps; //bundle work so they always have the same rng regardless of hardware concurrency struct WorkBundle { @@ -47,18 +40,23 @@ namespace OpenVic { provinces_chunk { new_provinces_chunk } {} }; + + enum struct work_t : uint8_t { + NONE, + GOOD_EXECUTE_ORDERS, + PROVINCE_INITIALISE_FOR_NEW_GAME, + PROVINCE_TICK, + COUNTRY_TICK_BEFORE_MAP, + COUNTRY_TICK_AFTER_MAP, + PROVINCE_UPDATE_GAMESTATE, + COUNTRY_UPDATE_GAMESTATE_AFTER_MAP, + PROVINCE_UPDATE_MODIFIER_SUMS, + COUNTRY_UPDATE_MODIFIER_SUMS_BEFORE_MAP, + COUNTRY_UPDATE_MODIFIER_SUMS_AFTER_MAP + }; struct ThreadPool { private: - enum struct work_t : uint8_t { - NONE, - GOOD_EXECUTE_ORDERS, - PROVINCE_INITIALISE_FOR_NEW_GAME, - PROVINCE_TICK, - COUNTRY_TICK_BEFORE_MAP, - COUNTRY_TICK_AFTER_MAP - }; - constexpr static std::size_t WORK_BUNDLE_COUNT = 32; std::array all_work_bundles; memory::vector threads; @@ -71,39 +69,21 @@ namespace OpenVic { void loop_until_cancelled( work_t& work_type, - GameRulesManager const& game_rules_manager, - GoodInstanceManager const& good_instance_manager, - ModifierEffectCache const& modifier_effect_cache, - PopsDefines const& pop_defines, - ProductionTypeManager const& production_type_manager, - forwardable_span country_keys, - const good_index_t good_count, - const strata_index_t strata_count, + const ThreadDeps deps, forwardable_span work_bundles ); void await_completion(); - void process_work(const work_t work_type); public: ThreadPool(Date const& new_current_date); ~ThreadPool(); void initialise_threadpool( - GameRulesManager const& game_rules_manager, - GoodInstanceManager const& good_instance_manager, - ModifierEffectCache const& modifier_effect_cache, - PopsDefines const& pop_defines, - ProductionTypeManager const& production_type_manager, - const strata_index_t strata_count, + ThreadDeps const& deps, forwardable_span goods, forwardable_span countries, forwardable_span provinces ); - - void process_good_execute_orders(); - void process_province_ticks(); - void process_province_initialise_for_new_game(); - void process_country_ticks_before_map(); - void process_country_ticks_after_map(); + void process(const work_t work_type); }; } \ No newline at end of file