Skip to content

Commit

Permalink
Switch STRUCTURE to use PagedEntityContainer<STRUCTURE> as backin…
Browse files Browse the repository at this point in the history 
…g storage

1. Split the structure storage into pages containing 256 structs
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 <[email protected]>
  • Loading branch information
@ManManson
ManManson committed Mar 23, 2024
1 parent c3381b0 commit ce622e1
Show file tree
Hide file tree
Showing 4 changed files with 119 additions and 60 deletions.
9 changes: 7 additions & 2 deletions src/display3d.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,8 @@ struct Blueprint
STRUCTURE *psStruct = buildBlueprint();
ASSERT_OR_RETURN(, psStruct != nullptr, "buildBlueprint returned nullptr");
renderStructure(psStruct, viewMatrix, perspectiveViewMatrix);
delete psStruct;

objmemDestroy(psStruct, false);
}
}

Expand Down Expand Up @@ -692,7 +693,11 @@ STRUCTURE *getTileBlueprintStructure(int mapX, int mapY)
Blueprint blueprint = getTileBlueprint(mapX, mapY);
if (blueprint.state == SS_BLUEPRINT_PLANNED)
{
delete psStruct; // Delete previously returned structure, if any.
if (psStruct)
{
// Delete previously returned structure, if any.
objmemDestroy(psStruct, false);
}
psStruct = blueprint.buildBlueprint();
return psStruct; // This blueprint was clicked on.
}
Expand Down
69 changes: 57 additions & 12 deletions src/objmem.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,14 +226,22 @@ bool objmemDestroy(BASE_OBJECT *psObj, bool checkRefs)
{
return false;
}
// Droids are managed by a separate droid container.
if (psObj->type == OBJ_DROID)
{
// Droids are managed by a separate droid container.
auto& droidContainer = GlobalDroidContainer();
auto it = droidContainer.find(*static_cast<DROID*>(psObj));
ASSERT(it != droidContainer.end(), "Droid not found in the global container!");
droidContainer.erase(it);
}
else if (psObj->type == OBJ_STRUCTURE)
{
// Structs are managed by a separate struct container.
auto& structContainer = GlobalStructContainer();
auto it = structContainer.find(*static_cast<STRUCTURE*>(psObj));
ASSERT(it != structContainer.end(), "Structure not found in the global container!");
structContainer.erase(it);
}
else
{
delete psObj;
Expand Down Expand Up @@ -452,16 +460,53 @@ void killDroid(DROID *psDel)
destroyObject(apsDroidLists, psDel);
}

static void freeAllDroidsImpl(PerPlayerDroidLists& droidLists)
template <typename EntityType>
struct GlobalEntityContainerTraits;

template <>
struct GlobalEntityContainerTraits<DROID>
{
auto& droidContainer = GlobalDroidContainer();
for (auto& list : droidLists)
using StorageType = DroidContainer;

static DroidContainer& getContainer()
{
return GlobalDroidContainer();
}

static const char* entityName()
{
return "Droid";
}
};

template <>
struct GlobalEntityContainerTraits<STRUCTURE>
{
using StorageType = StructContainer;

static StructContainer& getContainer()
{
return GlobalStructContainer();
}

static const char* entityName()
{
return "Structure";
}
};

template <typename Entity, unsigned PlayerCount>
static void freeAllEntitiesImpl(PerPlayerObjectLists<Entity, PlayerCount>& entityLists)
{
using Traits = GlobalEntityContainerTraits<Entity>;
auto& entityContainer = Traits::getContainer();
for (auto& list : entityLists)
{
for (DROID* d : list)
for (auto* ent : list)
{
auto it = droidContainer.find(*d);
ASSERT(it != droidContainer.end(), "Droid not found in the global container!");
droidContainer.erase(it);
auto it = entityContainer.find(*ent);
ASSERT(it != entityContainer.end(), "%s not found in the global container!", Traits::entityName());
entityContainer.erase(it);
}
list.clear();
}
Expand All @@ -470,7 +515,7 @@ static void freeAllDroidsImpl(PerPlayerDroidLists& droidLists)
/* Remove all droids */
void freeAllDroids()
{
freeAllDroidsImpl(apsDroidLists);
freeAllEntitiesImpl<DROID, MAX_PLAYERS>(apsDroidLists);
}

/*Remove a single Droid from a list*/
Expand Down Expand Up @@ -503,13 +548,13 @@ void removeDroid(DROID* psDroidToRemove, PerPlayerDroidLists& pList)
/*Removes all droids that may be stored in the mission lists*/
void freeAllMissionDroids()
{
freeAllDroidsImpl(mission.apsDroidLists);
freeAllEntitiesImpl<DROID, MAX_PLAYERS>(mission.apsDroidLists);
}

/*Removes all droids that may be stored in the limbo lists*/
void freeAllLimboDroids()
{
freeAllDroidsImpl(apsLimboDroids);
freeAllEntitiesImpl<DROID, MAX_PLAYERS>(apsLimboDroids);
}

/************************** STRUCTURE *******************************/
Expand Down Expand Up @@ -592,7 +637,7 @@ void killStruct(STRUCTURE *psBuilding)
/* Remove heapall structures */
void freeAllStructs()
{
releaseAllObjectsInList(apsStructLists);
freeAllEntitiesImpl<STRUCTURE, MAX_PLAYERS>(apsStructLists);
}

/*Remove a single Structure from a list*/
Expand Down
95 changes: 49 additions & 46 deletions src/structure.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1414,31 +1414,27 @@ STRUCTURE *buildStructureDir(STRUCTURE_STATS *pStructureType, UDWORD x, UDWORD y
}
}

// allocate memory for and initialize a structure object
psBuilding = new STRUCTURE(id, player);
if (psBuilding == nullptr)
{
return nullptr;
}
// initialize the structure object
STRUCTURE building(id, player);

//fill in other details
psBuilding->pStructureType = pStructureType;
building.pStructureType = pStructureType;

psBuilding->pos.x = x;
psBuilding->pos.y = y;
psBuilding->rot.direction = snapDirection(direction);
psBuilding->rot.pitch = 0;
psBuilding->rot.roll = 0;
building.pos.x = x;
building.pos.y = y;
building.rot.direction = snapDirection(direction);
building.rot.pitch = 0;
building.rot.roll = 0;

//This needs to be done before the functionality bit...
//load into the map data and structure list if not an upgrade
Vector2i map = map_coord(Vector2i(x, y)) - size / 2;

//set up the imd to use for the display
psBuilding->sDisplay.imd = pStructureType->pIMD[0];
building.sDisplay.imd = pStructureType->pIMD[0];

psBuilding->state = SAS_NORMAL;
psBuilding->lastStateTime = gameTime;
building.state = SAS_NORMAL;
building.lastStateTime = gameTime;

/* if resource extractor - need to remove oil feature first, but do not do any
* consistency checking here - save games do not have any feature to remove
Expand All @@ -1452,7 +1448,6 @@ STRUCTURE *buildStructureDir(STRUCTURE_STATS *pStructureType, UDWORD x, UDWORD y
if (fireOnLocation(psFeature->pos.x, psFeature->pos.y))
{
// Can't build on burning oil resource
delete psBuilding;
return nullptr;
}
// remove it from the map
Expand Down Expand Up @@ -1486,16 +1481,18 @@ STRUCTURE *buildStructureDir(STRUCTURE_STATS *pStructureType, UDWORD x, UDWORD y
#if defined(WZ_CC_GNU) && !defined(WZ_CC_INTEL) && !defined(WZ_CC_CLANG) && (7 <= __GNUC__)
# pragma GCC diagnostic pop
#endif
delete psBuilding;
return nullptr;
}
}
}
// Emplace the structure being built in the global storage to obtain stable address.
STRUCTURE& stableBuilding = GlobalStructContainer().emplace(std::move(building));
psBuilding = &stableBuilding;
for (int tileY = map.y; tileY < map.y + size.y; ++tileY)
{
for (int tileX = map.x; tileX < map.x + size.x; ++tileX)
{
// We now know the previous loop didn't return early, so it is safe to save references to psBuilding now.
// We now know the previous loop didn't return early, so it is safe to save references to `stableBuilding` now.
MAPTILE *psTile = mapTile(tileX, tileY);
psTile->psObject = psBuilding;

Expand Down Expand Up @@ -1612,7 +1609,7 @@ STRUCTURE *buildStructureDir(STRUCTURE_STATS *pStructureType, UDWORD x, UDWORD y
if (!setFunctionality(psBuilding, pStructureType->type))
{
removeStructFromMap(psBuilding);
delete psBuilding;
objmemDestroy(psBuilding, false);
//better reset these if you couldn't build the structure!
if (FromSave && player == selectedPlayer && missionLimboExpand())
{
Expand Down Expand Up @@ -1834,8 +1831,6 @@ STRUCTURE *buildStructureDir(STRUCTURE_STATS *pStructureType, UDWORD x, UDWORD y

STRUCTURE *buildBlueprint(STRUCTURE_STATS const *psStats, Vector3i pos, uint16_t direction, unsigned moduleIndex, STRUCT_STATES state, uint8_t ownerPlayer)
{
STRUCTURE *blueprint = nullptr;

ASSERT_OR_RETURN(nullptr, psStats != nullptr, "No blueprint stats");
ASSERT_OR_RETURN(nullptr, psStats->pIMD[0] != nullptr, "No blueprint model for %s", getStatsName(psStats));
ASSERT_OR_RETURN(nullptr, ownerPlayer < MAX_PLAYERS, "invalid ownerPlayer: %" PRIu8 "", ownerPlayer);
Expand Down Expand Up @@ -1866,55 +1861,57 @@ STRUCTURE *buildBlueprint(STRUCTURE_STATS const *psStats, Vector3i pos, uint16_t
}
}

blueprint = new STRUCTURE(0, ownerPlayer);
STRUCTURE blueprint(0, ownerPlayer);
// construct the fake structure
blueprint->pStructureType = const_cast<STRUCTURE_STATS *>(psStats); // Couldn't be bothered to fix const correctness everywhere.
blueprint.pStructureType = const_cast<STRUCTURE_STATS *>(psStats); // Couldn't be bothered to fix const correctness everywhere.
if (selectedPlayer < MAX_PLAYERS)
{
blueprint->visible[selectedPlayer] = UBYTE_MAX;
blueprint.visible[selectedPlayer] = UBYTE_MAX;
}
blueprint->sDisplay.imd = (*pIMD)[std::min<int>(moduleNumber, pIMD->size() - 1)];
blueprint->pos = pos;
blueprint->rot = rot;
blueprint->selected = false;
blueprint.sDisplay.imd = (*pIMD)[std::min<int>(moduleNumber, pIMD->size() - 1)];
blueprint.pos = pos;
blueprint.rot = rot;
blueprint.selected = false;

blueprint->numWeaps = 0;
blueprint->asWeaps[0].nStat = 0;
blueprint.numWeaps = 0;
blueprint.asWeaps[0].nStat = 0;

// give defensive structures a weapon
if (psStats->psWeapStat[0])
{
blueprint->asWeaps[0].nStat = psStats->psWeapStat[0] - asWeaponStats.data();
blueprint.asWeaps[0].nStat = psStats->psWeapStat[0] - asWeaponStats.data();
}
// things with sensors or ecm (or repair facilities) need these set, even if they have no official weapon
blueprint->numWeaps = 0;
blueprint->asWeaps[0].lastFired = 0;
blueprint->asWeaps[0].rot.pitch = 0;
blueprint->asWeaps[0].rot.direction = 0;
blueprint->asWeaps[0].rot.roll = 0;
blueprint->asWeaps[0].prevRot = blueprint->asWeaps[0].rot;
blueprint.numWeaps = 0;
blueprint.asWeaps[0].lastFired = 0;
blueprint.asWeaps[0].rot.pitch = 0;
blueprint.asWeaps[0].rot.direction = 0;
blueprint.asWeaps[0].rot.roll = 0;
blueprint.asWeaps[0].prevRot = blueprint.asWeaps[0].rot;

blueprint->expectedDamage = 0;
blueprint.expectedDamage = 0;

// Times must be different, but don't otherwise matter.
blueprint->time = 23;
blueprint->prevTime = 42;
blueprint.time = 23;
blueprint.prevTime = 42;

blueprint->status = state;
blueprint.status = state;

// Rotate wall if needed.
if (blueprint->pStructureType->type == REF_WALL || blueprint->pStructureType->type == REF_GATE)
if (blueprint.pStructureType->type == REF_WALL || blueprint.pStructureType->type == REF_GATE)
{
WallOrientation scanType = structChooseWallTypeBlueprint(map_coord(blueprint->pos.xy()));
WallOrientation scanType = structChooseWallTypeBlueprint(map_coord(blueprint.pos.xy()));
unsigned type = wallType(scanType);
if (scanType != WallConnectNone)
{
blueprint->rot.direction = wallDir(scanType);
blueprint->sDisplay.imd = blueprint->pStructureType->pIMD[std::min<unsigned>(type, blueprint->pStructureType->pIMD.size() - 1)];
blueprint.rot.direction = wallDir(scanType);
blueprint.sDisplay.imd = blueprint.pStructureType->pIMD[std::min<unsigned>(type, blueprint.pStructureType->pIMD.size() - 1)];
}
}

return blueprint;
auto& stableBlueprint = GlobalStructContainer().emplace(std::move(blueprint));

return &stableBlueprint;
}

static Vector2i defaultAssemblyPointPos(STRUCTURE *psBuilding)
Expand Down Expand Up @@ -7216,3 +7213,9 @@ LineBuild calcLineBuild(STRUCTURE_STATS const *stats, uint16_t direction, Vector
{
return calcLineBuild(stats->size(direction), stats->type, pos, pos2);
}

StructContainer& GlobalStructContainer()
{
static StructContainer instance;
return instance;
}
6 changes: 6 additions & 0 deletions src/structure.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -496,4 +496,10 @@ struct LineBuild
LineBuild calcLineBuild(Vector2i size, STRUCTURE_TYPE type, Vector2i pos, Vector2i pos2);
LineBuild calcLineBuild(STRUCTURE_STATS const *stats, uint16_t direction, Vector2i pos, Vector2i pos2);

// Split the struct storage into pages containing 256 structs, 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 StructContainer = PagedEntityContainer<STRUCTURE, 256, false>;
StructContainer& GlobalStructContainer();

#endif // __INCLUDED_SRC_STRUCTURE_H__

0 comments on commit ce622e1

Please sign in to comment.