From b77b5cbb5ea35c0baab224fbeaa0f32d8e3ffce2 Mon Sep 17 00:00:00 2001 From: Pavel Solodovnikov Date: Sun, 3 Mar 2024 12:32:56 +0300 Subject: [PATCH 1/5] PagedEntityContainer: make number of elements per page configurable The default value is 1024 elements per page. Signed-off-by: Pavel Solodovnikov --- lib/framework/paged_entity_container.h | 42 ++++++++++++++------------ 1 file changed, 22 insertions(+), 20 deletions(-) diff --git a/lib/framework/paged_entity_container.h b/lib/framework/paged_entity_container.h index 6d76862799c..9687fe10618 100644 --- a/lib/framework/paged_entity_container.h +++ b/lib/framework/paged_entity_container.h @@ -42,7 +42,8 @@ /// As noted above, the container allocates memory in fixed-size /// continuous chunks, or pages, hence the name. /// -/// Currently, each page is set to hold exactly 1024 elements. +/// Each page is set to hold exactly `MaxElementsPerPage` elements, +/// which is 1024 by default. /// /// Also, each element is equipped with some additional metadata, /// which allows the container to reuse the same memory (also called "slots") @@ -97,14 +98,14 @@ /// * https://github.com/Masstronaut/slot_array/blob/master/slot_map.hpp /// /// Entity type. Should be a complete type. -template +/// The fixed number of elements each page may hold. +template class PagedEntityContainer { using SlotIndexType = size_t; - static constexpr size_t MAX_ELEMENTS_PER_PAGE = 1024; // Default initial capacity is exactly 1 page. - static constexpr size_t DEFAULT_INITIAL_CAPACITY = 1 * MAX_ELEMENTS_PER_PAGE; + static constexpr size_t DEFAULT_INITIAL_CAPACITY = 1 * MaxElementsPerPage; static constexpr SlotIndexType INVALID_SLOT_IDX = std::numeric_limits::max(); static constexpr size_t INVALID_PAGE_IDX = std::numeric_limits::max(); @@ -174,7 +175,7 @@ class PagedEntityContainer /// This class holds the actual contiguous storage and metadata for the elements, /// as well as the queue for recycled indices. /// - /// Always allocates storage for exactly `MAX_ELEMENTS_PER_PAGE` elements. + /// Always allocates storage for exactly `MaxElementsPerPage` elements. /// class Page { @@ -195,7 +196,7 @@ class PagedEntityContainer bool is_full() const { - return _currentSize + _expiredSlotsCount == MAX_ELEMENTS_PER_PAGE; + return _currentSize + _expiredSlotsCount == MaxElementsPerPage; } void allocate_storage() @@ -203,9 +204,9 @@ class PagedEntityContainer assert(_storage == nullptr); assert(_slotMetadata == nullptr); - // Allocate storage for MAX_ELEMENTS_PER_PAGE elements. - _storage = std::make_unique(MAX_ELEMENTS_PER_PAGE); - _slotMetadata = std::make_unique(MAX_ELEMENTS_PER_PAGE); + // Allocate storage for `MaxElementsPerPage` elements. + _storage = std::make_unique(MaxElementsPerPage); + _slotMetadata = std::make_unique(MaxElementsPerPage); } bool has_recycled_indices() const @@ -274,7 +275,7 @@ class PagedEntityContainer bool is_expired() const { - return _expiredSlotsCount == MAX_ELEMENTS_PER_PAGE; + return _expiredSlotsCount == MaxElementsPerPage; } // Reset generations to least possible valid value for all slots, @@ -283,7 +284,7 @@ class PagedEntityContainer { auto* meta = slotMetadata(); assert(meta != nullptr); - for (size_t i = 0; i < MAX_ELEMENTS_PER_PAGE; ++i) + for (size_t i = 0; i < MaxElementsPerPage; ++i) { auto& slot = meta[i]; slot.reset_generation(); @@ -342,7 +343,7 @@ class PagedEntityContainer { return; } - size_t needed_nr_of_pages = (capacity / MAX_ELEMENTS_PER_PAGE) - _pages.size(); + size_t needed_nr_of_pages = (capacity / MaxElementsPerPage) - _pages.size(); while (needed_nr_of_pages-- != 0) { allocate_new_page(); @@ -354,7 +355,7 @@ class PagedEntityContainer Page newPage; newPage.allocate_storage(); _pages.emplace_back(std::move(newPage)); - _capacity += MAX_ELEMENTS_PER_PAGE; + _capacity += MaxElementsPerPage; } template @@ -682,7 +683,7 @@ class PagedEntityContainer } // Shrink the storage to just a single page. _pages.resize(1); - _capacity = MAX_ELEMENTS_PER_PAGE; + _capacity = MaxElementsPerPage; // No valid items in the container now. _maxIndex = INVALID_SLOT_IDX; _pages.front().reset_metadata(); @@ -742,12 +743,12 @@ class PagedEntityContainer static PageIndex global_to_page_index(SlotIndexType idx) { - return {idx / MAX_ELEMENTS_PER_PAGE, idx % MAX_ELEMENTS_PER_PAGE}; + return { idx / MaxElementsPerPage, idx % MaxElementsPerPage }; } static SlotIndexType page_index_to_global(const PageIndex& idx) { - return idx.first * MAX_ELEMENTS_PER_PAGE + idx.second; + return idx.first * MaxElementsPerPage + idx.second; } // No need to call destructors manually for trivially destructible types. @@ -793,9 +794,10 @@ class PagedEntityContainer size_t _expiredSlotsCount = 0; }; -template -constexpr typename PagedEntityContainer::SlotIndexType PagedEntityContainer::INVALID_SLOT_IDX; +template +constexpr typename PagedEntityContainer::SlotIndexType + PagedEntityContainer::INVALID_SLOT_IDX; -template -constexpr size_t PagedEntityContainer::INVALID_PAGE_IDX; +template +constexpr size_t PagedEntityContainer::INVALID_PAGE_IDX; From 969487927f5eb50e1c7454a636bddaf86d1c4ee4 Mon Sep 17 00:00:00 2001 From: Pavel Solodovnikov Date: Sat, 9 Mar 2024 12:25:35 +0300 Subject: [PATCH 2/5] PagedEntityContainer: add `ReuseSlots` boolean template parameter This allows to disable slot reuse in `PagedEntityContainer`. The feature is temporary and will be used only until we are confident that we don't have any issues in the WZ code base regarding object pointers not updated properly, e.g. when transitioning between the base and offworld missions. So, it's to prevent cases where one object would be overwritten by another and the game code haven't noticed that. It's better to crash/assert rather than silently use one object instead of another, which could lead to unobvious and weird bugs. Signed-off-by: Pavel Solodovnikov --- lib/framework/paged_entity_container.h | 44 ++++++++++++++++++++------ 1 file changed, 34 insertions(+), 10 deletions(-) diff --git a/lib/framework/paged_entity_container.h b/lib/framework/paged_entity_container.h index 9687fe10618..b18f0e41880 100644 --- a/lib/framework/paged_entity_container.h +++ b/lib/framework/paged_entity_container.h @@ -99,7 +99,8 @@ /// /// Entity type. Should be a complete type. /// The fixed number of elements each page may hold. -template +/// If `false`, slots are one-shot and set to expire after single use. +template class PagedEntityContainer { using SlotIndexType = size_t; @@ -150,6 +151,11 @@ class PagedEntityContainer return _isAlive; } + void invalidate() + { + _generation = INVALID_GENERATION; + } + void set_dead() { _isAlive = false; @@ -399,8 +405,11 @@ class PagedEntityContainer { return; } - // Advance slot generation number. - slotMetadata.advance_generation(); + + // Either advance slot generation number or invalidate the slot right away, + // the behavior depends on whether we reuse the slots or not. + advance_slot_generation(slotMetadata); + // Ensure that the element pointed-to by this slot is dead. slotMetadata.set_dead(); @@ -580,7 +589,7 @@ class PagedEntityContainer const_iterator begin() const { - return const_iterator(const_cast*>(this)->begin()); + return const_iterator(const_cast(this)->begin()); } iterator begin() @@ -657,7 +666,7 @@ class PagedEntityContainer const_iterator find(const T& x) const { - return const_iterator(const_cast*>(this)->find(const_cast(x))); + return const_iterator(const_cast(this)->find(const_cast(x))); } void erase(const_iterator it) @@ -787,6 +796,21 @@ class PagedEntityContainer return (reinterpret_cast(addr) % alignof(T)) == 0; } + template = true> + void advance_slot_generation(SlotMetadata& meta) + { + // Advance slot generation number, when `ReuseSlots=true`. + meta.advance_generation(); + } + + // Specialization for the case when `ReuseSlots=false`. + template = true> + void advance_slot_generation(SlotMetadata& meta) + { + // Invalidate slot right away so that it cannot be reused anymore. + meta.invalidate(); + } + std::vector _pages; SlotIndexType _maxIndex = INVALID_SLOT_IDX; size_t _size = 0; @@ -794,10 +818,10 @@ class PagedEntityContainer size_t _expiredSlotsCount = 0; }; -template -constexpr typename PagedEntityContainer::SlotIndexType - PagedEntityContainer::INVALID_SLOT_IDX; +template +constexpr typename PagedEntityContainer::SlotIndexType + PagedEntityContainer::INVALID_SLOT_IDX; -template -constexpr size_t PagedEntityContainer::INVALID_PAGE_IDX; +template +constexpr size_t PagedEntityContainer::INVALID_PAGE_IDX; From de6a149cc7319c824d05d2435aaf59f3a99e4995 Mon Sep 17 00:00:00 2001 From: Pavel Solodovnikov Date: Sun, 3 Mar 2024 12:36:41 +0300 Subject: [PATCH 3/5] Switch `DROID` to use `PagedEntityContainer` as backing storage 1. Split the droid storage into pages containing 256 droids 2. Disable slot reuse to guard against memory-related issues when some object pointers won't get updated properly, e.g. when transitioning between the base and offworld missions. Signed-off-by: Pavel Solodovnikov --- src/component.h | 2 +- src/droid.cpp | 87 ++++++++++++++++++++++++++--------------------- src/droid.h | 7 ++++ src/mechanics.cpp | 2 +- src/objmem.cpp | 38 +++++++++++++++++---- src/objmem.h | 4 +++ src/wzapi.cpp | 1 + 7 files changed, 94 insertions(+), 47 deletions(-) diff --git a/src/component.h b/src/component.h index 6280187759b..65bf2906a58 100644 --- a/src/component.h +++ b/src/component.h @@ -83,7 +83,7 @@ void drawMuzzleFlash(WEAPON sWeap, iIMDShape *weaponImd, iIMDShape *flashImd, PI #define PART_IMD(STATS,DROID,COMPONENT,PLAYER) (STATS[DROID->asBits[COMPONENT]].pIMD) /* Get the chassis imd */ -#define BODY_IMD(DROID,PLAYER) (DROID->getBodyStats()->pIMD) +#define BODY_IMD(DROID,PLAYER) ((DROID)->getBodyStats()->pIMD) /* Get the brain imd - NOTE: Unused!*/ #define BRAIN_IMD(DROID,PLAYER) (DROID->getBrainStats()->pIMD) /* Get the weapon imd */ diff --git a/src/droid.cpp b/src/droid.cpp index 237868c1b14..c0e3f64269a 100644 --- a/src/droid.cpp +++ b/src/droid.cpp @@ -444,14 +444,17 @@ DROID::~DROID() if (psDroid->psGroup) { //free all droids associated with this Transporter - mutating_list_iterate(psDroid->psGroup->psList, [psDroid](DROID* psCurr) + auto& droidContainer = GlobalDroidContainer(); + mutating_list_iterate(psDroid->psGroup->psList, [psDroid, &droidContainer](DROID* psCurr) { if (psCurr == psDroid) { return IterationResult::BREAK_ITERATION; } // This will cause each droid to self-remove from `psGroup->psList`. - delete psCurr; + auto it = droidContainer.find(*psCurr); + ASSERT(it != droidContainer.end(), "Droid not found in the global container!"); + droidContainer.erase(it); return IterationResult::CONTINUE_ITERATION; }); } @@ -1624,90 +1627,90 @@ DROID *reallyBuildDroid(const DROID_TEMPLATE *pTemplate, Position pos, UDWORD pl ASSERT_OR_RETURN(nullptr, player < MAX_PLAYERS, "Invalid player: %" PRIu32 "", player); - DROID *psDroid = new DROID(id, player); - droidSetName(psDroid, getLocalizedStatsName(pTemplate)); + DROID& droid = GlobalDroidContainer().emplace(id, player); + droidSetName(&droid, getLocalizedStatsName(pTemplate)); // Set the droids type - psDroid->droidType = droidTemplateType(pTemplate); // Is set again later to the same thing, in droidSetBits. - psDroid->pos = pos; - psDroid->rot = rot; + droid.droidType = droidTemplateType(pTemplate); // Is set again later to the same thing, in droidSetBits. + droid.pos = pos; + droid.rot = rot; //don't worry if not on homebase cos not being drawn yet if (!onMission) { //set droid height - psDroid->pos.z = map_Height(psDroid->pos.x, psDroid->pos.y); + droid.pos.z = map_Height(droid.pos.x, droid.pos.y); } - if (psDroid->isTransporter() || psDroid->droidType == DROID_COMMAND) + if (droid.isTransporter() || droid.droidType == DROID_COMMAND) { DROID_GROUP *psGrp = grpCreate(); - psGrp->add(psDroid); + psGrp->add(&droid); } // find the highest stored experience // Unless game time is stopped, then we're hopefully loading a game and // don't want to use up recycled experience for the droids we just loaded. if (!gameTimeIsStopped() && - (psDroid->droidType != DROID_CONSTRUCT) && - (psDroid->droidType != DROID_CYBORG_CONSTRUCT) && - (psDroid->droidType != DROID_REPAIR) && - (psDroid->droidType != DROID_CYBORG_REPAIR) && - !psDroid->isTransporter() && - !recycled_experience[psDroid->player].empty()) + (droid.droidType != DROID_CONSTRUCT) && + (droid.droidType != DROID_CYBORG_CONSTRUCT) && + (droid.droidType != DROID_REPAIR) && + (droid.droidType != DROID_CYBORG_REPAIR) && + !droid.isTransporter() && + !recycled_experience[droid.player].empty()) { - psDroid->experience = recycled_experience[psDroid->player].top(); - recycled_experience[psDroid->player].pop(); + droid.experience = recycled_experience[droid.player].top(); + recycled_experience[droid.player].pop(); } else { - psDroid->experience = 0; + droid.experience = 0; } - psDroid->kills = 0; + droid.kills = 0; - droidSetBits(pTemplate, psDroid); + droidSetBits(pTemplate, &droid); //calculate the droids total weight - psDroid->weight = calcDroidWeight(pTemplate); + droid.weight = calcDroidWeight(pTemplate); // Initialise the movement stuff - psDroid->baseSpeed = calcDroidBaseSpeed(pTemplate, psDroid->weight, (UBYTE)player); + droid.baseSpeed = calcDroidBaseSpeed(pTemplate, droid.weight, (UBYTE)player); - initDroidMovement(psDroid); + initDroidMovement(&droid); //allocate 'easy-access' data! - psDroid->body = calcDroidBaseBody(psDroid); // includes upgrades - ASSERT(psDroid->body > 0, "Invalid number of hitpoints"); - psDroid->originalBody = psDroid->body; + droid.body = calcDroidBaseBody(&droid); // includes upgrades + ASSERT(droid.body > 0, "Invalid number of hitpoints"); + droid.originalBody = droid.body; /* Set droid's initial illumination */ - psDroid->sDisplay.imd = BODY_IMD(psDroid, psDroid->player); + droid.sDisplay.imd = BODY_IMD(&droid, droid.player); //don't worry if not on homebase cos not being drawn yet if (!onMission) { /* People always stand upright */ - if (psDroid->droidType != DROID_PERSON) + if (droid.droidType != DROID_PERSON) { - updateDroidOrientation(psDroid); + updateDroidOrientation(&droid); } - visTilesUpdate(psDroid); + visTilesUpdate(&droid); } /* transporter-specific stuff */ - if (psDroid->isTransporter()) + if (droid.isTransporter()) { //add transporter launch button if selected player and not a reinforcable situation if (player == selectedPlayer && !missionCanReEnforce()) { - (void)intAddTransporterLaunch(psDroid); + (void)intAddTransporterLaunch(&droid); } //set droid height to be above the terrain - psDroid->pos.z += TRANSPORTER_HOVER_HEIGHT; + droid.pos.z += TRANSPORTER_HOVER_HEIGHT; /* reset halt secondary order from guard to hold */ - secondarySetState(psDroid, DSO_HALTTYPE, DSS_HALT_HOLD); + secondarySetState(&droid, DSO_HALTTYPE, DSS_HALT_HOLD); } if (player == selectedPlayer) @@ -1716,12 +1719,12 @@ DROID *reallyBuildDroid(const DROID_TEMPLATE *pTemplate, Position pos, UDWORD pl } // Avoid droid appearing to jump or turn on spawn. - psDroid->prevSpacetime.pos = psDroid->pos; - psDroid->prevSpacetime.rot = psDroid->rot; + droid.prevSpacetime.pos = droid.pos; + droid.prevSpacetime.rot = droid.rot; - debug(LOG_LIFE, "created droid for player %d, droid = %p, id=%d (%s): position: x(%d)y(%d)z(%d)", player, static_cast(psDroid), (int)psDroid->id, psDroid->aName, psDroid->pos.x, psDroid->pos.y, psDroid->pos.z); + debug(LOG_LIFE, "created droid for player %d, droid = %p, id=%d (%s): position: x(%d)y(%d)z(%d)", player, static_cast(&droid), (int)droid.id, droid.aName, droid.pos.x, droid.pos.y, droid.pos.z); - return psDroid; + return &droid; } DROID *reallyBuildDroid(const DROID_TEMPLATE *pTemplate, Position pos, UDWORD player, bool onMission, Rotation rot) @@ -3598,3 +3601,9 @@ CONSTRUCT_STATS* DROID::getConstructStats() const { return &asConstructStats[asBits[COMP_CONSTRUCT]]; } + +DroidContainer& GlobalDroidContainer() +{ + static DroidContainer instance; + return instance; +} diff --git a/src/droid.h b/src/droid.h index 1f1156dd63a..194ef03d6a3 100644 --- a/src/droid.h +++ b/src/droid.h @@ -24,6 +24,7 @@ #ifndef __INCLUDED_SRC_DROID_H__ #define __INCLUDED_SRC_DROID_H__ +#include "lib/framework/paged_entity_container.h" #include "lib/framework/string_ext.h" #include "lib/gamelib/gtime.h" @@ -425,4 +426,10 @@ static inline DROID const *castDroid(SIMPLE_OBJECT const *psObject) */ void droidWasFullyRepaired(DROID *psDroid, const REPAIR_FACILITY *psRepairFac); +// Split the droid storage into pages containing 256 droids, disable slot reuse +// to guard against memory-related issues when some object pointers won't get +// updated properly, e.g. when transitioning between the base and offworld missions. +using DroidContainer = PagedEntityContainer; +DroidContainer& GlobalDroidContainer(); + #endif // __INCLUDED_SRC_DROID_H__ diff --git a/src/mechanics.cpp b/src/mechanics.cpp index 35b7f0aa3d2..79c76f65f50 100644 --- a/src/mechanics.cpp +++ b/src/mechanics.cpp @@ -39,7 +39,7 @@ bool mechanicsShutdown() { for (BASE_OBJECT* psObj : psDestroyedObj) { - delete psObj; + objmemDestroy(psObj); } psDestroyedObj.clear(); diff --git a/src/objmem.cpp b/src/objmem.cpp index e54ddc598d5..22bd0bf3d18 100644 --- a/src/objmem.cpp +++ b/src/objmem.cpp @@ -201,7 +201,7 @@ static bool checkReferences(BASE_OBJECT *psVictim) /* Remove an object from the destroyed list, finally freeing its memory * Hopefully by this time, no pointers still refer to it! */ -static bool objmemDestroy(BASE_OBJECT *psObj) +bool objmemDestroy(BASE_OBJECT *psObj) { switch (psObj->type) { @@ -224,8 +224,19 @@ static bool objmemDestroy(BASE_OBJECT *psObj) { return false; } - debug(LOG_MEMORY, "BASE_OBJECT* 0x%p is freed.", static_cast(psObj)); - delete psObj; + // Droids are managed by a separate droid container. + if (psObj->type == OBJ_DROID) + { + auto& droidContainer = GlobalDroidContainer(); + auto it = droidContainer.find(*static_cast(psObj)); + ASSERT(it != droidContainer.end(), "Droid not found in the global container!"); + droidContainer.erase(it); + } + else + { + delete psObj; + } + debug(LOG_MEMORY, "BASE_OBJECT* is freed."); return true; } @@ -439,10 +450,25 @@ void killDroid(DROID *psDel) destroyObject(apsDroidLists, psDel); } +static void freeAllDroidsImpl(PerPlayerDroidLists& droidLists) +{ + auto& droidContainer = GlobalDroidContainer(); + for (auto& list : droidLists) + { + for (DROID* d : list) + { + auto it = droidContainer.find(*d); + ASSERT(it != droidContainer.end(), "Droid not found in the global container!"); + droidContainer.erase(it); + } + list.clear(); + } +} + /* Remove all droids */ void freeAllDroids() { - releaseAllObjectsInList(apsDroidLists); + freeAllDroidsImpl(apsDroidLists); } /*Remove a single Droid from a list*/ @@ -475,13 +501,13 @@ void removeDroid(DROID* psDroidToRemove, PerPlayerDroidLists& pList) /*Removes all droids that may be stored in the mission lists*/ void freeAllMissionDroids() { - releaseAllObjectsInList(mission.apsDroidLists); + freeAllDroidsImpl(mission.apsDroidLists); } /*Removes all droids that may be stored in the limbo lists*/ void freeAllLimboDroids() { - releaseAllObjectsInList(apsLimboDroids); + freeAllDroidsImpl(apsLimboDroids); } /************************** STRUCTURE *******************************/ diff --git a/src/objmem.h b/src/objmem.h index 3d3c4122d04..924dec133d4 100644 --- a/src/objmem.h +++ b/src/objmem.h @@ -74,6 +74,10 @@ void objmemShutdown(); /* General housekeeping for the object system */ void objmemUpdate(); +/* Remove an object from the destroyed list, finally freeing its memory + * Hopefully by this time, no pointers still refer to it! */ +bool objmemDestroy(BASE_OBJECT* psObj); + /// Generates a new, (hopefully) unique object id. uint32_t generateNewObjectId(); /// Generates a new, (hopefully) unique object id, which all clients agree on. diff --git a/src/wzapi.cpp b/src/wzapi.cpp index f15ab9bbd14..7b2fbccf307 100644 --- a/src/wzapi.cpp +++ b/src/wzapi.cpp @@ -2158,6 +2158,7 @@ std::unique_ptr wzapi::getDroidProduction(WZAPI_PARAMS(const STRUCT { return nullptr; } + // Since it's not intended to be used anywhere, don't put it in the global droid storage. DROID *psDroid = new DROID(0, player); psDroid->pos = psStruct->pos; psDroid->rot = psStruct->rot; From e8158276127144101c2fa056de0f50940f8b4c11 Mon Sep 17 00:00:00 2001 From: past-due <30942300+past-due@users.noreply.github.com> Date: Tue, 19 Mar 2024 19:26:46 -0400 Subject: [PATCH 4/5] Clear the PagedEntityContainer in objmemShutdown() --- src/objmem.cpp | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/objmem.cpp b/src/objmem.cpp index 22bd0bf3d18..e491183efbd 100644 --- a/src/objmem.cpp +++ b/src/objmem.cpp @@ -86,6 +86,8 @@ bool objmemInitialise() /* Release the object heaps */ void objmemShutdown() { + auto& droidContainer = GlobalDroidContainer(); + droidContainer.clear(); } // Check that psVictim is not referred to by any other object in the game. We can dump out some extra data in debug builds that help track down sources of dangling pointer errors. From 45e1b3e734df06af2dc11d953591c518f478463a Mon Sep 17 00:00:00 2001 From: Pavel Solodovnikov Date: Wed, 20 Mar 2024 19:47:50 +0300 Subject: [PATCH 5/5] PagedEntityContainer: free memory for expired pages when `ReuseSlots=false` This helps to keep memory consumption under control. Existing expired pages are left in the deactivated state indefinitely instead of reallocating their storage upon expiration, so that we don't break the indexing and memory safety guarantees for the container. Signed-off-by: Pavel Solodovnikov --- lib/framework/paged_entity_container.h | 41 ++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/lib/framework/paged_entity_container.h b/lib/framework/paged_entity_container.h index b18f0e41880..5f03fe7fc0f 100644 --- a/lib/framework/paged_entity_container.h +++ b/lib/framework/paged_entity_container.h @@ -182,6 +182,10 @@ class PagedEntityContainer /// as well as the queue for recycled indices. /// /// Always allocates storage for exactly `MaxElementsPerPage` elements. + /// + /// If the parent container is instantiated with the `ReuseSlots=false` mode, + /// expired pages will deallocate their storage to keep memory consumption + /// under control. /// class Page { @@ -279,11 +283,23 @@ class PagedEntityContainer ++_expiredSlotsCount; } + // When `ReuseSlots=false`, expired pages will also deallocate + // their storage upon reaching "expired" state. bool is_expired() const { return _expiredSlotsCount == MaxElementsPerPage; } + // This method renders the page unusable! + // + // The page will then need to call `allocate_storage()` + `reset_metadata()` + // to be usable once again. + void deallocate_storage() + { + _storage.reset(); + _slotMetadata.reset(); + } + // Reset generations to least possible valid value for all slots, // plus mark all slots as dead, so that the page appears clean and empty. void reset_metadata() @@ -428,6 +444,19 @@ class PagedEntityContainer // Increase counters for expired slots tracking. _pages[pageIdx.first].increase_expired_slots_count(); ++_expiredSlotsCount; + // Looks like at least GCC is smart enough to optimize the following + // instructions away, even when this is not a `constexpr if`, but a regular `if`, + // See https://godbolt.org/z/bY8oEYsdz. + if (!ReuseSlots) + { + if (_pages[pageIdx.first].is_expired()) + { + // Free storage pointers for expired pages when not reusing slots, + // this should alleviate the problem of excessive memory consumption + // in such cases. + _pages[pageIdx.first].deallocate_storage(); + } + } } else { @@ -695,6 +724,18 @@ class PagedEntityContainer _capacity = MaxElementsPerPage; // No valid items in the container now. _maxIndex = INVALID_SLOT_IDX; + if (!ReuseSlots) + { + // If the first page is in the expired state, then + // its storage is deallocated, too. So, reallocate it. + // + // NOTE: expired pages free their storage only when + // the slot recycling mechanism is disabled! + if (_pages.front().is_expired()) + { + _pages.front().allocate_storage(); + } + } _pages.front().reset_metadata(); _expiredSlotsCount = 0; }