diff --git a/defects/megaglest-0001/patch/megaglest-0001.patch b/defects/megaglest-0001/patch/megaglest-0001.patch new file mode 100644 index 000000000..cfd5967c4 --- /dev/null +++ b/defects/megaglest-0001/patch/megaglest-0001.patch @@ -0,0 +1,70 @@ +--- a/source/glest_game/type_instances/unit.cpp ++++ b/source/glest_game/type_instances/unit.cpp +@@ -2,6 +2,7 @@ + // unit.cpp + // + ++#include + #include "unit.h" + + #include +@@ -2576,7 +2577,12 @@ + UnitUpdater *unitUpdater = this->game->getWorld()->getUnitUpdater(); + + const AttackBoost *attackBoost = currSkill->getAttackBoost(); + vector candidates = unitUpdater->findUnitsInRange(this, attackBoost->radius); +- vector candidateValidIdList; +- candidateValidIdList.reserve(candidates.size()); ++ std::unordered_set candidateValidIdSet; ++ candidateValidIdSet.reserve(candidates.size()); ++ ++ // Build hash set of currently boosted unit IDs for O(1) membership test ++ std::unordered_set boostedIdSet( ++ currentAttackBoostOriginatorEffect.currentAttackBoostUnits.begin(), ++ currentAttackBoostOriginatorEffect.currentAttackBoostUnits.end()); + + if (debugBoost) + printf("Line: %d candidates unit size: " MG_SIZE_T_SPECIFIER " attackBoost: %s\n", __LINE__, candidates.size(), +@@ -2585,9 +2591,9 @@ + for (unsigned int i = 0; i < candidates.size(); ++i) { + Unit *affectedUnit = candidates[i]; +- candidateValidIdList.push_back(affectedUnit->getId()); ++ candidateValidIdSet.insert(affectedUnit->getId()); + +- std::vector::iterator iterFound = std::find(currentAttackBoostOriginatorEffect.currentAttackBoostUnits.begin(), +- currentAttackBoostOriginatorEffect.currentAttackBoostUnits.end(), affectedUnit->getId()); ++ bool alreadyBoosted = boostedIdSet.count(affectedUnit->getId()) > 0; + + if (attackBoost->isAffected(this, affectedUnit) == true) { +- if (iterFound == currentAttackBoostOriginatorEffect.currentAttackBoostUnits.end()) { ++ if (!alreadyBoosted) { + if (affectedUnit->applyAttackBoost(attackBoost, this) == true) { + currentAttackBoostOriginatorEffect.currentAttackBoostUnits.push_back(affectedUnit->getId()); ++ boostedIdSet.insert(affectedUnit->getId()); + } + } + } else { +- if (iterFound != currentAttackBoostOriginatorEffect.currentAttackBoostUnits.end()) { ++ if (alreadyBoosted) { + affectedUnit->deapplyAttackBoost(currentAttackBoostOriginatorEffect.skillType->getAttackBoost(), this); +- currentAttackBoostOriginatorEffect.currentAttackBoostUnits.erase(iterFound); ++ auto eraseIt = std::find(currentAttackBoostOriginatorEffect.currentAttackBoostUnits.begin(), ++ currentAttackBoostOriginatorEffect.currentAttackBoostUnits.end(), affectedUnit->getId()); ++ if (eraseIt != currentAttackBoostOriginatorEffect.currentAttackBoostUnits.end()) { ++ currentAttackBoostOriginatorEffect.currentAttackBoostUnits.erase(eraseIt); ++ } ++ boostedIdSet.erase(affectedUnit->getId()); + } + } + } +@@ -2616,7 +2622,7 @@ + if (currentAttackBoostOriginatorEffect.currentAttackBoostUnits.empty() == false) { + for (int i = (int)currentAttackBoostOriginatorEffect.currentAttackBoostUnits.size() - 1; i >= 0; --i) { + int findUnitId = currentAttackBoostOriginatorEffect.currentAttackBoostUnits[i]; + +- std::vector::iterator iterFound = std::find(candidateValidIdList.begin(), candidateValidIdList.end(), findUnitId); +- if (iterFound == candidateValidIdList.end()) { ++ if (candidateValidIdSet.count(findUnitId) == 0) { + Unit *affectedUnit = game->getWorld()->findUnitById(findUnitId); + if (affectedUnit != NULL) { + affectedUnit->deapplyAttackBoost(currentAttackBoostOriginatorEffect.skillType->getAttackBoost(), this); diff --git a/defects/megaglest-0001/test/test_attack_boost_lookup b/defects/megaglest-0001/test/test_attack_boost_lookup new file mode 100755 index 000000000..1c133c3c6 Binary files /dev/null and b/defects/megaglest-0001/test/test_attack_boost_lookup differ diff --git a/defects/megaglest-0001/test/test_attack_boost_lookup.cpp b/defects/megaglest-0001/test/test_attack_boost_lookup.cpp new file mode 100644 index 000000000..a98ba1027 --- /dev/null +++ b/defects/megaglest-0001/test/test_attack_boost_lookup.cpp @@ -0,0 +1,188 @@ +// Unit test for megaglest-0001: Attack boost unit membership lookup +// Defect: std::find on vector inside per-frame loop = O(C * B) +// Fix: std::unordered_set for O(1) membership test +// +// Simulates our attack boost update loop where we check if each candidate +// unit is already in our boosted-units list, and check if each boosted unit +// is still a valid candidate. + +#include +#include +#include +#include +#include +#include + +// --- DEFECTIVE: O(C * B) linear scan --- +struct AttackBoostDefective { + std::vector currentAttackBoostUnits; + + void updateBoost(const std::vector &candidateIds) { + std::vector candidateValidIdList; + candidateValidIdList.reserve(candidateIds.size()); + + for (size_t i = 0; i < candidateIds.size(); ++i) { + int id = candidateIds[i]; + candidateValidIdList.push_back(id); + + // O(B) linear scan per candidate + auto iterFound = std::find(currentAttackBoostUnits.begin(), + currentAttackBoostUnits.end(), id); + if (iterFound == currentAttackBoostUnits.end()) { + // New unit: apply boost + currentAttackBoostUnits.push_back(id); + } + } + + // Remove units no longer in range: O(B * C) linear scan + for (int i = (int)currentAttackBoostUnits.size() - 1; i >= 0; --i) { + int findUnitId = currentAttackBoostUnits[i]; + auto iterFound = std::find(candidateValidIdList.begin(), + candidateValidIdList.end(), findUnitId); + if (iterFound == candidateValidIdList.end()) { + currentAttackBoostUnits.erase( + currentAttackBoostUnits.begin() + i); + } + } + } +}; + +// --- FIXED: O(C + B) with hash sets --- +struct AttackBoostFixed { + std::vector currentAttackBoostUnits; + + void updateBoost(const std::vector &candidateIds) { + std::unordered_set candidateValidIdSet; + candidateValidIdSet.reserve(candidateIds.size()); + + // Build hash set of currently boosted IDs for O(1) lookup + std::unordered_set boostedIdSet( + currentAttackBoostUnits.begin(), + currentAttackBoostUnits.end()); + + for (size_t i = 0; i < candidateIds.size(); ++i) { + int id = candidateIds[i]; + candidateValidIdSet.insert(id); + + // O(1) hash lookup + if (boostedIdSet.count(id) == 0) { + currentAttackBoostUnits.push_back(id); + boostedIdSet.insert(id); + } + } + + // Remove units no longer in range: O(1) hash lookup per boosted unit + for (int i = (int)currentAttackBoostUnits.size() - 1; i >= 0; --i) { + int findUnitId = currentAttackBoostUnits[i]; + if (candidateValidIdSet.count(findUnitId) == 0) { + currentAttackBoostUnits.erase( + currentAttackBoostUnits.begin() + i); + } + } + } +}; + +// Verify correctness: both produce same result +void test_correctness() { + printf("test_correctness... "); + + // Initial boosted set + std::vector initialBoosted = {10, 20, 30, 40, 50}; + // New candidates: some overlap, some new, some old ones gone + std::vector candidates = {20, 30, 60, 70, 80}; + + AttackBoostDefective defective; + defective.currentAttackBoostUnits = initialBoosted; + defective.updateBoost(candidates); + + AttackBoostFixed fixed; + fixed.currentAttackBoostUnits = initialBoosted; + fixed.updateBoost(candidates); + + // Sort both for comparison + std::vector defResult = defective.currentAttackBoostUnits; + std::vector fixResult = fixed.currentAttackBoostUnits; + std::sort(defResult.begin(), defResult.end()); + std::sort(fixResult.begin(), fixResult.end()); + + assert(defResult == fixResult); + // Should contain: 20, 30, 60, 70, 80 (10, 40, 50 removed; 60, 70, 80 added) + std::vector expected = {20, 30, 60, 70, 80}; + assert(defResult == expected); + + printf("PASS\n"); +} + +// Verify empty candidates removes all boosted +void test_empty_candidates() { + printf("test_empty_candidates... "); + + std::vector initialBoosted = {1, 2, 3, 4, 5}; + std::vector candidates = {}; + + AttackBoostDefective defective; + defective.currentAttackBoostUnits = initialBoosted; + defective.updateBoost(candidates); + + AttackBoostFixed fixed; + fixed.currentAttackBoostUnits = initialBoosted; + fixed.updateBoost(candidates); + + assert(defective.currentAttackBoostUnits.empty()); + assert(fixed.currentAttackBoostUnits.empty()); + + printf("PASS\n"); +} + +// Benchmark: measure O(C*B) vs O(C+B) +void test_performance() { + printf("test_performance... "); + + const int B = 2000; // boosted units + const int C = 2000; // candidates + const int ITERS = 20; + + // Build initial boosted list (1..B) + std::vector initialBoosted; + for (int i = 1; i <= B; ++i) initialBoosted.push_back(i); + + // Candidates: half overlap (1..C/2), half new (B+1..B+C/2) + std::vector candidates; + for (int i = 1; i <= C / 2; ++i) candidates.push_back(i); + for (int i = B + 1; i <= B + C / 2; ++i) candidates.push_back(i); + + // Defective + auto t0 = std::chrono::high_resolution_clock::now(); + for (int iter = 0; iter < ITERS; ++iter) { + AttackBoostDefective d; + d.currentAttackBoostUnits = initialBoosted; + d.updateBoost(candidates); + } + auto t1 = std::chrono::high_resolution_clock::now(); + + // Fixed + auto t2 = std::chrono::high_resolution_clock::now(); + for (int iter = 0; iter < ITERS; ++iter) { + AttackBoostFixed f; + f.currentAttackBoostUnits = initialBoosted; + f.updateBoost(candidates); + } + auto t3 = std::chrono::high_resolution_clock::now(); + + double defective_ms = std::chrono::duration(t1 - t0).count(); + double fixed_ms = std::chrono::duration(t3 - t2).count(); + double ratio = defective_ms / fixed_ms; + + printf("defective=%.1fms fixed=%.1fms ratio=%.1fx ", defective_ms, fixed_ms, ratio); + assert(ratio > 2.0); + printf("PASS\n"); +} + +int main() { + printf("=== megaglest-0001: Attack boost unit membership O(C*B) -> O(C+B) ===\n"); + test_correctness(); + test_empty_candidates(); + test_performance(); + printf("All tests passed.\n"); + return 0; +} diff --git a/defects/megaglest-0002/patch/megaglest-0002.patch b/defects/megaglest-0002/patch/megaglest-0002.patch new file mode 100644 index 000000000..fa849ac25 --- /dev/null +++ b/defects/megaglest-0002/patch/megaglest-0002.patch @@ -0,0 +1,62 @@ +--- a/source/glest_game/world/unit_updater.h ++++ b/source/glest_game/world/unit_updater.h +@@ -1,6 +1,7 @@ + #ifndef _GLEST_GAME_UNITUPDATER_H_ + #define _GLEST_GAME_UNITUPDATER_H_ + ++#include + #include + + ... +@@ -127,7 +128,7 @@ + vector findUnitsInRange(const Unit *unit, int radius); + + private: +- void findUnitsForCell(Cell *cell, vector &units); ++ void findUnitsForCell(Cell *cell, vector &units, std::unordered_set &seenIds); + +--- a/source/glest_game/world/unit_updater.cpp ++++ b/source/glest_game/world/unit_updater.cpp +@@ -3461,13 +3461,10 @@ +-void UnitUpdater::findUnitsForCell(Cell *cell, vector &units) { ++void UnitUpdater::findUnitsForCell(Cell *cell, vector &units, std::unordered_set &seenIds) { + // all fields + if (cell != NULL) { + for (int k = 0; k < fieldCount; k++) { + Field f = static_cast(k); + + // check field + Unit *cellUnit = cell->getUnit(f); + + if (cellUnit != NULL && cellUnit->isAlive()) { +- // check if unit already is in list +- bool found = false; +- for (unsigned int i = 0; i < units.size(); ++i) { +- Unit *unitInList = units[i]; +- if (unitInList->getId() == cellUnit->getId()) { +- found = true; +- break; +- } +- } +- if (found == false) { ++ // O(1) dedup via hash set instead of O(U) linear scan ++ if (seenIds.insert(cellUnit->getId()).second) { + units.push_back(cellUnit); + } + } +@@ -3492,6 +3489,8 @@ + vector UnitUpdater::findUnitsInRange(const Unit *unit, int radius) { + int range = radius; + vector units; ++ std::unordered_set seenIds; + + // aux vars + int size = unit->getType()->getSize(); +@@ -3507,7 +3506,7 @@ + #endif + Cell *cell = map->getCell(i, j); +- findUnitsForCell(cell, units); ++ findUnitsForCell(cell, units, seenIds); + } + } + } diff --git a/defects/megaglest-0002/test/test_find_units_dedup b/defects/megaglest-0002/test/test_find_units_dedup new file mode 100755 index 000000000..db7d5689b Binary files /dev/null and b/defects/megaglest-0002/test/test_find_units_dedup differ diff --git a/defects/megaglest-0002/test/test_find_units_dedup.cpp b/defects/megaglest-0002/test/test_find_units_dedup.cpp new file mode 100644 index 000000000..02995e7a8 --- /dev/null +++ b/defects/megaglest-0002/test/test_find_units_dedup.cpp @@ -0,0 +1,152 @@ +// Unit test for megaglest-0002: findUnitsForCell O(R²*U) linear dedup +// Defect: linear scan of units vector to check for duplicates in +// findUnitsForCell, called once per cell in radius grid = O(R² * U) +// Fix: std::unordered_set for O(1) duplicate check +// +// Simulates our findUnitsInRange hot path where cells in a radius grid +// may reference our same unit (multi-cell units), requiring dedup. + +#include +#include +#include +#include +#include +#include + +// Simulated cell unit reference +struct CellUnit { + int id; + bool alive; +}; + +// --- DEFECTIVE: O(cells * U) linear scan for dedup --- +std::vector findUnitsInRangeDefective(const std::vector &cells) { + std::vector units; + for (size_t c = 0; c < cells.size(); ++c) { + const CellUnit &cu = cells[c]; + if (!cu.alive) continue; + + // Linear scan for duplicate check + bool found = false; + for (size_t i = 0; i < units.size(); ++i) { + if (units[i] == cu.id) { + found = true; + break; + } + } + if (!found) { + units.push_back(cu.id); + } + } + return units; +} + +// --- FIXED: O(cells) with hash set dedup --- +std::vector findUnitsInRangeFixed(const std::vector &cells) { + std::vector units; + std::unordered_set seenIds; + for (size_t c = 0; c < cells.size(); ++c) { + const CellUnit &cu = cells[c]; + if (!cu.alive) continue; + + // O(1) hash set dedup + if (seenIds.insert(cu.id).second) { + units.push_back(cu.id); + } + } + return units; +} + +void test_correctness() { + printf("test_correctness... "); + + // Simulate cells where same unit appears multiple times (multi-cell units) + std::vector cells = { + {1, true}, {2, true}, {1, true}, {3, true}, + {2, true}, {4, true}, {5, false}, {3, true}, + {6, true}, {1, true} + }; + + auto defResult = findUnitsInRangeDefective(cells); + auto fixResult = findUnitsInRangeFixed(cells); + + std::sort(defResult.begin(), defResult.end()); + std::sort(fixResult.begin(), fixResult.end()); + + assert(defResult == fixResult); + std::vector expected = {1, 2, 3, 4, 6}; + assert(defResult == expected); + + printf("PASS\n"); +} + +void test_all_same_unit() { + printf("test_all_same_unit... "); + + // Worst case: all cells reference same unit (large multi-cell building) + std::vector cells; + for (int i = 0; i < 100; ++i) { + cells.push_back({42, true}); + } + + auto defResult = findUnitsInRangeDefective(cells); + auto fixResult = findUnitsInRangeFixed(cells); + + assert(defResult.size() == 1); + assert(fixResult.size() == 1); + assert(defResult[0] == 42); + assert(fixResult[0] == 42); + + printf("PASS\n"); +} + +void test_performance() { + printf("test_performance... "); + + // Simulate radius=15, size=1: (2*15+1)^2 = 961 cells + // With ~100 unique units, each occupying ~10 cells on average + const int NUM_CELLS = 5000; + const int NUM_UNIQUE_UNITS = 500; + const int ITERS = 200; + + std::vector cells; + cells.reserve(NUM_CELLS); + for (int c = 0; c < NUM_CELLS; ++c) { + // Distribute units across cells with heavy overlap + int unitId = (c * 7) % NUM_UNIQUE_UNITS + 1; + cells.push_back({unitId, true}); + } + + // Defective + auto t0 = std::chrono::high_resolution_clock::now(); + for (int iter = 0; iter < ITERS; ++iter) { + auto result = findUnitsInRangeDefective(cells); + assert(!result.empty()); + } + auto t1 = std::chrono::high_resolution_clock::now(); + + // Fixed + auto t2 = std::chrono::high_resolution_clock::now(); + for (int iter = 0; iter < ITERS; ++iter) { + auto result = findUnitsInRangeFixed(cells); + assert(!result.empty()); + } + auto t3 = std::chrono::high_resolution_clock::now(); + + double defective_ms = std::chrono::duration(t1 - t0).count(); + double fixed_ms = std::chrono::duration(t3 - t2).count(); + double ratio = defective_ms / fixed_ms; + + printf("defective=%.1fms fixed=%.1fms ratio=%.1fx ", defective_ms, fixed_ms, ratio); + assert(ratio > 2.0); + printf("PASS\n"); +} + +int main() { + printf("=== megaglest-0002: findUnitsForCell dedup O(R²*U) -> O(R²) ===\n"); + test_correctness(); + test_all_same_unit(); + test_performance(); + printf("All tests passed.\n"); + return 0; +}