From be147651d42ea1cc573198a0df3a220d264da5d2 Mon Sep 17 00:00:00 2001 From: Peter Nelson Date: Sun, 21 Jan 2024 14:09:44 +0000 Subject: [PATCH] Codechange: Replace GroupStatistics' num_engines with std::map. (#11849) This removes manual memory management with calloc/free calls, and prevents potentially large arrays being allocated for each group. (cherry picked from commit 8797cc7ef25700e414809244a9bcde682cdff067) --- src/build_vehicle_gui.cpp | 2 +- src/group.h | 8 ++++---- src/group_cmd.cpp | 31 ++++++++++++++++--------------- src/script/api/script_engine.cpp | 2 +- 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/src/build_vehicle_gui.cpp b/src/build_vehicle_gui.cpp index 5a6889f7c3..707709ea27 100644 --- a/src/build_vehicle_gui.cpp +++ b/src/build_vehicle_gui.cpp @@ -252,7 +252,7 @@ static bool EngineIntroDateSorter(const GUIEngineListItem &a, const GUIEngineLis static bool EngineVehicleCountSorter(const GUIEngineListItem &a, const GUIEngineListItem &b) { const GroupStatistics &stats = GroupStatistics::Get(_local_company, ALL_GROUP, Engine::Get(a.engine_id)->type); - const int r = ((int) stats.num_engines[a.engine_id]) - ((int) stats.num_engines[b.engine_id]); + const int r = ((int) stats.GetNumEngines(a.engine_id)) - ((int) stats.GetNumEngines(b.engine_id)); /* Use EngineID to sort instead since we want consistent sorting */ if (r == 0) return EngineNumberSorter(a, b); diff --git a/src/group.h b/src/group.h index d5762717aa..f75668ea6b 100644 --- a/src/group.h +++ b/src/group.h @@ -16,6 +16,7 @@ #include "vehicle_type.h" #include "engine_type.h" #include "livery.h" +#include "3rdparty/cpp-btree/btree_map.h" #include typedef Pool GroupPool; @@ -25,15 +26,12 @@ extern GroupPool _group_pool; ///< Pool of groups. struct GroupStatistics { Money profit_last_year; ///< Sum of profits for all vehicles. Money profit_last_year_min_age; ///< Sum of profits for vehicles considered for profit statistics. - uint16_t *num_engines; ///< Caches the number of engines of each type the company owns. + btree::btree_map num_engines; ///< Caches the number of engines of each type the company owns. uint16_t num_vehicle; ///< Number of vehicles. uint16_t num_vehicle_min_age; ///< Number of vehicles considered for profit statistics; bool autoreplace_defined; ///< Are any autoreplace rules set? bool autoreplace_finished; ///< Have all autoreplacement finished? - GroupStatistics(); - ~GroupStatistics(); - void Clear(); void ClearProfits() @@ -50,6 +48,8 @@ struct GroupStatistics { this->autoreplace_finished = false; } + uint16_t GetNumEngines(EngineID engine) const; + static GroupStatistics &Get(CompanyID company, GroupID id_g, VehicleType type); static GroupStatistics &Get(const Vehicle *v); static GroupStatistics &GetAllGroup(const Vehicle *v); diff --git a/src/group_cmd.cpp b/src/group_cmd.cpp index 105aee4df2..d9a7d7485e 100644 --- a/src/group_cmd.cpp +++ b/src/group_cmd.cpp @@ -35,16 +35,6 @@ GroupID _new_group_id; GroupPool _group_pool("Group"); INSTANTIATE_POOL_METHODS(Group) -GroupStatistics::GroupStatistics() -{ - this->num_engines = CallocT(Engine::GetPoolSize()); -} - -GroupStatistics::~GroupStatistics() -{ - free(this->num_engines); -} - /** * Clear all caches. */ @@ -55,9 +45,20 @@ void GroupStatistics::Clear() this->num_vehicle_min_age = 0; this->profit_last_year_min_age = 0; - /* This is also called when NewGRF change. So the number of engines might have changed. Reallocate. */ - free(this->num_engines); - this->num_engines = CallocT(Engine::GetPoolSize()); + /* This is also called when NewGRF change. So the number of engines might have changed. Reset. */ + this->num_engines.clear(); +} + +/** + * Get number of vehicles of a specific engine ID. + * @param engine Engine ID. + * @returns number of vehicles of this engine ID. + */ +uint16_t GroupStatistics::GetNumEngines(EngineID engine) const +{ + auto found = this->num_engines.find(engine); + if (found != std::end(this->num_engines)) return found->second; + return 0; } /** @@ -1032,9 +1033,9 @@ uint GetGroupNumEngines(CompanyID company, GroupID id_g, EngineID id_e) uint count = 0; const Engine *e = Engine::Get(id_e); IterateDescendantsOfGroup(id_g, [&](Group *g) { - count += GroupStatistics::Get(company, g->index, e->type).num_engines[id_e]; + count += GroupStatistics::Get(company, g->index, e->type).GetNumEngines(id_e); }); - return count + GroupStatistics::Get(company, id_g, e->type).num_engines[id_e]; + return count + GroupStatistics::Get(company, id_g, e->type).GetNumEngines(id_e); } /** diff --git a/src/script/api/script_engine.cpp b/src/script/api/script_engine.cpp index 2f2c130ad8..7aecda3b1a 100644 --- a/src/script/api/script_engine.cpp +++ b/src/script/api/script_engine.cpp @@ -30,7 +30,7 @@ /* AIs have only access to engines they can purchase or still have in use. * Deity has access to all engined that will be or were available ever. */ CompanyID company = ScriptObject::GetCompany(); - return ScriptCompanyMode::IsDeity() || ::IsEngineBuildable(engine_id, e->type, company) || ::Company::Get(company)->group_all[e->type].num_engines[engine_id] > 0; + return ScriptCompanyMode::IsDeity() || ::IsEngineBuildable(engine_id, e->type, company) || ::Company::Get(company)->group_all[e->type].GetNumEngines(engine_id) > 0; } /* static */ bool ScriptEngine::IsBuildable(EngineID engine_id)