From 3038e2975b427f109299b6b80626cea31f3b338b Mon Sep 17 00:00:00 2001 From: Peter Nelson Date: Wed, 13 Nov 2024 17:10:35 +0000 Subject: [PATCH] Change: Make NewGRF persistent storage variably sized. Replace fixed array of 256 elements with a vector. This potentially reduces the amount of memory (and savegame space) required as usually the full 256 entries are not used. --- src/newgrf_debug_gui.cpp | 10 ++++--- src/newgrf_storage.cpp | 38 ++++++++++++++++++++++++- src/newgrf_storage.h | 50 ++++++--------------------------- src/saveload/afterload.cpp | 23 ++------------- src/saveload/industry_sl.cpp | 10 +++---- src/saveload/saveload.h | 1 + src/saveload/station_sl.cpp | 10 +++---- src/saveload/storage_sl.cpp | 53 +++++++++++++++++++++++++++++++++-- src/table/newgrf_debug_data.h | 6 ++-- 9 files changed, 119 insertions(+), 82 deletions(-) diff --git a/src/newgrf_debug_gui.cpp b/src/newgrf_debug_gui.cpp index cdeb928f89..c603850266 100644 --- a/src/newgrf_debug_gui.cpp +++ b/src/newgrf_debug_gui.cpp @@ -186,7 +186,7 @@ public: * @param grfid Parameter for the PSA. Only required for items with parameters. * @return Span of the storage array or an empty span when not present. */ - virtual const std::span GetPSA([[maybe_unused]] uint index, [[maybe_unused]] uint32_t grfid) const + virtual std::span GetPSA([[maybe_unused]] uint index, [[maybe_unused]] uint32_t grfid) const { return {}; } @@ -468,9 +468,11 @@ struct NewGRFInspectWindow : Window { } else { this->DrawString(r, i++, "Persistent storage:"); } - assert(psa.size() % 4 == 0); - for (size_t j = 0; j < psa.size(); j += 4) { - this->DrawString(r, i++, fmt::format(" {}: {} {} {} {}", j, psa[j], psa[j + 1], psa[j + 2], psa[j + 3])); + for (uint index = 0; const int32_t &value : psa) { + if (value != 0) { + this->DrawString(r, i++, fmt::format(" {:02x}: {:08x} ({})", index, value, value)); + } + ++index; } } diff --git a/src/newgrf_storage.cpp b/src/newgrf_storage.cpp index ff309a66df..f62eff6a3b 100644 --- a/src/newgrf_storage.cpp +++ b/src/newgrf_storage.cpp @@ -39,11 +39,47 @@ BasePersistentStorageArray::~BasePersistentStorageArray() * arrays, which saves quite a few clears, etc. after callbacks. * @param storage the array that has changed */ -void AddChangedPersistentStorage(BasePersistentStorageArray *storage) +static void AddChangedPersistentStorage(BasePersistentStorageArray *storage) { _changed_storage_arrays->insert(storage); } +/** + * Stores some value at a given position. + * If there is no backup of the data that backup is made and then + * we write the data. + * @param pos the position to write at + * @param value the value to write + */ +void PersistentStorageArray::StoreValue(uint pos, int32_t value) +{ + /* Out of the scope of the array */ + if (pos >= PersistentStorageArray::SIZE) return; + + if (pos >= std::size(this->storage)) { + if (value == 0) return; + + this->storage.resize(pos + 1); + } else { + /* The value hasn't changed, so we pretend nothing happened. + * Saves a few cycles and such and it's pretty easy to check. */ + if (value == this->storage[pos]) return; + } + + /* We do not have made a backup; lets do so */ + if (AreChangesPersistent()) { + assert(!this->prev_storage); + } else if (!this->prev_storage) { + this->prev_storage = std::make_unique(this->storage); + + /* We only need to register ourselves when we made the backup + * as that is the only time something will have changed */ + AddChangedPersistentStorage(this); + } + + this->storage[pos] = value; +} + /** * Clear temporary changes made since the last call to SwitchMode, and * set whether subsequent changes shall be persistent or temporary. diff --git a/src/newgrf_storage.h b/src/newgrf_storage.h index 13b6ff8f1f..cd957c3fd6 100644 --- a/src/newgrf_storage.h +++ b/src/newgrf_storage.h @@ -60,59 +60,30 @@ private: /** * Class for persistent storage of data. * On #ClearChanges that data is either reverted or saved. - * @tparam SIZE the size of the array. */ -template struct PersistentStorageArray : BasePersistentStorageArray { - using StorageType = std::array; + using StorageType = std::vector; + static constexpr size_t SIZE = 256; StorageType storage{}; ///< Memory for the storage array std::unique_ptr prev_storage{}; ///< Temporary memory to store previous state so it can be reverted, e.g. for command tests. - /** - * Stores some value at a given position. - * If there is no backup of the data that backup is made and then - * we write the data. - * @param pos the position to write at - * @param value the value to write - */ - void StoreValue(uint pos, int32_t value) - { - /* Out of the scope of the array */ - if (pos >= SIZE) return; - - /* The value hasn't changed, so we pretend nothing happened. - * Saves a few cycles and such and it's pretty easy to check. */ - if (this->storage[pos] == value) return; - - /* We do not have made a backup; lets do so */ - if (AreChangesPersistent()) { - assert(!this->prev_storage); - } else if (!this->prev_storage) { - this->prev_storage = std::make_unique(this->storage); - - /* We only need to register ourselves when we made the backup - * as that is the only time something will have changed */ - AddChangedPersistentStorage(this); - } - - this->storage[pos] = value; - } + void StoreValue(uint pos, int32_t value); /** * Gets the value from a given position. * @param pos the position to get the data from * @return the data from that position */ - int32_t GetValue(uint pos) const + inline int32_t GetValue(uint pos) const { /* Out of the scope of the array */ - if (pos >= SIZE) return 0; + if (pos >= std::size(this->storage)) return 0; return this->storage[pos]; } - void ClearChanges() override + inline void ClearChanges() override { if (this->prev_storage) { this->storage = *this->prev_storage; @@ -180,10 +151,6 @@ struct TemporaryStorageArray { } }; -void AddChangedPersistentStorage(BasePersistentStorageArray *storage); - -typedef PersistentStorageArray<16> OldPersistentStorage; - typedef uint32_t PersistentStorageID; struct PersistentStorage; @@ -194,7 +161,7 @@ extern PersistentStoragePool _persistent_storage_pool; /** * Class for pooled persistent storage of data. */ -struct PersistentStorage : PersistentStorageArray<256>, PersistentStoragePool::PoolItem<&_persistent_storage_pool> { +struct PersistentStorage : PersistentStorageArray, PersistentStoragePool::PoolItem<&_persistent_storage_pool> { /** We don't want GCC to zero our struct! It already is zeroed and has an index! */ PersistentStorage(const uint32_t new_grfid, uint8_t feature, TileIndex tile) { @@ -204,6 +171,7 @@ struct PersistentStorage : PersistentStorageArray<256>, PersistentStoragePool::P } }; -static_assert(std::tuple_size_v <= std::tuple_size_v); +/* storage_sl.cpp */ +PersistentStorage *ConvertOldPersistentStorage(std::span old_storage); #endif /* NEWGRF_STORAGE_H */ diff --git a/src/saveload/afterload.cpp b/src/saveload/afterload.cpp index de104efa0c..40da48d437 100644 --- a/src/saveload/afterload.cpp +++ b/src/saveload/afterload.cpp @@ -2800,16 +2800,7 @@ bool AfterLoadGame() for (Industry *ind : Industry::Iterate()) { assert(ind->psa != nullptr); - /* Check if the old storage was empty. */ - bool is_empty = true; - for (uint i = 0; i < sizeof(ind->psa->storage); i++) { - if (ind->psa->GetValue(i) != 0) { - is_empty = false; - break; - } - } - - if (!is_empty) { + if (!ind->psa->storage.empty()) { ind->psa->grfid = _industry_mngr.GetGRFID(ind->type); } else { delete ind->psa; @@ -2823,21 +2814,11 @@ bool AfterLoadGame() if (!(st->facilities & FACIL_AIRPORT)) continue; assert(st->airport.psa != nullptr); - /* Check if the old storage was empty. */ - bool is_empty = true; - for (uint i = 0; i < sizeof(st->airport.psa->storage); i++) { - if (st->airport.psa->GetValue(i) != 0) { - is_empty = false; - break; - } - } - - if (!is_empty) { + if (!st->airport.psa->storage.empty()) { st->airport.psa->grfid = _airport_mngr.GetGRFID(st->airport.type); } else { delete st->airport.psa; st->airport.psa = nullptr; - } } } diff --git a/src/saveload/industry_sl.cpp b/src/saveload/industry_sl.cpp index 409631911b..9cb11d6570 100644 --- a/src/saveload/industry_sl.cpp +++ b/src/saveload/industry_sl.cpp @@ -17,7 +17,8 @@ #include "../safeguards.h" -static OldPersistentStorage _old_ind_persistent_storage; +/** Old persistent storage for industries was a fixed array of 16 elements. */ +static std::array _old_ind_persistent_storage; class SlIndustryAccepted : public VectorSaveLoadHandler { public: @@ -156,7 +157,7 @@ static const SaveLoad _industry_desc[] = { SLE_CONDVAR(Industry, exclusive_supplier, SLE_UINT8, SLV_GS_INDUSTRY_CONTROL, SL_MAX_VERSION), SLE_CONDVAR(Industry, exclusive_consumer, SLE_UINT8, SLV_GS_INDUSTRY_CONTROL, SL_MAX_VERSION), - SLEG_CONDARR("storage", _old_ind_persistent_storage.storage, SLE_UINT32, 16, SLV_76, SLV_161), + SLEG_CONDARR("storage", _old_ind_persistent_storage, SLE_FILE_U32 | SLE_VAR_I32, 16, SLV_76, SLV_161), SLE_CONDREF(Industry, psa, REF_STORAGE, SLV_161, SL_MAX_VERSION), SLE_CONDVAR(Industry, random, SLE_UINT16, SLV_82, SL_MAX_VERSION), @@ -207,6 +208,7 @@ struct INDYChunkHandler : ChunkHandler { { const std::vector slt = SlCompatTableHeader(_industry_desc, _industry_sl_compat); + _old_ind_persistent_storage.fill(0); int index; SlIndustryAccepted::ResetOldStructure(); @@ -219,9 +221,7 @@ struct INDYChunkHandler : ChunkHandler { /* Before savegame version 161, persistent storages were not stored in a pool. */ if (IsSavegameVersionBefore(SLV_161) && !IsSavegameVersionBefore(SLV_76)) { /* Store the old persistent storage. The GRFID will be added later. */ - assert(PersistentStorage::CanAllocateItem()); - i->psa = new PersistentStorage(0, 0, TileIndex{}); - std::copy(std::begin(_old_ind_persistent_storage.storage), std::end(_old_ind_persistent_storage.storage), std::begin(i->psa->storage)); + i->psa = ConvertOldPersistentStorage(_old_ind_persistent_storage); } if (IsSavegameVersionBefore(SLV_EXTEND_INDUSTRY_CARGO_SLOTS)) { LoadMoveAcceptsProduced(i, INDUSTRY_ORIGINAL_NUM_INPUTS, INDUSTRY_ORIGINAL_NUM_OUTPUTS); diff --git a/src/saveload/saveload.h b/src/saveload/saveload.h index 8caa133ef8..ff4aff172f 100644 --- a/src/saveload/saveload.h +++ b/src/saveload/saveload.h @@ -395,6 +395,7 @@ enum SaveLoadVersion : uint16_t { SLV_PATH_CACHE_FORMAT, ///< 346 PR#12345 Vehicle path cache format changed. SLV_ANIMATED_TILE_STATE_IN_MAP, ///< 347 PR#13082 Animated tile state saved for improved performance. SLV_INCREASE_HOUSE_LIMIT, ///< 348 PR#12288 Increase house limit to 4096. + SLV_VARIABLE_PERSISTENT_STORAGE, ///< 349 PR#10670 NewGRF persistent storage moved from fixed array to dynamic. SL_MAX_VERSION, ///< Highest possible saveload version }; diff --git a/src/saveload/station_sl.cpp b/src/saveload/station_sl.cpp index 998e488ab4..0ce2115931 100644 --- a/src/saveload/station_sl.cpp +++ b/src/saveload/station_sl.cpp @@ -182,7 +182,8 @@ struct FlowSaveLoad { typedef std::pair > StationCargoPair; -static OldPersistentStorage _old_st_persistent_storage; +/** Old persistent storage for stations was a fixed array of 16 elements. */ +static std::array _old_st_persistent_storage; /** * Swap the temporary packets with the packets without specific destination in @@ -399,9 +400,7 @@ public: /* Before savegame version 161, persistent storages were not stored in a pool. */ if (IsSavegameVersionBefore(SLV_161) && !IsSavegameVersionBefore(SLV_145) && st->facilities & FACIL_AIRPORT) { /* Store the old persistent storage. The GRFID will be added later. */ - assert(PersistentStorage::CanAllocateItem()); - st->airport.psa = new PersistentStorage(0, 0, TileIndex{}); - std::copy(std::begin(_old_st_persistent_storage.storage), std::end(_old_st_persistent_storage.storage), std::begin(st->airport.psa->storage)); + st->airport.psa = ConvertOldPersistentStorage(_old_st_persistent_storage); } auto end = std::next(std::begin(st->goods), std::min(this->GetNumCargo(), std::size(st->goods))); @@ -610,7 +609,7 @@ public: SLE_CONDVAR(Station, airport.layout, SLE_UINT8, SLV_145, SL_MAX_VERSION), SLE_VAR(Station, airport.flags, SLE_UINT64), SLE_CONDVAR(Station, airport.rotation, SLE_UINT8, SLV_145, SL_MAX_VERSION), - SLEG_CONDARR("storage", _old_st_persistent_storage.storage, SLE_UINT32, 16, SLV_145, SLV_161), + SLEG_CONDARR("storage", _old_st_persistent_storage, SLE_FILE_U32 | SLE_VAR_I32, 16, SLV_145, SLV_161), SLE_CONDREF(Station, airport.psa, REF_STORAGE, SLV_161, SL_MAX_VERSION), SLE_VAR(Station, indtype, SLE_UINT8), @@ -709,6 +708,7 @@ struct STNNChunkHandler : ChunkHandler { { const std::vector slt = SlCompatTableHeader(_station_desc, _station_sl_compat); + _old_st_persistent_storage.fill(0); _old_num_flows = 0; int index; diff --git a/src/saveload/storage_sl.cpp b/src/saveload/storage_sl.cpp index 772ef76989..8ca915d748 100644 --- a/src/saveload/storage_sl.cpp +++ b/src/saveload/storage_sl.cpp @@ -16,11 +16,51 @@ #include "../safeguards.h" +/** + * Convert old fixed-sized array of persistent storage. + * @param old_storage Span containing range of old persistent storage. + * @returns Pointer to pool-allocated PersistentStorage object. + */ +PersistentStorage *ConvertOldPersistentStorage(std::span old_storage) +{ + /* Find last non-zero value to truncate the storage. */ + auto last = std::find_if(std::rbegin(old_storage), std::rend(old_storage), [](uint32_t value) { return value != 0; }).base(); + if (last == std::begin(old_storage)) return nullptr; + + assert(PersistentStorage::CanAllocateItem()); + PersistentStorage *ps = new PersistentStorage(0, 0, TileIndex{}); + + ps->storage.reserve(std::distance(std::begin(old_storage), last)); + for (auto it = std::begin(old_storage); it != last; ++it) { + ps->storage.push_back(*it); + } + + return ps; +} + +class SlPersistentStorage : public VectorSaveLoadHandler { +public: + struct PersistentStorageWrapper { + int32_t value; + }; + + inline static const SaveLoad description[] = { + SLE_VAR(PersistentStorageWrapper, value, SLE_INT32), + }; + inline const static SaveLoadCompatTable compat_description = {}; + + std::vector &GetVector(PersistentStorage *ps) const override { return ps->storage; } +}; + +/** Old persistent storage was a fixed array of up to 256 elements. */ +static std::array _old_persistent_storage; + /** Description of the data to save and load in #PersistentStorage. */ static const SaveLoad _storage_desc[] = { SLE_CONDVAR(PersistentStorage, grfid, SLE_UINT32, SLV_6, SL_MAX_VERSION), - SLE_CONDARR(PersistentStorage, storage, SLE_UINT32, 16, SLV_161, SLV_EXTEND_PERSISTENT_STORAGE), - SLE_CONDARR(PersistentStorage, storage, SLE_UINT32, 256, SLV_EXTEND_PERSISTENT_STORAGE, SL_MAX_VERSION), + SLEG_CONDARR("storage", _old_persistent_storage, SLE_FILE_U32 | SLE_VAR_I32, 16, SLV_161, SLV_EXTEND_PERSISTENT_STORAGE), + SLEG_CONDARR("storage", _old_persistent_storage, SLE_FILE_U32 | SLE_VAR_I32, 256, SLV_EXTEND_PERSISTENT_STORAGE, SLV_VARIABLE_PERSISTENT_STORAGE), + SLEG_CONDSTRUCTLIST("storage", SlPersistentStorage, SLV_VARIABLE_PERSISTENT_STORAGE, SL_MAX_VERSION), }; /** Persistent storage data. */ @@ -31,12 +71,21 @@ struct PSACChunkHandler : ChunkHandler { { const std::vector slt = SlCompatTableHeader(_storage_desc, _storage_sl_compat); + _old_persistent_storage.fill(0); int index; while ((index = SlIterateArray()) != -1) { assert(PersistentStorage::CanAllocateItem()); PersistentStorage *ps = new (index) PersistentStorage(0, 0, TileIndex{}); SlObject(ps, slt); + + if (IsSavegameVersionBefore(SLV_VARIABLE_PERSISTENT_STORAGE)) { + auto last = std::find_if(std::rbegin(_old_persistent_storage), std::rend(_old_persistent_storage), [](uint32_t value) { return value != 0; }).base(); + ps->storage.reserve(std::distance(std::begin(_old_persistent_storage), last)); + for (auto it = std::begin(_old_persistent_storage); it != last; ++it) { + ps->storage.push_back(*it); + } + } } } diff --git a/src/table/newgrf_debug_data.h b/src/table/newgrf_debug_data.h index 641b059afc..91b66c8f11 100644 --- a/src/table/newgrf_debug_data.h +++ b/src/table/newgrf_debug_data.h @@ -381,7 +381,7 @@ class NIHIndustry : public NIHelper { return ro.GetScope(VSG_SCOPE_SELF)->GetVariable(var, param, avail); } - const std::span GetPSA(uint index, uint32_t) const override + std::span GetPSA(uint index, uint32_t) const override { const Industry *i = (const Industry *)this->GetInstance(index); if (i->psa == nullptr) return {}; @@ -557,7 +557,7 @@ class NIHAirport : public NIHelper { return ro.GetScope(VSG_SCOPE_SELF)->GetVariable(var, param, avail); } - const std::span GetPSA(uint index, uint32_t) const override + std::span GetPSA(uint index, uint32_t) const override { const Station *st = (const Station *)this->GetInstance(index); if (st->airport.psa == nullptr) return {}; @@ -603,7 +603,7 @@ class NIHTown : public NIHelper { return ro.GetScope(VSG_SCOPE_SELF)->GetVariable(var, param, avail); } - const std::span GetPSA(uint index, uint32_t grfid) const override + std::span GetPSA(uint index, uint32_t grfid) const override { Town *t = Town::Get(index);