perf: npc/monster storage with vector indexing (#3209)

This modifies the internal storage of NPCs and monsters in the Game class from std::unordered_map to a combination of std::vector and std::unordered_map indexes for improved lookup efficiency and better memory locality.

Motivation
Previously, every time a search was performed for an NPC or a monster, the unordered map was iterated, and even though it provides average O(1) complexity for lookups, the actual cost becomes significant when handling a large dataset (e.g., 80,000 monsters). This cost arises from the overhead of hashing and poor memory locality, as unordered maps store elements in a hash table.

Switching to a vector with index-based lookup improves the following:

1. Memory locality: Vectors store elements contiguously in memory, improving cache efficiency when accessing elements sequentially or repeatedly.
2. Lookup efficiency: Using an unordered map as an index to the vector allows leveraging direct index-based access (constant time) to the actual data, combining the best of both structures.

Author: dudantas

src/creatures/combat/condition.cpp

@@ -1769,6 +1769,10 @@ bool ConditionDamage::getNextDamage(int32_t &damage) {
 }
 
 bool ConditionDamage::doDamage(const std::shared_ptr<Creature> &creature, int32_t healthChange) const {
+	if (owner == 0) {
+		return false;
+	}
+
 	const auto &attacker = g_game().getPlayerByGUID(owner) ? g_game().getPlayerByGUID(owner)->getCreature() : g_game().getCreatureByID(owner);
 	bool isPlayer = attacker && attacker->getPlayer();
 	if (creature->isSuppress(getType(), isPlayer)) {

src/creatures/monsters/monster.cpp

@@ -35,6 +35,7 @@ std::shared_ptr<Monster> Monster::createMonster(const std::string &name) {
 }
 
 Monster::Monster(const std::shared_ptr<MonsterType> &mType) :
+	m_lowerName(asLowerCaseString(mType->name)),
 	nameDescription(asLowerCaseString(mType->nameDescription)),
 	mType(mType) {
 	defaultOutfit = mType->info.outfit;

src/creatures/monsters/monster.hpp

@@ -49,6 +49,10 @@ class Monster final : public Creature {
 	void setNameDescription(std::string_view nameDescription);
 	std::string getDescription(int32_t) override;
 
+	const std::string &getLowerName() const {
+		return m_lowerName;
+	}
+
 	CreatureType_t getType() const override;
 
 	const Position &getMasterPos() const;
@@ -244,6 +248,7 @@ class Monster final : public Creature {
 	ForgeClassifications_t monsterForgeClassification = ForgeClassifications_t::FORGE_NORMAL_MONSTER;
 
 	std::string name;
+	std::string m_lowerName;
 	std::string nameDescription;
 
 	std::shared_ptr<MonsterType> mType;

src/creatures/npcs/npc.cpp

@@ -97,6 +97,10 @@ void Npc::setName(std::string newName) const {
 	npcType->name = std::move(newName);
 }
 
+const std::string &Npc::getLowerName() const {
+	return npcType->m_lowerName;
+}
+
 CreatureType_t Npc::getType() const {
 	return CREATURETYPE_NPC;
 }

src/creatures/npcs/npc.hpp

@@ -51,6 +51,8 @@ class Npc final : public Creature {
 
 	void setName(std::string newName) const;
 
+	const std::string &getLowerName() const;
+
 	CreatureType_t getType() const override;
 
 	const Position &getMasterPos() const;

src/creatures/npcs/npcs.cpp

@@ -16,6 +16,9 @@
 #include "lua/scripts/scripts.hpp"
 #include "lib/di/container.hpp"
 
+NpcType::NpcType(const std::string &initName) :
+	name(initName), m_lowerName(asLowerCaseString(initName)), typeName(initName), nameDescription(initName) {};
+
 bool NpcType::canSpawn(const Position &pos) const {
 	bool canSpawn = true;
 	const bool isDay = g_game().gameIsDay();

src/creatures/npcs/npcs.hpp

@@ -77,14 +77,14 @@ class NpcType final : public SharedObject {
 
 public:
 	NpcType() = default;
-	explicit NpcType(const std::string &initName) :
-		name(initName), typeName(initName), nameDescription(initName) {};
+	explicit NpcType(const std::string &initName);
 
 	// non-copyable
 	NpcType(const NpcType &) = delete;
 	NpcType &operator=(const NpcType &) = delete;
 
 	std::string name;
+	std::string m_lowerName;
 	std::string typeName;
 	std::string nameDescription;
 	NpcInfo info;

src/game/game.cpp

@@ -424,15 +424,15 @@ Game &Game::getInstance() {
 }
 
 void Game::resetMonsters() const {
-	for (const auto &[monsterId, monster] : getMonsters()) {
+	for (const auto &monster : getMonsters()) {
 		monster->clearTargetList();
 		monster->clearFriendList();
 	}
 }
 
 void Game::resetNpcs() const {
 	// Close shop window from all npcs and reset the shopPlayerSet
-	for (const auto &[npcId, npc] : getNpcs()) {
+	for (const auto &npc : getNpcs()) {
 		npc->closeAllShopWindows();
 		npc->resetPlayerInteractions();
 	}
@@ -954,23 +954,29 @@ std::shared_ptr<Monster> Game::getMonsterByID(uint32_t id) {
 		return nullptr;
 	}
 
-	auto it = monsters.find(id);
-	if (it == monsters.end()) {
+	auto it = monstersIdIndex.find(id);
+	if (it == monstersIdIndex.end()) {
 		return nullptr;
 	}
-	return it->second;
+
+	if (it->second >= monsters.size()) {
+		return nullptr;
+	}
+
+	return monsters[it->second];
 }
 
 std::shared_ptr<Npc> Game::getNpcByID(uint32_t id) {
 	if (id == 0) {
 		return nullptr;
 	}
 
-	auto it = npcs.find(id);
-	if (it == npcs.end()) {
+	auto it = npcsIdIndex.find(id);
+	if (it == npcsIdIndex.end()) {
 		return nullptr;
 	}
-	return it->second;
+
+	return npcs[it->second];
 }
 
 std::shared_ptr<Player> Game::getPlayerByID(uint32_t id, bool allowOffline /* = false */) {
@@ -990,43 +996,41 @@ std::shared_ptr<Player> Game::getPlayerByID(uint32_t id, bool allowOffline /* =
 	return tmpPlayer;
 }
 
-std::shared_ptr<Creature> Game::getCreatureByName(const std::string &s) {
-	if (s.empty()) {
+std::shared_ptr<Creature> Game::getCreatureByName(const std::string &creatureName) {
+	if (creatureName.empty()) {
 		return nullptr;
 	}
 
-	const std::string &lowerCaseName = asLowerCaseString(s);
+	const std::string &lowerCaseName = asLowerCaseString(creatureName);
 
 	auto m_it = mappedPlayerNames.find(lowerCaseName);
 	if (m_it != mappedPlayerNames.end()) {
 		return m_it->second.lock();
 	}
 
-	for (const auto &it : npcs) {
-		if (lowerCaseName == asLowerCaseString(it.second->getName())) {
-			return it.second;
-		}
+	auto npcIterator = npcsNameIndex.find(lowerCaseName);
+	if (npcIterator != npcsNameIndex.end()) {
+		return npcs[npcIterator->second];
 	}
 
-	for (const auto &it : monsters) {
-		if (lowerCaseName == asLowerCaseString(it.second->getName())) {
-			return it.second;
-		}
+	auto monsterIterator = monstersNameIndex.find(lowerCaseName);
+	if (monsterIterator != monstersNameIndex.end()) {
+		return monsters[monsterIterator->second];
 	}
 	return nullptr;
 }
 
-std::shared_ptr<Npc> Game::getNpcByName(const std::string &s) {
-	if (s.empty()) {
+std::shared_ptr<Npc> Game::getNpcByName(const std::string &npcName) {
+	if (npcName.empty()) {
 		return nullptr;
 	}
 
-	const char* npcName = s.c_str();
-	for (const auto &it : npcs) {
-		if (strcasecmp(npcName, it.second->getName().c_str()) == 0) {
-			return it.second;
-		}
+	const std::string lowerCaseName = asLowerCaseString(npcName);
+	auto it = npcsNameIndex.find(lowerCaseName);
+	if (it != npcsNameIndex.end()) {
+		return npcs[it->second];
 	}
+
 	return nullptr;
 }
 
@@ -3184,13 +3188,18 @@ ReturnValue Game::internalCollectManagedItems(const std::shared_ptr<Player> &pla
 
 ReturnValue Game::collectRewardChestItems(const std::shared_ptr<Player> &player, uint32_t maxMoveItems /* = 0*/) {
 	// Check if have item on player reward chest
-	std::shared_ptr<RewardChest> rewardChest = player->getRewardChest();
+	const std::shared_ptr<RewardChest> &rewardChest = player->getRewardChest();
 	if (rewardChest->empty()) {
 		g_logger().debug("Reward chest is empty");
 		return RETURNVALUE_REWARDCHESTISEMPTY;
 	}
 
-	auto rewardItemsVector = player->getRewardsFromContainer(rewardChest->getContainer());
+	const auto &container = rewardChest->getContainer();
+	if (!container) {
+		return RETURNVALUE_REWARDCHESTISEMPTY;
+	}
+
+	auto rewardItemsVector = player->getRewardsFromContainer(container);
 	auto rewardCount = rewardItemsVector.size();
 	uint32_t movedRewardItems = 0;
 	std::string lootedItemsMessage;
@@ -9931,19 +9940,72 @@ void Game::removePlayer(const std::shared_ptr<Player> &player) {
 }
 
 void Game::addNpc(const std::shared_ptr<Npc> &npc) {
-	npcs[npc->getID()] = npc;
+	npcs.push_back(npc);
+	size_t index = npcs.size() - 1;
+	npcsNameIndex[npc->getLowerName()] = index;
+	npcsIdIndex[npc->getID()] = index;
 }
 
 void Game::removeNpc(const std::shared_ptr<Npc> &npc) {
-	npcs.erase(npc->getID());
+	if (!npc) {
+		return;
+	}
+
+	auto npcId = npc->getID();
+	const auto &npcLowerName = npc->getLowerName();
+	auto it = npcsIdIndex.find(npcId);
+	if (it != npcsIdIndex.end()) {
+		size_t index = it->second;
+		npcsNameIndex.erase(npcLowerName);
+		npcsIdIndex.erase(npcId);
+
+		if (index != npcs.size() - 1) {
+			std::swap(npcs[index], npcs.back());
+
+			const auto &movedNpc = npcs[index];
+			npcsNameIndex[movedNpc->getLowerName()] = index;
+			npcsIdIndex[movedNpc->getID()] = index;
+		}
+
+		npcs.pop_back();
+	}
 }
 
 void Game::addMonster(const std::shared_ptr<Monster> &monster) {
-	monsters[monster->getID()] = monster;
+	if (!monster) {
+		return;
+	}
+
+	const auto &lowerName = monster->getLowerName();
+	monsters.push_back(monster);
+	size_t index = monsters.size() - 1;
+	monstersNameIndex[lowerName] = index;
+	monstersIdIndex[monster->getID()] = index;
 }
 
 void Game::removeMonster(const std::shared_ptr<Monster> &monster) {
-	monsters.erase(monster->getID());
+	if (!monster) {
+		return;
+	}
+
+	auto monsterId = monster->getID();
+	const auto &monsterLowerName = monster->getLowerName();
+	auto it = monstersIdIndex.find(monsterId);
+	if (it != monstersIdIndex.end()) {
+		size_t index = it->second;
+		monstersNameIndex.erase(monsterLowerName);
+		monstersIdIndex.erase(monsterId);
+
+		if (index != monsters.size() - 1) {
+			std::swap(monsters[index], monsters.back());
+
+			const auto &movedMonster = monsters[index];
+			monstersNameIndex[movedMonster->getLowerName()] = index;
+			monstersIdIndex[movedMonster->getID()] = index;
+		}
+
+		monsters.pop_back();
+	}
 }
 
 std::shared_ptr<Guild> Game::getGuild(uint32_t id, bool allowOffline /* = flase */) const {
@@ -10152,7 +10214,7 @@ uint32_t Game::makeFiendishMonster(uint32_t forgeableMonsterId /* = 0*/, bool cr
 		forgeableMonsters.clear();
 		// If the forgeable monsters haven't been created
 		// Then we'll create them so they don't return in the next if (forgeableMonsters.empty())
-		for (const auto &[monsterId, monster] : monsters) {
+		for (const auto &monster : monsters) {
 			auto monsterTile = monster->getTile();
 			if (!monster || !monsterTile) {
 				continue;
@@ -10325,7 +10387,7 @@ void Game::updateForgeableMonsters() {
 	if (auto influencedLimit = g_configManager().getNumber(FORGE_INFLUENCED_CREATURES_LIMIT);
 	    forgeableMonsters.size() < influencedLimit) {
 		forgeableMonsters.clear();
-		for (const auto &[monsterId, monster] : monsters) {
+		for (const auto &monster : monsters) {
 			const auto &monsterTile = monster->getTile();
 			if (!monsterTile) {
 				continue;
@@ -10541,7 +10603,7 @@ void Game::playerRewardChestCollect(uint32_t playerId, const Position &pos, uint
 	}
 
 	// Updates the parent of the reward chest and reward containers to avoid memory usage after cleaning
-	auto playerRewardChest = player->getRewardChest();
+	const auto &playerRewardChest = player->getRewardChest();
 	if (playerRewardChest && playerRewardChest->empty()) {
 		player->sendCancelMessage(RETURNVALUE_REWARDCHESTISEMPTY);
 		return;

src/game/game.hpp

@@ -522,10 +522,10 @@ class Game {
 	const phmap::parallel_flat_hash_map<uint32_t, std::shared_ptr<Player>> &getPlayers() const {
 		return players;
 	}
-	const std::map<uint32_t, std::shared_ptr<Monster>> &getMonsters() const {
+	const auto &getMonsters() const {
 		return monsters;
 	}
-	const std::map<uint32_t, std::shared_ptr<Npc>> &getNpcs() const {
+	const auto &getNpcs() const {
 		return npcs;
 	}
 
@@ -539,8 +539,8 @@ class Game {
 	void addNpc(const std::shared_ptr<Npc> &npc);
 	void removeNpc(const std::shared_ptr<Npc> &npc);
 
-	void addMonster(const std::shared_ptr<Monster> &npc);
-	void removeMonster(const std::shared_ptr<Monster> &npc);
+	void addMonster(const std::shared_ptr<Monster> &monster);
+	void removeMonster(const std::shared_ptr<Monster> &monster);
 
 	std::shared_ptr<Guild> getGuild(uint32_t id, bool allowOffline = false) const;
 	std::shared_ptr<Guild> getGuildByName(const std::string &name, bool allowOffline = false) const;
@@ -851,8 +851,16 @@ class Game {
 
 	std::shared_ptr<WildcardTreeNode> wildcardTree = nullptr;
 
-	std::map<uint32_t, std::shared_ptr<Npc>> npcs;
-	std::map<uint32_t, std::shared_ptr<Monster>> monsters;
+	std::vector<std::shared_ptr<Monster>> monsters;
+	// This works only for unique monsters (bosses, quest monsters, etc)
+	std::unordered_map<std::string, size_t> monstersNameIndex;
+	std::unordered_map<uint32_t, size_t> monstersIdIndex;
+
+	std::vector<std::shared_ptr<Npc>> npcs;
+	// This works only for unique npcs (quest npcs, etc)
+	std::unordered_map<std::string, size_t> npcsNameIndex;
+	std::unordered_map<uint32_t, size_t> npcsIdIndex;
+
 	std::vector<uint32_t> forgeableMonsters;
 
 	std::map<uint32_t, std::unique_ptr<TeamFinder>> teamFinderMap; // [leaderGUID] = TeamFinder*

src/lua/functions/creatures/monster/monster_type_functions.cpp

@@ -1170,18 +1170,22 @@ int MonsterTypeFunctions::luaMonsterTypeGetCreatureEvents(lua_State* L) {
 int MonsterTypeFunctions::luaMonsterTypeRegisterEvent(lua_State* L) {
 	// monsterType:registerEvent(name)
 	const auto &monsterType = Lua::getUserdataShared<MonsterType>(L, 1);
-	if (monsterType) {
-		const auto eventName = Lua::getString(L, 2);
-		monsterType->info.scripts.insert(eventName);
-		for (const auto &[_, monster] : g_game().getMonsters()) {
-			if (monster->getMonsterType() == monsterType) {
-				monster->registerCreatureEvent(eventName);
-			}
-		}
-		Lua::pushBoolean(L, true);
-	} else {
+	if (!monsterType) {
 		lua_pushnil(L);
+		return 1;
 	}
+
+	const auto eventName = Lua::getString(L, 2);
+	monsterType->info.scripts.insert(eventName);
+
+	for (const auto &monster : g_game().getMonsters()) {
+		const auto monsterTypeCompare = monster->getMonsterType();
+		if (monsterTypeCompare == monsterType) {
+			monster->registerCreatureEvent(eventName);
+		}
+	}
+
+	Lua::pushBoolean(L, true);
 	return 1;
 }