From f56b6756f1d0df1beb652c54749276838406e2c7 Mon Sep 17 00:00:00 2001 From: Peter Nelson Date: Thu, 21 Nov 2024 22:05:08 +0000 Subject: [PATCH] Codechange: Sort EngineOverrideManager for fast lookups. Allows quickly finding the EngineID given the type, grfid and local id of an engine, instead a linear scan. This can reduce loading time when lots of engines are present and also affects performance in-game. Lookup can be on the order of 10000 times faster. --- src/engine.cpp | 81 ++++++++++++++++++++++++++++---------- src/engine_base.h | 20 +++++++--- src/newgrf.cpp | 22 +++-------- src/newgrf_engine.cpp | 16 ++++---- src/saveload/engine_sl.cpp | 32 +++++++++++---- 5 files changed, 112 insertions(+), 59 deletions(-) diff --git a/src/engine.cpp b/src/engine.cpp index f8d3710d33..63761c7e8e 100644 --- a/src/engine.cpp +++ b/src/engine.cpp @@ -8,6 +8,7 @@ /** @file engine.cpp Base for all engine handling. */ #include "stdafx.h" +#include "core/container_func.hpp" #include "company_func.h" #include "command_func.h" #include "news_func.h" @@ -67,8 +68,6 @@ const uint8_t _engine_offsets[4] = { static_assert(lengthof(_orig_rail_vehicle_info) + lengthof(_orig_road_vehicle_info) + lengthof(_orig_ship_vehicle_info) + lengthof(_orig_aircraft_vehicle_info) == lengthof(_orig_engine_info)); -const uint EngineOverrideManager::NUM_DEFAULT_ENGINES = _engine_counts[VEH_TRAIN] + _engine_counts[VEH_ROAD] + _engine_counts[VEH_SHIP] + _engine_counts[VEH_AIRCRAFT]; - Engine::Engine(VehicleType type, EngineID base) { this->type = type; @@ -509,10 +508,12 @@ bool Engine::IsVariantHidden(CompanyID c) const */ void EngineOverrideManager::ResetToDefaultMapping() { - this->mappings.clear(); + EngineID id = 0; for (VehicleType type = VEH_TRAIN; type <= VEH_AIRCRAFT; type++) { - for (uint internal_id = 0; internal_id < _engine_counts[type]; internal_id++) { - this->mappings.emplace_back(INVALID_GRFID, internal_id, type, internal_id); + auto &map = this->mappings[type]; + map.clear(); + for (uint internal_id = 0; internal_id < _engine_counts[type]; internal_id++, id++) { + map.emplace_back(INVALID_GRFID, internal_id, type, internal_id, id); } } } @@ -528,14 +529,52 @@ void EngineOverrideManager::ResetToDefaultMapping() */ EngineID EngineOverrideManager::GetID(VehicleType type, uint16_t grf_local_id, uint32_t grfid) { - EngineID index = 0; - for (const EngineIDMapping &eid : this->mappings) { - if (eid.type == type && eid.grfid == grfid && eid.internal_id == grf_local_id) { - return index; - } - index++; + const auto &map = this->mappings[type]; + const auto key = EngineIDMapping::Key(grfid, grf_local_id); + auto it = std::ranges::lower_bound(map, key, std::less{}, EngineIDMappingKeyProjection{}); + if (it == std::end(map) || it->Key() != key) return INVALID_ENGINE; + return it->engine; +} + +/** + * Look for an unreserved EngineID matching the local id, and reserve it if found. + * @param type Vehicle type + * @param grf_local_id The local id in the newgrf + * @param grfid The GrfID that defines the scope of grf_local_id. + * If a newgrf overrides the engines of another newgrf, the "scope grfid" is the ID of the overridden newgrf. + * If dynnamic_engines is disabled, all newgrf share the same ID scope identified by INVALID_GRFID. + * @param static_access Whether to actually reserve the EngineID. + * @return The engine ID if present and now reserved, or INVALID_ENGINE if not. + */ +EngineID EngineOverrideManager::UseUnreservedID(VehicleType type, uint16_t grf_local_id, uint32_t grfid, bool static_access) +{ + auto &map = _engine_mngr.mappings[type]; + const auto key = EngineIDMapping::Key(INVALID_GRFID, grf_local_id); + auto it = std::ranges::lower_bound(map, key, std::less{}, EngineIDMappingKeyProjection{}); + if (it == std::end(map) || it->Key() != key) return INVALID_ENGINE; + + if (!static_access && grfid != INVALID_GRFID) { + /* Reserve the engine slot for the new grfid. */ + it->grfid = grfid; + + /* Relocate entry to its new position in the mapping list to keep it sorted. */ + auto p = std::ranges::lower_bound(map, EngineIDMapping::Key(grfid, grf_local_id), std::less{}, EngineIDMappingKeyProjection{}); + it = Slide(it, std::next(it), p).first; + } + + return it->engine; +} + +void EngineOverrideManager::SetID(VehicleType type, uint16_t grf_local_id, uint32_t grfid, uint8_t substitute_id, EngineID engine) +{ + auto &map = this->mappings[type]; + const auto key = EngineIDMapping::Key(grfid, grf_local_id); + auto it = std::ranges::lower_bound(map, key, std::less{}, EngineIDMappingKeyProjection{}); + if (it == std::end(map) || it->Key() != key) { + map.emplace(it, grfid, grf_local_id, type, substitute_id, engine); + } else { + it->engine = engine; } - return INVALID_ENGINE; } /** @@ -564,15 +603,15 @@ void SetupEngines() CloseWindowByClass(WC_ENGINE_PREVIEW); _engine_pool.CleanPool(); - assert(_engine_mngr.mappings.size() >= EngineOverrideManager::NUM_DEFAULT_ENGINES); - [[maybe_unused]] uint index = 0; - for (const EngineIDMapping &eid : _engine_mngr.mappings) { - /* Assert is safe; there won't be more than 256 original vehicles - * in any case, and we just cleaned the pool. */ - assert(Engine::CanAllocateItem()); - [[maybe_unused]] const Engine *e = new Engine(eid.type, eid.internal_id); - assert(e->index == index); - index++; + for (VehicleType type = VEH_BEGIN; type != VEH_COMPANY_END; type++) { + const auto &mapping = _engine_mngr.mappings[type]; + + /* Verify that the engine override manager has at least been set up with the default engines. */ + assert(std::size(mapping) >= _engine_counts[type]); + + for (const EngineIDMapping &eid : mapping) { + new (eid.engine) Engine(type, eid.internal_id); + } } } diff --git a/src/engine_base.h b/src/engine_base.h index 3e9b7ab439..1031c6875e 100644 --- a/src/engine_base.h +++ b/src/engine_base.h @@ -198,10 +198,20 @@ struct EngineIDMapping { uint16_t internal_id; ///< The internal ID within the GRF file VehicleType type; ///< The engine type uint8_t substitute_id; ///< The (original) entity ID to use if this GRF is not available (currently not used) + EngineID engine; + + static inline uint64_t Key(uint32_t grfid, uint16_t internal_id) { return static_cast(grfid) << 32 | internal_id; } + + inline uint64_t Key() const { return Key(this->grfid, this->internal_id); } EngineIDMapping() {} - EngineIDMapping(uint32_t grfid, uint16_t internal_id, VehicleType type, uint8_t substitute_id) - : grfid(grfid), internal_id(internal_id),type(type), substitute_id(substitute_id) {} + EngineIDMapping(uint32_t grfid, uint16_t internal_id, VehicleType type, uint8_t substitute_id, EngineID engine) + : grfid(grfid), internal_id(internal_id), type(type), substitute_id(substitute_id), engine(engine) {} +}; + +/** Projection to get a unique key of an EngineIDMapping, used for sorting in EngineOverrideManager. */ +struct EngineIDMappingKeyProjection { + inline size_t operator()(const EngineIDMapping &eid) const { return eid.Key(); } }; /** @@ -209,12 +219,12 @@ struct EngineIDMapping { * Note: This is not part of Engine, as the data in the EngineOverrideManager and the engine pool get resetted in different cases. */ struct EngineOverrideManager { - std::vector mappings; - - static const uint NUM_DEFAULT_ENGINES; ///< Number of default entries + std::array, VEH_COMPANY_END> mappings; void ResetToDefaultMapping(); EngineID GetID(VehicleType type, uint16_t grf_local_id, uint32_t grfid); + EngineID UseUnreservedID(VehicleType type, uint16_t grf_local_id, uint32_t grfid, bool static_access); + void SetID(VehicleType type, uint16_t grf_local_id, uint32_t grfid, uint8_t substitute_id, EngineID engine); static bool ResetToCurrentNewGRFConfig(); }; diff --git a/src/newgrf.cpp b/src/newgrf.cpp index bbc623eeed..d217c4da1e 100644 --- a/src/newgrf.cpp +++ b/src/newgrf.cpp @@ -641,7 +641,7 @@ static Engine *GetNewEngine(const GRFFile *file, VehicleType type, uint16_t inte } /* Check if there is an unreserved slot */ - EngineID engine = _engine_mngr.GetID(type, internal_id, INVALID_GRFID); + EngineID engine = _engine_mngr.UseUnreservedID(type, internal_id, scope_grfid, static_access); if (engine != INVALID_ENGINE) { Engine *e = Engine::Get(engine); @@ -651,12 +651,6 @@ static Engine *GetNewEngine(const GRFFile *file, VehicleType type, uint16_t inte GrfMsg(5, "Replaced engine at index {} for GRFID {:x}, type {}, index {}", e->index, BSWAP32(file->grfid), type, internal_id); } - /* Reserve the engine slot */ - if (!static_access) { - EngineIDMapping &eid = _engine_mngr.mappings[engine]; - eid.grfid = scope_grfid; // Note: this is INVALID_GRFID if dynamic_engines is disabled, so no reservation - } - return e; } @@ -675,13 +669,7 @@ static Engine *GetNewEngine(const GRFFile *file, VehicleType type, uint16_t inte e->grf_prop.grffile = file; /* Reserve the engine slot */ - assert(_engine_mngr.mappings.size() == e->index); - _engine_mngr.mappings.emplace_back( - scope_grfid, // Note: this is INVALID_GRFID if dynamic_engines is disabled, so no reservation - internal_id, - type, - std::min(internal_id, _engine_counts[type]) // substitute_id == _engine_counts[subtype] means "no substitute" - ); + _engine_mngr.SetID(type, internal_id, scope_grfid, std::min(internal_id, _engine_counts[type]), e->index); if (engine_pool_size != Engine::GetPoolSize()) { /* Resize temporary engine data ... */ @@ -9250,8 +9238,8 @@ static void FinaliseEngineArray() { for (Engine *e : Engine::Iterate()) { if (e->GetGRF() == nullptr) { - const EngineIDMapping &eid = _engine_mngr.mappings[e->index]; - if (eid.grfid != INVALID_GRFID || eid.internal_id != eid.substitute_id) { + auto found = std::ranges::find(_engine_mngr.mappings[e->type], e->index, &EngineIDMapping::engine); + if (found == std::end(_engine_mngr.mappings[e->type]) || found->grfid != INVALID_GRFID || found->internal_id != found->substitute_id) { e->info.string_id = STR_NEWGRF_INVALID_ENGINE; } } @@ -9302,7 +9290,7 @@ static void FinaliseEngineArray() /* Engine looped back on itself, so clear the variant. */ e->info.variant_id = INVALID_ENGINE; - GrfMsg(1, "FinaliseEngineArray: Variant of engine {:x} in '{}' loops back on itself", _engine_mngr.mappings[e->index].internal_id, e->GetGRF()->filename); + GrfMsg(1, "FinaliseEngineArray: Variant of engine {:x} in '{}' loops back on itself", e->grf_prop.local_id, e->GetGRF()->filename); break; } diff --git a/src/newgrf_engine.cpp b/src/newgrf_engine.cpp index 3be7c65101..1d78504f88 100644 --- a/src/newgrf_engine.cpp +++ b/src/newgrf_engine.cpp @@ -1311,17 +1311,17 @@ void AlterVehicleListOrder(EngineID engine, uint target) */ static bool EnginePreSort(const EngineID &a, const EngineID &b) { - const EngineIDMapping &id_a = _engine_mngr.mappings.at(a); - const EngineIDMapping &id_b = _engine_mngr.mappings.at(b); + const Engine &engine_a = *Engine::Get(a); + const Engine &engine_b = *Engine::Get(b); /* 1. Sort by engine type */ - if (id_a.type != id_b.type) return (int)id_a.type < (int)id_b.type; + if (engine_a.type != engine_b.type) return static_cast(engine_a.type) < static_cast(engine_b.type); /* 2. Sort by scope-GRFID */ - if (id_a.grfid != id_b.grfid) return id_a.grfid < id_b.grfid; + if (engine_a.grf_prop.grfid != engine_b.grf_prop.grfid) return engine_a.grf_prop.grfid < engine_b.grf_prop.grfid; /* 3. Sort by local ID */ - return (int)id_a.internal_id < (int)id_b.internal_id; + return static_cast(engine_a.grf_prop.local_id) < static_cast(engine_b.grf_prop.local_id); } /** @@ -1341,10 +1341,10 @@ void CommitVehicleListOrderChanges() EngineID source = it.engine; uint local_target = it.target; - const EngineIDMapping &id_source = _engine_mngr.mappings[source]; - if (id_source.internal_id == local_target) continue; + Engine *engine_source = Engine::Get(source); + if (engine_source->grf_prop.local_id == local_target) continue; - EngineID target = _engine_mngr.GetID(id_source.type, local_target, id_source.grfid); + EngineID target = _engine_mngr.GetID(engine_source->type, local_target, engine_source->grf_prop.grfid); if (target == INVALID_ENGINE) continue; int source_index = find_index(ordering, source); diff --git a/src/saveload/engine_sl.cpp b/src/saveload/engine_sl.cpp index b3584d4f8e..8571d2f5fe 100644 --- a/src/saveload/engine_sl.cpp +++ b/src/saveload/engine_sl.cpp @@ -189,11 +189,25 @@ struct EIDSChunkHandler : ChunkHandler { { SlTableHeader(_engine_id_mapping_desc); - uint index = 0; - for (EngineIDMapping &eid : _engine_mngr.mappings) { - SlSetArrayIndex(index); + /* Count total entries needed for combined list. */ + size_t total = 0; + for (const auto &mapping : _engine_mngr.mappings) { + total += std::size(mapping); + } + + /* Combine per-type mappings into single list for all types. */ + std::vector temp; + temp.reserve(total); + for (const auto &mapping : _engine_mngr.mappings) { + temp.insert(std::end(temp), std::begin(mapping), std::end(mapping)); + } + + /* Sort combined list by EngineID */ + std::ranges::sort(temp, std::less{}, &EngineIDMapping::engine); + + for (EngineIDMapping &eid : temp) { + SlSetArrayIndex(eid.engine); SlObject(&eid, _engine_id_mapping_desc); - index++; } } @@ -201,11 +215,13 @@ struct EIDSChunkHandler : ChunkHandler { { const std::vector slt = SlCompatTableHeader(_engine_id_mapping_desc, _engine_id_mapping_sl_compat); - _engine_mngr.mappings.clear(); + _engine_mngr.mappings = {}; - while (SlIterateArray() != -1) { - EngineIDMapping *eid = &_engine_mngr.mappings.emplace_back(); - SlObject(eid, slt); + int index; + while ((index = SlIterateArray()) != -1) { + EngineIDMapping eid; + SlObject(&eid, slt); + _engine_mngr.SetID(eid.type, eid.internal_id, eid.grfid, eid.substitute_id, index); } } };