Merge pull request #403 from OpenVicProject/optimize/construction

Optimize vector construction

Author: Spartan322

src/openvic-simulation/console/ConsoleInstance.cpp

@@ -116,7 +116,7 @@ bool ConsoleInstance::execute(std::string_view command) {
 			}
 		}
 
-		arguments.push_back(std::string_view { arg_start, arg_end });
+		arguments.emplace_back(arg_start, arg_end);
 
 		// Accessing command.end() is UB
 		if (it == command.end()) {

src/openvic-simulation/country/CountryInstance.cpp

@@ -1832,7 +1832,10 @@ void CountryInstanceManager::update_rankings(Date today, DefineManager const& de
 
 CountryInstanceManager::CountryInstanceManager(CountryDefinitionManager const& new_country_definition_manager)
   : country_definition_manager { new_country_definition_manager },
-	country_definition_to_instance_map { &new_country_definition_manager.get_country_definitions() } {}
+	country_definition_to_instance_map { &new_country_definition_manager.get_country_definitions() } {
+		great_powers.reserve(16);
+		secondary_powers.reserve(16);
+	}
 
 CountryInstance& CountryInstanceManager::get_country_instance_from_definition(CountryDefinition const& country) {
 	return *country_definition_to_instance_map[country];

src/openvic-simulation/economy/production/ResourceGatheringOperation.cpp

@@ -162,7 +162,7 @@ void ResourceGatheringOperation::rgo_tick() {
 			get_country_to_report_economy_nullable->report_output(production_type, output_quantity_yesterday);
 		}
 
-		market_instance.place_market_sell_order(MarketSellOrder {
+		market_instance.place_market_sell_order({
 			production_type.get_output_good(),
 			output_quantity_yesterday,
 			this,

src/openvic-simulation/economy/trading/GoodMarket.cpp

@@ -53,14 +53,14 @@ void GoodMarket::update_next_price_limits() {
 	price_inverse = fixed_point_t::_1() / price;
 }
 
-void GoodMarket::add_buy_up_to_order(GoodBuyUpToOrder const& buy_up_to_order) {
+void GoodMarket::add_buy_up_to_order(GoodBuyUpToOrder&& buy_up_to_order) {
 	const std::lock_guard<std::mutex> lock_guard { *buy_lock };
-	buy_up_to_orders.push_back(buy_up_to_order);
+	buy_up_to_orders.push_back(std::move(buy_up_to_order));
 }
 
-void GoodMarket::add_market_sell_order(GoodMarketSellOrder const& market_sell_order) {
+void GoodMarket::add_market_sell_order(GoodMarketSellOrder&& market_sell_order) {
 	const std::lock_guard<std::mutex> lock_guard { *sell_lock };
-	market_sell_orders.push_back(market_sell_order);
+	market_sell_orders.push_back(std::move(market_sell_order));
 }
 
 void GoodMarket::execute_orders(std::vector<fixed_point_t>& reusable_vector_0, std::vector<fixed_point_t>& reusable_vector_1) {

src/openvic-simulation/economy/trading/GoodMarket.hpp

@@ -46,8 +46,8 @@ namespace OpenVic {
 		GoodMarket(GoodMarket&&) = default;
 
 		//thread safe
-		void add_buy_up_to_order(GoodBuyUpToOrder const& buy_up_to_order);
-		void add_market_sell_order(GoodMarketSellOrder const& market_sell_order);
+		void add_buy_up_to_order(GoodBuyUpToOrder&& buy_up_to_order);
+		void add_market_sell_order(GoodMarketSellOrder&& market_sell_order);
 
 		//not thread safe
 		void execute_orders(std::vector<fixed_point_t>& reusable_vector_0, std::vector<fixed_point_t>& reusable_vector_1);

src/openvic-simulation/economy/trading/MarketInstance.cpp

@@ -31,7 +31,7 @@ fixed_point_t MarketInstance::get_price_inverse(GoodDefinition const& good_defin
 	return good_instance_manager.get_good_instance_from_definition(good_definition).get_price_inverse();
 }
 
-void MarketInstance::place_buy_up_to_order(BuyUpToOrder const& buy_up_to_order) {
+void MarketInstance::place_buy_up_to_order(BuyUpToOrder&& buy_up_to_order) {
 	GoodDefinition const& good = buy_up_to_order.get_good();
 	if (OV_unlikely(buy_up_to_order.get_max_quantity() <= 0)) {
 		Logger::error("Received BuyUpToOrder for ",good," with max quantity ",buy_up_to_order.get_max_quantity());
@@ -40,10 +40,10 @@ void MarketInstance::place_buy_up_to_order(BuyUpToOrder const& buy_up_to_order)
 	}
 
 	GoodMarket& good_instance = good_instance_manager.get_good_instance_from_definition(good);
-	good_instance.add_buy_up_to_order(buy_up_to_order);
+	good_instance.add_buy_up_to_order(std::move(buy_up_to_order));
 }
 
-void MarketInstance::place_market_sell_order(MarketSellOrder const& market_sell_order) {
+void MarketInstance::place_market_sell_order(MarketSellOrder&& market_sell_order) {
 	GoodDefinition const& good = market_sell_order.get_good();
 	if (OV_unlikely(market_sell_order.get_quantity() <= 0)) {
 		Logger::error("Received MarketSellOrder for ", good, " with quantity ", market_sell_order.get_quantity());
@@ -60,7 +60,7 @@ void MarketInstance::place_market_sell_order(MarketSellOrder const& market_sell_
 	}
 
 	GoodMarket& good_instance = good_instance_manager.get_good_instance_from_definition(good);
-	good_instance.add_market_sell_order(market_sell_order);
+	good_instance.add_market_sell_order(std::move(market_sell_order));
 }
 
 void MarketInstance::execute_orders() {

src/openvic-simulation/economy/trading/MarketInstance.hpp

@@ -29,8 +29,8 @@ namespace OpenVic {
 		fixed_point_t get_min_next_price(GoodDefinition const& good_definition) const;
 		fixed_point_t get_max_money_to_allocate_to_buy_quantity(GoodDefinition const& good_definition, const fixed_point_t quantity) const;
 		fixed_point_t get_price_inverse(GoodDefinition const& good_definition) const;
-		void place_buy_up_to_order(BuyUpToOrder const& buy_up_to_order);
-		void place_market_sell_order(MarketSellOrder const& market_sell_order);
+		void place_buy_up_to_order(BuyUpToOrder&& buy_up_to_order);
+		void place_market_sell_order(MarketSellOrder&& market_sell_order);
 		void execute_orders();
 		void record_price_history();
 	};

src/openvic-simulation/history/DiplomaticHistory.cpp

@@ -111,6 +111,16 @@ std::vector<WarHistory const*> DiplomaticHistoryManager::get_wars(Date date) con
 bool DiplomaticHistoryManager::load_diplomacy_history_file(
 	CountryDefinitionManager const& country_definition_manager, ast::NodeCPtr root
 ) {
+	// Default vanilla alliances is 2, 54 is the max I've seen in mods
+	// Eliminates reallocations at the cost of memory usage
+	alliances.reserve(16);
+	// Default vanilla subjects is 42, 152 is the max I've seen in mods
+	// Eliminates reallocations at the cost of memory usage
+	subjects.reserve(64);
+	// Default vanilla reparations is 0, yet to see a mod use it
+	// Eliminates reallocations at the cost of memory usage
+	reparations.reserve(1);
+
 	return expect_dictionary_keys(
 		"alliance", ZERO_OR_MORE, [this, &country_definition_manager](ast::NodeCPtr node) -> bool {
 			CountryDefinition const* first = nullptr;
@@ -129,7 +139,7 @@ bool DiplomaticHistoryManager::load_diplomacy_history_file(
 				"end_date", ZERO_OR_ONE, expect_date_identifier_or_string(assign_variable_callback(end))
 			)(node);
 
-			alliances.push_back({ first, second, { start, end } });
+			alliances.emplace_back(first, second, Period { start, end });
 			return ret;
 		},
 		"vassal", ZERO_OR_MORE, [this, &country_definition_manager](ast::NodeCPtr node) -> bool {
@@ -149,7 +159,7 @@ bool DiplomaticHistoryManager::load_diplomacy_history_file(
 				"end_date", ZERO_OR_ONE, expect_date_identifier_or_string(assign_variable_callback(end))
 			)(node);
 
-			subjects.push_back({ overlord, subject, SubjectHistory::type_t::VASSAL, { start, end } });
+			subjects.emplace_back(overlord, subject, SubjectHistory::type_t::VASSAL, Period { start, end });
 			return ret;
 		},
 		"union", ZERO_OR_MORE, [this, &country_definition_manager](ast::NodeCPtr node) -> bool {
@@ -169,7 +179,7 @@ bool DiplomaticHistoryManager::load_diplomacy_history_file(
 				"end_date", ZERO_OR_ONE, expect_date_identifier_or_string(assign_variable_callback(end))
 			)(node);
 
-			subjects.push_back({ overlord, subject, SubjectHistory::type_t::UNION, { start, end } });
+			subjects.emplace_back(overlord, subject, SubjectHistory::type_t::UNION, Period { start, end });
 			return ret;
 		},
 		"substate", ZERO_OR_MORE, [this, &country_definition_manager](ast::NodeCPtr node) -> bool {
@@ -189,7 +199,7 @@ bool DiplomaticHistoryManager::load_diplomacy_history_file(
 				"end_date", ZERO_OR_ONE, expect_date_identifier_or_string(assign_variable_callback(end))
 			)(node);
 
-			subjects.push_back({ overlord, subject, SubjectHistory::type_t::SUBSTATE, { start, end } });
+			subjects.emplace_back(overlord, subject, SubjectHistory::type_t::SUBSTATE, Period { start, end });
 			return ret;
 		},
 		"reparations", ZERO_OR_MORE, [this, &country_definition_manager](ast::NodeCPtr node) -> bool {
@@ -209,29 +219,45 @@ bool DiplomaticHistoryManager::load_diplomacy_history_file(
 				"end_date", ZERO_OR_ONE, expect_date_identifier_or_string(assign_variable_callback(end))
 			)(node);
 
-			reparations.push_back({ receiver, sender, { start, end } });
+			reparations.emplace_back(receiver, sender, Period { start, end });
 			return ret;
 		}
 	)(root);
 }
 
 bool DiplomaticHistoryManager::load_war_history_file(DefinitionManager const& definition_manager, ast::NodeCPtr root) {
 	std::string_view name {};
-	std::vector<WarHistory::war_participant_t> attackers {};
-	std::vector<WarHistory::war_participant_t> defenders {};
-	std::vector<WarHistory::added_wargoal_t> wargoals {};
+
+	// Ensures that if ever multithreaded, only one vector is used per thread
+	// Else acts like static
+	thread_local std::vector<WarHistory::war_participant_t> attackers;
+	// Default max vanilla attackers is 1, 1 is the max I've seen in mods
+	// Eliminates reallocations
+	attackers.reserve(1);
+
+	thread_local std::vector<WarHistory::war_participant_t> defenders;
+	// Default max vanilla defenders is 1, 1 is the max I've seen in mods
+	// Eliminates reallocations
+	defenders.reserve(1);
+
+	thread_local std::vector<WarHistory::added_wargoal_t> wargoals;
+	// Default max vanilla wargoals is 2, 2 is the max I've seen in mods
+	// Eliminates reallocations
+	wargoals.reserve(1);
+
 	Date current_date {};
 
+
 	bool ret = expect_dictionary_keys_and_default(
-		[&definition_manager, &attackers, &defenders, &wargoals, &current_date, &name](
+		[&definition_manager, &current_date, &name](
 			std::string_view key, ast::NodeCPtr node
 		) -> bool {
 			bool ret = expect_date_str(assign_variable_callback(current_date))(key);
 
 			ret &= expect_dictionary_keys(
 				"add_attacker", ZERO_OR_MORE,
 					definition_manager.get_country_definition_manager().expect_country_definition_identifier(
-						[&attackers, &current_date, &name](CountryDefinition const& country) -> bool {
+						[&current_date, &name](CountryDefinition const& country) -> bool {
 							for (auto const& attacker : attackers) {
 								if (attacker.get_country() == &country) {
 									Logger::error(
@@ -243,13 +269,13 @@ bool DiplomaticHistoryManager::load_war_history_file(DefinitionManager const& de
 								}
 							}
 
-							attackers.push_back({ &country, { current_date, {} } });
+							attackers.emplace_back(&country, Period { current_date, {} });
 							return true;
 						}
 					),
 				"add_defender", ZERO_OR_MORE,
 					definition_manager.get_country_definition_manager().expect_country_definition_identifier(
-						[&defenders, &current_date, &name](CountryDefinition const& country) -> bool {
+						[&current_date, &name](CountryDefinition const& country) -> bool {
 							for (auto const& defender : defenders) {
 								if (defender.get_country() == &country) {
 									Logger::error(
@@ -261,13 +287,13 @@ bool DiplomaticHistoryManager::load_war_history_file(DefinitionManager const& de
 								}
 							}
 
-							defenders.push_back({ &country, { current_date, {} } });
+							defenders.emplace_back(&country, Period { current_date, {} });
 							return true;
 						}
 					),
 				"rem_attacker", ZERO_OR_MORE,
 					definition_manager.get_country_definition_manager().expect_country_definition_identifier(
-						[&attackers, &current_date, &name](CountryDefinition const& country) -> bool {
+						[&current_date, &name](CountryDefinition const& country) -> bool {
 							WarHistory::war_participant_t* participant_to_remove = nullptr;
 
 							for (auto& attacker : attackers) {
@@ -291,7 +317,7 @@ bool DiplomaticHistoryManager::load_war_history_file(DefinitionManager const& de
 					),
 				"rem_defender", ZERO_OR_MORE,
 					definition_manager.get_country_definition_manager().expect_country_definition_identifier(
-						[&defenders, &current_date, &name](CountryDefinition const& country) -> bool {
+						[&current_date, &name](CountryDefinition const& country) -> bool {
 							WarHistory::war_participant_t* participant_to_remove = nullptr;
 
 							for (auto& defender : defenders) {
@@ -313,7 +339,7 @@ bool DiplomaticHistoryManager::load_war_history_file(DefinitionManager const& de
 							return participant_to_remove->period.try_set_end(current_date);
 						}
 					),
-				"war_goal", ZERO_OR_MORE, [&definition_manager, &wargoals, &current_date](ast::NodeCPtr value) -> bool {
+				"war_goal", ZERO_OR_MORE, [&definition_manager, &current_date](ast::NodeCPtr value) -> bool {
 					CountryDefinition const* actor = nullptr;
 					CountryDefinition const* receiver = nullptr;
 					WargoalType const* type = nullptr;
@@ -341,7 +367,7 @@ bool DiplomaticHistoryManager::load_war_history_file(DefinitionManager const& de
 							)
 					)(value);
 
-					wargoals.push_back({ current_date, actor, receiver, type, third_party, target });
+					wargoals.emplace_back(current_date, actor, receiver, type, third_party, target);
 					return ret;
 				}
 			)(node);
@@ -351,7 +377,7 @@ bool DiplomaticHistoryManager::load_war_history_file(DefinitionManager const& de
 		"name", ZERO_OR_ONE, expect_string(assign_variable_callback(name))
 	)(root);
 
-	wars.push_back({ name, std::move(attackers), std::move(defenders), std::move(wargoals) });
+	wars.emplace_back(name, std::move(attackers), std::move(defenders), std::move(wargoals));
 
 	return ret;
 }

src/openvic-simulation/history/DiplomaticHistory.hpp

@@ -20,6 +20,12 @@ namespace OpenVic {
 		struct added_wargoal_t {
 			friend struct DiplomaticHistoryManager;
 
+			added_wargoal_t(
+				Date new_added, CountryDefinition const* new_actor, CountryDefinition const* new_receiver,
+				WargoalType const* new_wargoal, std::optional<CountryDefinition const*> new_third_party,
+				std::optional<ProvinceDefinition const*> new_target
+			);
+
 		private:
 			Date PROPERTY_CUSTOM_PREFIX(added, get_date);
 			CountryDefinition const* PROPERTY(actor);
@@ -29,23 +35,24 @@ namespace OpenVic {
 			// TODO - could these just be nullptr when unset rather than using optionals?
 			std::optional<CountryDefinition const*> PROPERTY(third_party);
 			std::optional<ProvinceDefinition const*> PROPERTY(target);
-
-			added_wargoal_t(
-				Date new_added, CountryDefinition const* new_actor, CountryDefinition const* new_receiver,
-				WargoalType const* new_wargoal, std::optional<CountryDefinition const*> new_third_party,
-				std::optional<ProvinceDefinition const*> new_target
-			);
 		};
+		static_assert(std::is_trivially_move_constructible_v<added_wargoal_t>);
 
 		struct war_participant_t {
 			friend struct DiplomaticHistoryManager;
 
+			war_participant_t(CountryDefinition const* new_country, Period period);
+
 		private:
 			CountryDefinition const* PROPERTY(country);
 			Period PROPERTY(period);
-
-			war_participant_t(CountryDefinition const* new_country, Period period);
 		};
+		static_assert(std::is_trivially_move_constructible_v<war_participant_t>);
+
+		WarHistory(
+			std::string_view new_war_name, std::vector<war_participant_t>&& new_attackers,
+			std::vector<war_participant_t>&& new_defenders, std::vector<added_wargoal_t>&& new_wargoals
+		);
 
 	private:
 		/* Edge cases where this is empty/undef for some reason,
@@ -54,54 +61,52 @@ namespace OpenVic {
 		std::vector<war_participant_t> PROPERTY(attackers);
 		std::vector<war_participant_t> PROPERTY(defenders);
 		std::vector<added_wargoal_t> PROPERTY(wargoals);
-
-		WarHistory(
-			std::string_view new_war_name, std::vector<war_participant_t>&& new_attackers,
-			std::vector<war_participant_t>&& new_defenders, std::vector<added_wargoal_t>&& new_wargoals
-		);
 	};
 
 	struct AllianceHistory {
 		friend struct DiplomaticHistoryManager;
 
+		AllianceHistory(CountryDefinition const* new_first, CountryDefinition const* new_second, Period period);
+
 	private:
 		CountryDefinition const* PROPERTY(first);
 		CountryDefinition const* PROPERTY(second);
 		const Period PROPERTY(period);
-
-		AllianceHistory(CountryDefinition const* new_first, CountryDefinition const* new_second, Period period);
 	};
+	static_assert(std::is_trivially_move_constructible_v<AllianceHistory>);
 
 	struct ReparationsHistory {
 		friend struct DiplomaticHistoryManager;
 
+		ReparationsHistory(CountryDefinition const* new_receiver, CountryDefinition const* new_sender, Period period);
+
 	private:
 		CountryDefinition const* PROPERTY(receiver);
 		CountryDefinition const* PROPERTY(sender);
 		const Period PROPERTY(period);
-
-		ReparationsHistory(CountryDefinition const* new_receiver, CountryDefinition const* new_sender, Period period);
 	};
+	static_assert(std::is_trivially_move_constructible_v<ReparationsHistory>);
 
 	struct SubjectHistory {
 		friend struct DiplomaticHistoryManager;
 
-		enum class type_t {
+		enum class type_t : uint8_t {
 			VASSAL,
 			UNION,
 			SUBSTATE
 		};
 
+		SubjectHistory(
+			CountryDefinition const* new_overlord, CountryDefinition const* new_subject, type_t new_type, Period period
+		);
+
 	private:
 		CountryDefinition const* PROPERTY(overlord);
 		CountryDefinition const* PROPERTY(subject);
 		const type_t PROPERTY_CUSTOM_PREFIX(type, get_subject);
 		const Period PROPERTY(period);
-
-		SubjectHistory(
-			CountryDefinition const* new_overlord, CountryDefinition const* new_subject, type_t new_type, Period period
-		);
 	};
+	static_assert(std::is_trivially_move_constructible_v<SubjectHistory>);
 
 	struct CountryDefinitionManager;
 	struct DefinitionManager;