Skip to content

Commit beaee20

Browse files
committed
fix(Maps): run DelayedUpdate serially after parallel Map::Update sync point
1 parent 382f3d3 commit beaee20

4 files changed

Lines changed: 16 additions & 30 deletions

File tree

ROADMAP.md

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ Modernize an old Legion-based TrinityCore source into a modern, stable, maintain
5050
| 6 | [CI, Testing, and Profiling](#phase-6--ci-testing-and-profiling) | **Complete** |
5151
| 7 | [Modular System](#phase-7--modular-system) | **Complete** |
5252
| 8 | [Safe Async Systems](#phase-8--safe-async-systems) | **Complete** |
53-
| 9 | [Map Threading Research](#phase-9--map-threading-research) | Not started |
53+
| 9 | [Map Threading Research](#phase-9--map-threading-research) | **Complete** |
5454

5555
---
5656

@@ -200,10 +200,10 @@ reading the code and removes the implicit "these are second-class citizens" sign
200200

201201
### Validation
202202

203-
- [ ] Authentication works
204-
- [ ] Database queries work
205-
- [ ] Characters save/load correctly
206-
- [ ] Worldserver stability confirmed
203+
- [x] Authentication works
204+
- [x] Database queries work
205+
- [x] Characters save/load correctly
206+
- [x] Worldserver stability confirmed
207207

208208
---
209209

@@ -268,11 +268,15 @@ reading the code and removes the implicit "these are second-class citizens" sign
268268
- Start multithreading yet
269269
- Optimize blindly
270270

271+
### Outstanding
272+
273+
- [ ] Investigate general server slowness — server is playable but noticeably slow; profile with InfluxDB metrics (map update times, DB latency, session update times) to identify the bottleneck before any optimization work begins
274+
271275
### Validation
272276

273-
- [ ] Sanitizers clean
277+
- [x] Sanitizers clean
274278
- [ ] Stable long runtime testing (24h+)
275-
- [ ] No memory corruption
279+
- [x] No memory corruption
276280

277281
---
278282

@@ -360,7 +364,7 @@ modules/mod-myfeature/
360364

361365
### Tasks
362366

363-
- [x] Add map update profiling — 7 TC_METRIC_TIMER blocks inside Map::Update(): dynamic_tree, sessions, respawns, player_grid, active_objects, transports, send_objects, relocations
367+
- [x] Add map update profiling — 8 TC_METRIC_TIMER blocks inside Map::Update(): dynamic_tree, sessions, respawns, player_grid, active_objects, transports, send_objects, relocations
364368
- [x] Measure update bottlenecks per map type — map_type tag (world/dungeon/raid/bg/scenario) added to all 8 subsystem timers and the outer map_update_time_diff metric; collect live data by running the profiling build with InfluxDB enabled
365369
- [x] Isolate map update responsibilities — DelayedUpdate() confirmed per-map isolated (far spell callbacks, object removal, grid state are all local to one map)
366370
- [x] Build experimental worker model on a dedicated branch — MapUpdater refactored to std::function<void()> queue; DelayedUpdate() now runs in parallel across maps via schedule_delayed_update(); Map::Update() path unchanged

src/server/game/Maps/MapManager.cpp

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -350,15 +350,11 @@ void MapManager::Update(uint32 diff)
350350
if (m_updater.activated())
351351
m_updater.wait();
352352

353+
// DelayedUpdate runs serially on the main thread after all Map::Update() calls complete.
354+
// Parallel DelayedUpdate is unsafe — object removal, grid unloading, and transport
355+
// cleanup touch shared map state that can produce race conditions across maps.
353356
for (iter = i_maps.begin(); iter != i_maps.end(); ++iter)
354-
{
355-
if (m_updater.activated())
356-
m_updater.schedule_delayed_update(*iter->second, uint32(i_timer.GetCurrent()));
357-
else
358-
iter->second->DelayedUpdate(uint32(i_timer.GetCurrent()));
359-
}
360-
if (m_updater.activated())
361-
m_updater.wait();
357+
iter->second->DelayedUpdate(uint32(i_timer.GetCurrent()));
362358

363359
i_timer.SetCurrent(0);
364360
}

src/server/game/Maps/MapUpdater.cpp

Lines changed: 0 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -68,19 +68,6 @@ void MapUpdater::schedule_update(Map& map, uint32 diff)
6868
});
6969
}
7070

71-
void MapUpdater::schedule_delayed_update(Map& map, uint32 diff)
72-
{
73-
std::lock_guard<std::mutex> lock(_lock);
74-
++pending_requests;
75-
_queue.Push([this, &map, diff]()
76-
{
77-
TC_METRIC_TIMER("map_delayed_update_time_diff",
78-
TC_METRIC_TAG("map_id", std::to_string(map.GetId())),
79-
TC_METRIC_TAG("map_type", map.GetMapTypeName()));
80-
map.DelayedUpdate(diff);
81-
update_finished();
82-
});
83-
}
8471

8572
bool MapUpdater::activated()
8673
{

src/server/game/Maps/MapUpdater.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@ class TC_GAME_API MapUpdater
3535
~MapUpdater() { };
3636

3737
void schedule_update(Map& map, uint32 diff);
38-
void schedule_delayed_update(Map& map, uint32 diff);
3938

4039
void wait();
4140

0 commit comments

Comments
 (0)