perf(Core/Unit): Replace red-black tree with flat_multimap for aura containers (#25885)

Co-authored-by: blinkysc <blinkysc@users.noreply.github.com>
Co-authored-by: Takenbacon <Takenbacon@users.noreply.github.com>
This commit is contained in:
blinkysc
2026-07-14 17:54:41 -04:00
committed by GitHub
parent 0a3e199df0
commit de866ed4f8
3 changed files with 634 additions and 19 deletions

View File

@@ -384,8 +384,6 @@ Unit::Unit() : WorldObject(),
for (uint8 i = 0; i < MAX_GAMEOBJECT_SLOT; ++i)
m_ObjectSlot[i].Clear();
m_auraUpdateIterator = m_ownedAuras.end();
m_interruptMask = 0;
m_transform = 0;
m_canModifyStats = false;
@@ -4034,13 +4032,16 @@ void Unit::_UpdateSpells(uint32 time)
}
}
// m_auraUpdateIterator can be updated in indirect called code at aura remove to skip next planned to update but removed auras
for (m_auraUpdateIterator = m_ownedAuras.begin(); m_auraUpdateIterator != m_ownedAuras.end();)
{
Aura* i_aura = m_auraUpdateIterator->second;
++m_auraUpdateIterator; // need shift to next for allow update if need into aura update
i_aura->UpdateOwner(time, this);
}
// snapshot - UpdateOwner can mutate the map; pointers stay valid (removed auras
// are deleted later in _DeleteRemovedAuras)
m_auraUpdateSnapshot.clear();
m_auraUpdateSnapshot.reserve(m_ownedAuras.size());
for (auto& [id, aura] : m_ownedAuras)
m_auraUpdateSnapshot.push_back(aura);
for (Aura* aura : m_auraUpdateSnapshot)
if (!aura->IsRemoved())
aura->UpdateOwner(time, this);
// remove expired auras - do that after updates(used in scripts?)
for (AuraMap::iterator i = m_ownedAuras.begin(); i != m_ownedAuras.end();)
@@ -4986,10 +4987,6 @@ void Unit::RemoveOwnedAura(AuraMap::iterator& i, AuraRemoveMode removeMode)
Aura* aura = i->second;
ASSERT(!aura->IsRemoved());
// if unit currently update aura list then make safe update iterator shift to next
if (m_auraUpdateIterator == i && m_auraUpdateIterator != m_ownedAuras.end())
++m_auraUpdateIterator;
m_ownedAuras.erase(i);
m_removedAuras.push_back(aura);
@@ -5063,6 +5060,9 @@ void Unit::RemoveAura(AuraApplicationMap::iterator& i, AuraRemoveMode mode)
// Remove aura - for Area and Target auras
if (aura->GetOwner() == this)
aura->Remove(mode);
// Aura::Remove can cascade into m_appliedAuras mutations - reset again for the caller
i = m_appliedAuras.begin();
}
void Unit::RemoveAura(uint32 spellId, ObjectGuid caster, uint8 reqEffMask, AuraRemoveMode removeMode)
@@ -5150,7 +5150,8 @@ void Unit::RemoveAppliedAuras(std::function<bool(AuraApplication const*)> const&
{
for (AuraApplicationMap::iterator iter = m_appliedAuras.begin(); iter != m_appliedAuras.end();)
{
if (check(iter->second))
// RemoveAura no-ops on applications already mid-removal
if (!iter->second->GetRemoveMode() && check(iter->second))
{
RemoveAura(iter);
continue;
@@ -5176,9 +5177,11 @@ void Unit::RemoveAppliedAuras(uint32 spellId, std::function<bool(AuraApplication
{
for (AuraApplicationMap::iterator iter = m_appliedAuras.lower_bound(spellId); iter != m_appliedAuras.upper_bound(spellId);)
{
if (check(iter->second))
// RemoveAura no-ops on applications already mid-removal
if (!iter->second->GetRemoveMode() && check(iter->second))
{
RemoveAura(iter);
iter = m_appliedAuras.lower_bound(spellId);
continue;
}
++iter;

View File

@@ -31,6 +31,7 @@
#include "ThreatManager.h"
#include "UnitDefines.h"
#include "UnitUtils.h"
#include <boost/container/flat_map.hpp>
#include <functional>
#include <utility>
@@ -666,15 +667,15 @@ public:
typedef std::unordered_set<Unit*> AttackerSet;
typedef std::set<Unit*> ControlSet;
typedef std::multimap<uint32, Aura*> AuraMap;
typedef boost::container::flat_multimap<uint32, Aura*> AuraMap;
typedef std::pair<AuraMap::const_iterator, AuraMap::const_iterator> AuraMapBounds;
typedef std::pair<AuraMap::iterator, AuraMap::iterator> AuraMapBoundsNonConst;
typedef std::multimap<uint32, AuraApplication*> AuraApplicationMap;
typedef boost::container::flat_multimap<uint32, AuraApplication*> AuraApplicationMap;
typedef std::pair<AuraApplicationMap::const_iterator, AuraApplicationMap::const_iterator> AuraApplicationMapBounds;
typedef std::pair<AuraApplicationMap::iterator, AuraApplicationMap::iterator> AuraApplicationMapBoundsNonConst;
typedef std::multimap<AuraStateType, AuraApplication*> AuraStateAurasMap;
typedef boost::container::flat_multimap<AuraStateType, AuraApplication*> AuraStateAurasMap;
typedef std::pair<AuraStateAurasMap::const_iterator, AuraStateAurasMap::const_iterator> AuraStateAurasMapBounds;
typedef std::vector<AuraEffect*> AuraEffectList;
@@ -2164,7 +2165,7 @@ protected:
AuraMap m_ownedAuras;
AuraApplicationMap m_appliedAuras;
AuraList m_removedAuras;
AuraMap::iterator m_auraUpdateIterator;
std::vector<Aura*> m_auraUpdateSnapshot; // _UpdateSpells scratch buffer
uint32 m_removedAurasCount;
AuraEffectList m_modAuras[TOTAL_AURAS];

View File

@@ -0,0 +1,611 @@
/*
* This file is part of the AzerothCore Project. See AUTHORS file for Copyright information
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
* the Free Software Foundation; either version 2 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful, but WITHOUT
* ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
* FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
* more details.
*
* You should have received a copy of the GNU General Public License along
* with this program. If not, see <http://www.gnu.org/licenses/>.
*/
#include "gtest/gtest.h"
#include <boost/container/flat_map.hpp>
#include <cstdint>
#include <set>
#include <vector>
// Lightweight fake aura — just enough state for pattern testing.
struct FakeAura
{
uint32_t spellId;
bool removed = false;
bool expired = false;
bool updated = false;
explicit FakeAura(uint32_t id) : spellId(id) {}
bool IsRemoved() const { return removed; }
bool IsExpired() const { return expired; }
};
using AuraMap = boost::container::flat_multimap<uint32_t, FakeAura*>;
// -----------------------------------------------------------------------
// 1. Snapshot-update pattern (mirrors _UpdateSpells snapshot loop)
//
// Snapshot pointers to a vector, then iterate the vector.
// Entries marked removed during iteration are skipped.
// -----------------------------------------------------------------------
TEST(FlatMultimapAuraPattern, SnapshotUpdate)
{
FakeAura a1(100), a2(100), a3(200), a4(300);
AuraMap map;
map.emplace(100, &a1);
map.emplace(100, &a2);
map.emplace(200, &a3);
map.emplace(300, &a4);
// Snapshot pointers (mirrors Unit::_UpdateSpells)
std::vector<FakeAura*> snapshot;
snapshot.reserve(map.size());
for (auto& [id, aura] : map)
snapshot.push_back(aura);
ASSERT_EQ(snapshot.size(), 4u);
// Mark a2 as removed mid-iteration (simulates cascading removal)
a2.removed = true;
// Process snapshot — skip removed entries
for (FakeAura* aura : snapshot)
{
if (!aura->IsRemoved())
aura->updated = true;
}
EXPECT_TRUE(a1.updated);
EXPECT_FALSE(a2.updated); // skipped — was removed
EXPECT_TRUE(a3.updated);
EXPECT_TRUE(a4.updated);
}
// -----------------------------------------------------------------------
// 2. Erase-and-reset-to-begin (mirrors RemoveOwnedAura / expire loop)
//
// for (auto i = map.begin(); i != map.end();)
// if (should_remove) { erase(i); i = map.begin(); }
// else ++i;
// -----------------------------------------------------------------------
TEST(FlatMultimapAuraPattern, EraseResetToBegin)
{
FakeAura a1(100), a2(200), a3(300), a4(400), a5(500);
a2.expired = true;
a4.expired = true;
AuraMap map;
map.emplace(100, &a1);
map.emplace(200, &a2);
map.emplace(300, &a3);
map.emplace(400, &a4);
map.emplace(500, &a5);
std::vector<FakeAura*> removed;
// Mirrors the expire loop in _UpdateSpells / RemoveOwnedAuras
for (AuraMap::iterator i = map.begin(); i != map.end();)
{
if (i->second->IsExpired())
{
FakeAura* aura = i->second;
map.erase(i);
i = map.begin(); // reset — flat_multimap invalidates all
removed.push_back(aura);
}
else
++i;
}
// a2 and a4 should have been removed
ASSERT_EQ(removed.size(), 2u);
EXPECT_EQ(removed[0]->spellId, 200u);
EXPECT_EQ(removed[1]->spellId, 400u);
// Remaining entries
ASSERT_EQ(map.size(), 3u);
std::set<uint32_t> remaining;
for (auto& [id, aura] : map)
remaining.insert(id);
EXPECT_TRUE(remaining.count(100));
EXPECT_TRUE(remaining.count(300));
EXPECT_TRUE(remaining.count(500));
}
// -----------------------------------------------------------------------
// 3. lower_bound / upper_bound erase (mirrors RemoveOwnedAura by spellId
// and RemoveAurasDueToSpell)
//
// Iterate a key range, erase matching entries, reset iterator to
// lower_bound after each erase.
// -----------------------------------------------------------------------
TEST(FlatMultimapAuraPattern, LowerUpperBoundErase)
{
FakeAura a1(100), a2(100), a3(100), a4(200), a5(200);
AuraMap map;
map.emplace(100, &a1);
map.emplace(100, &a2);
map.emplace(100, &a3);
map.emplace(200, &a4);
map.emplace(200, &a5);
// Remove all entries with key 100 — mirrors RemoveOwnedAura(spellId)
uint32_t targetKey = 100;
for (auto itr = map.lower_bound(targetKey);
itr != map.upper_bound(targetKey);)
{
map.erase(itr);
itr = map.lower_bound(targetKey); // reset after erase
}
// All key-100 entries gone
EXPECT_EQ(map.count(100), 0u);
// Key-200 entries untouched
EXPECT_EQ(map.count(200), 2u);
ASSERT_EQ(map.size(), 2u);
}
// -----------------------------------------------------------------------
// 3b. Selective erase within a key range
// (mirrors RemoveOwnedAura with casterGUID / effectMask filter)
// -----------------------------------------------------------------------
TEST(FlatMultimapAuraPattern, SelectiveLowerBoundErase)
{
FakeAura a1(100), a2(100), a3(100);
// Only remove entries whose spellId matches AND which are expired
a1.expired = false;
a2.expired = true;
a3.expired = false;
AuraMap map;
map.emplace(100, &a1);
map.emplace(100, &a2);
map.emplace(100, &a3);
uint32_t targetKey = 100;
for (auto itr = map.lower_bound(targetKey);
itr != map.upper_bound(targetKey);)
{
if (itr->second->IsExpired())
{
map.erase(itr);
itr = map.lower_bound(targetKey);
}
else
++itr;
}
// Only a2 removed
EXPECT_EQ(map.count(100), 2u);
// Verify the right ones survived
std::set<FakeAura*> survivors;
for (auto itr = map.lower_bound(100); itr != map.upper_bound(100); ++itr)
survivors.insert(itr->second);
EXPECT_TRUE(survivors.count(&a1));
EXPECT_FALSE(survivors.count(&a2));
EXPECT_TRUE(survivors.count(&a3));
}
// -----------------------------------------------------------------------
// 4. Erase during snapshot (mirrors cascading removal — snapshot loop
// processes pointers while the map is mutated underneath)
// -----------------------------------------------------------------------
TEST(FlatMultimapAuraPattern, EraseDuringSnapshot)
{
FakeAura a1(100), a2(200), a3(300), a4(400);
AuraMap map;
map.emplace(100, &a1);
map.emplace(200, &a2);
map.emplace(300, &a3);
map.emplace(400, &a4);
// Snapshot pointers
std::vector<FakeAura*> snapshot;
snapshot.reserve(map.size());
for (auto& [id, aura] : map)
snapshot.push_back(aura);
// Process snapshot; during processing, erase entries from the map
std::vector<uint32_t> processed;
for (FakeAura* aura : snapshot)
{
if (aura->IsRemoved())
continue;
processed.push_back(aura->spellId);
// Simulate cascading removal: processing a1 causes a3 to be
// removed from the map and marked removed
if (aura == &a1)
{
a3.removed = true;
// Erase a3 from map by finding it
for (auto it = map.begin(); it != map.end(); ++it)
{
if (it->second == &a3)
{
map.erase(it);
break;
}
}
}
}
// a1, a2, a4 processed; a3 skipped due to IsRemoved()
ASSERT_EQ(processed.size(), 3u);
EXPECT_EQ(processed[0], 100u);
EXPECT_EQ(processed[1], 200u);
EXPECT_EQ(processed[2], 400u);
// Map should have 3 entries (a3 was erased)
EXPECT_EQ(map.size(), 3u);
}
// -----------------------------------------------------------------------
// 5. Insert during snapshot iteration
// Snapshot loop must NOT see newly inserted entries.
// -----------------------------------------------------------------------
TEST(FlatMultimapAuraPattern, InsertDuringSnapshot)
{
FakeAura a1(100), a2(200);
FakeAura a3(300); // will be inserted during snapshot iteration
AuraMap map;
map.emplace(100, &a1);
map.emplace(200, &a2);
// Snapshot before iteration
std::vector<FakeAura*> snapshot;
snapshot.reserve(map.size());
for (auto& [id, aura] : map)
snapshot.push_back(aura);
ASSERT_EQ(snapshot.size(), 2u);
// During iteration, insert a new entry
std::vector<uint32_t> processed;
for (FakeAura* aura : snapshot)
{
if (!aura->IsRemoved())
{
processed.push_back(aura->spellId);
aura->updated = true;
}
// Insert a3 while iterating the snapshot
if (aura == &a1)
map.emplace(300, &a3);
}
// Only the original 2 were processed
ASSERT_EQ(processed.size(), 2u);
EXPECT_EQ(processed[0], 100u);
EXPECT_EQ(processed[1], 200u);
// a3 was NOT processed (not in snapshot)
EXPECT_FALSE(a3.updated);
// But it IS in the map for future iterations
EXPECT_EQ(map.size(), 3u);
EXPECT_EQ(map.count(300), 1u);
}
// -----------------------------------------------------------------------
// 6. Predicate-based removal with reset-to-begin
// (mirrors RemoveOwnedAuras(std::function<bool(Aura const*)>))
// -----------------------------------------------------------------------
TEST(FlatMultimapAuraPattern, PredicateRemovalResetBegin)
{
FakeAura a1(100), a2(200), a3(300), a4(400), a5(500);
// Remove even-numbered spell IDs
AuraMap map;
map.emplace(100, &a1);
map.emplace(200, &a2);
map.emplace(300, &a3);
map.emplace(400, &a4);
map.emplace(500, &a5);
auto shouldRemove = [](FakeAura const* a) {
return (a->spellId % 200) == 0;
};
for (AuraMap::iterator iter = map.begin(); iter != map.end();)
{
if (shouldRemove(iter->second))
{
map.erase(iter);
iter = map.begin(); // reset — mirrors RemoveOwnedAuras
continue;
}
++iter;
}
ASSERT_EQ(map.size(), 3u);
EXPECT_EQ(map.count(100), 1u);
EXPECT_EQ(map.count(200), 0u);
EXPECT_EQ(map.count(300), 1u);
EXPECT_EQ(map.count(400), 0u);
EXPECT_EQ(map.count(500), 1u);
}
// -----------------------------------------------------------------------
// 7. Predicate removal within a key range with reset-to-lower_bound
// (mirrors RemoveOwnedAuras(spellId, check))
// -----------------------------------------------------------------------
TEST(FlatMultimapAuraPattern, PredicateRemovalInKeyRange)
{
FakeAura a1(100), a2(100), a3(100), a4(100);
a1.expired = false;
a2.expired = true;
a3.expired = false;
a4.expired = true;
AuraMap map;
map.emplace(100, &a1);
map.emplace(100, &a2);
map.emplace(100, &a3);
map.emplace(100, &a4);
map.emplace(200, new FakeAura(200)); // different key, untouched
uint32_t spellId = 100;
for (auto iter = map.lower_bound(spellId);
iter != map.upper_bound(spellId);)
{
if (iter->second->IsExpired())
{
map.erase(iter);
iter = map.lower_bound(spellId);
continue;
}
++iter;
}
// 2 expired removed, 2 non-expired remain under key 100
EXPECT_EQ(map.count(100), 2u);
// Key 200 untouched
EXPECT_EQ(map.count(200), 1u);
// Clean up heap-allocated entry
for (auto& [id, aura] : map)
if (id == 200)
delete aura;
}
// -----------------------------------------------------------------------
// 8. AuraStateAuras erase-and-reset-to-lower_bound
// (mirrors _UnapplyAura's m_auraStateAuras cleanup)
// -----------------------------------------------------------------------
TEST(FlatMultimapAuraPattern, AuraStateMapErasePattern)
{
// Uses an enum-like key (AuraStateType is an enum in the real code)
using AuraStateMap = boost::container::flat_multimap<uint32_t, FakeAura*>;
FakeAura a1(10), a2(20), a3(30), a4(40);
AuraStateMap map;
// State 1 has multiple entries
map.emplace(1, &a1);
map.emplace(1, &a2);
map.emplace(1, &a3);
// State 2 has one entry
map.emplace(2, &a4);
// Remove a2 from state 1 — mirrors _UnapplyAura pattern
uint32_t auraState = 1;
FakeAura* target = &a2;
for (auto itr = map.lower_bound(auraState);
itr != map.upper_bound(auraState);)
{
if (itr->second == target)
{
map.erase(itr);
itr = map.lower_bound(auraState);
continue;
}
++itr;
}
EXPECT_EQ(map.count(1), 2u);
EXPECT_EQ(map.count(2), 1u);
// Verify a2 is gone, a1 and a3 remain
std::set<FakeAura*> stateOnes;
for (auto itr = map.lower_bound(1); itr != map.upper_bound(1); ++itr)
stateOnes.insert(itr->second);
EXPECT_TRUE(stateOnes.count(&a1));
EXPECT_FALSE(stateOnes.count(&a2));
EXPECT_TRUE(stateOnes.count(&a3));
}
// -----------------------------------------------------------------------
// 9. Empty map edge cases — all patterns should be no-ops on empty maps
// -----------------------------------------------------------------------
TEST(FlatMultimapAuraPattern, EmptyMapPatterns)
{
AuraMap map;
// Snapshot of empty map
std::vector<FakeAura*> snapshot;
for (auto& [id, aura] : map)
snapshot.push_back(aura);
EXPECT_TRUE(snapshot.empty());
// Erase-reset-to-begin on empty map
for (auto i = map.begin(); i != map.end();)
{
map.erase(i);
i = map.begin();
}
EXPECT_TRUE(map.empty());
// lower_bound/upper_bound on empty map
for (auto itr = map.lower_bound(100); itr != map.upper_bound(100);)
{
map.erase(itr);
itr = map.lower_bound(100);
}
EXPECT_TRUE(map.empty());
}
// -----------------------------------------------------------------------
// 10. Single-element map — boundary case for erase patterns
// -----------------------------------------------------------------------
TEST(FlatMultimapAuraPattern, SingleElementPatterns)
{
FakeAura a1(100);
a1.expired = true;
AuraMap map;
map.emplace(100, &a1);
// Erase-reset-to-begin with single element
for (auto i = map.begin(); i != map.end();)
{
if (i->second->IsExpired())
{
map.erase(i);
i = map.begin();
}
else
++i;
}
EXPECT_TRUE(map.empty());
}
// -----------------------------------------------------------------------
// 11. Erase all via reset-to-begin (stress test)
// Every entry matches removal criteria.
// -----------------------------------------------------------------------
TEST(FlatMultimapAuraPattern, EraseAllViaResetBegin)
{
constexpr int N = 50;
std::vector<FakeAura> auras;
auras.reserve(N);
AuraMap map;
for (int i = 0; i < N; ++i)
{
auras.emplace_back(static_cast<uint32_t>(i * 10));
map.emplace(auras.back().spellId, &auras.back());
}
ASSERT_EQ(map.size(), static_cast<size_t>(N));
// Remove everything — every entry triggers erase + reset
for (auto i = map.begin(); i != map.end();)
{
map.erase(i);
i = map.begin();
}
EXPECT_TRUE(map.empty());
}
// -----------------------------------------------------------------------
// 12. Duplicate keys with interleaved erase (multiple auras per spellId)
// -----------------------------------------------------------------------
TEST(FlatMultimapAuraPattern, DuplicateKeysInterleavedErase)
{
FakeAura a1(100), a2(100), a3(100), a4(200), a5(200);
a2.expired = true;
a5.expired = true;
AuraMap map;
map.emplace(100, &a1);
map.emplace(100, &a2);
map.emplace(100, &a3);
map.emplace(200, &a4);
map.emplace(200, &a5);
// Global erase-reset-to-begin for expired entries
for (auto i = map.begin(); i != map.end();)
{
if (i->second->IsExpired())
{
map.erase(i);
i = map.begin();
}
else
++i;
}
EXPECT_EQ(map.size(), 3u);
EXPECT_EQ(map.count(100), 2u);
EXPECT_EQ(map.count(200), 1u);
}
// -----------------------------------------------------------------------
// 13. Cascade-after-reset (Unit::RemoveAura contract): Aura::Remove can
// mutate the map after _UnapplyAura reset the iterator. The DISABLED_
// test is deliberate UB - run it only under ASAN, where it reports a
// deterministic heap-use-after-free:
// ./unit_tests --gtest_also_run_disabled_tests --gtest_filter='*StaleIteratorAfterCascade*'
// -----------------------------------------------------------------------
TEST(FlatMultimapAuraPattern, DISABLED_StaleIteratorAfterCascade_AsanOnly)
{
FakeAura a1(100), a2(200), a3(300), cascade1(999), cascade2(998);
AuraMap map;
map.emplace(100, &a1);
map.emplace(200, &a2);
map.emplace(300, &a3);
map.shrink_to_fit();
// caller loop holds an iterator
AuraMap::iterator iter = map.begin();
// RemoveAura: erase + reset, then cascades grow past capacity -> realloc
map.erase(iter);
iter = map.begin();
map.emplace(999, &cascade1);
map.emplace(998, &cascade2);
// resume with the pre-cascade iterator: heap-use-after-free under ASAN
EXPECT_NE(iter->second->spellId, 0u);
}
// The fixed contract: reset after all cascade mutations (mirrors Unit::RemoveAura).
TEST(FlatMultimapAuraPattern, IteratorResetAfterCascade)
{
FakeAura a1(100), a2(200), a3(300), cascade1(999), cascade2(998);
AuraMap map;
map.emplace(100, &a1);
map.emplace(200, &a2);
map.emplace(300, &a3);
map.shrink_to_fit();
AuraMap::iterator iter = map.begin();
map.erase(iter);
map.emplace(999, &cascade1);
map.emplace(998, &cascade2);
iter = map.begin(); // reset AFTER the cascades
std::set<uint32_t> seen;
for (; iter != map.end(); ++iter)
seen.insert(iter->second->spellId);
EXPECT_EQ(seen.size(), 4u);
EXPECT_FALSE(seen.count(100)); // removed
EXPECT_TRUE(seen.count(200));
EXPECT_TRUE(seen.count(300));
EXPECT_TRUE(seen.count(999)); // cascade-applied
EXPECT_TRUE(seen.count(998));
}