fix: sweep map holder state of departed players - #14240
Conversation
|
Part of me feels that this should be a form of BiMap rather than relying on a sweep over collections the server doesn't really do a good job of cleaning up; I mean, ideally we'd have proper cleanup of the backing objects but I think mojang made that a bit messy |
|
Agreed a direct index beats a sweep. carriedByPlayers is keyed by live Player and tickCarriedBy never runs for a departed key, so that entry stays pinned. This patch adds MapItemSavedData.removeHolders mirroring that removal branch. PlayerList.remove queues the UUID. MapHolderCleanup.tick sweeps once per 100 ticks via SavedDataStorage.getLoadedData when pendingDepartures is non empty. Rejoined UUIDs are skipped by live lookup. At ba3825f the sweep holds no extra state. The 25k map rig in #14088 measured 1504.2 ms total for 100 per-player scans and 30.308 ms total for one coalesced scan, both clearing 42000 relationships. Single quit measured 10.934 ms and 9.563 ms batched. I did not run a build or heap rig here, numbers above are the reporter's. An index from UUID to maps would make single quits cheaper than a catalogue scan. It needs writes in getHoldingPlayer and drops on unload and in removeHolders. I can switch this to that form. |
See #14088.
Problem: disconnected players stay in every map's carriedByPlayers and carriedBy forever. Each entry pins a ServerPlayer. 100 departed viewers left 101 ServerPlayers and 42,000 stale holder records on the heap after full GC.
Root cause: holders are only removed inside tickCarriedBy, which never runs again for a departed player.
Fix: MapItemSavedData.removeHolders drops holders for given UUIDs in one pass, mirroring the tickCarriedBy removal branch. PlayerList.remove queues the UUID. A 5s debounced server tick sweep cleans all levels in one pass, so quit storms cost one catalogue scan. Rejoined players are skipped by live lookup. SavedDataStorage.getLoadedData exposes loaded entries without disk reads. Holders are runtime-only state, the sweep writes nothing to disk.
Test: hunks apply cleanly with context match, every API used read off the 26.2 classes. No build or heap-rig run here, the numbers above are the reporter's. Patch line numbers are best effort, run rebuildPatches in a dev env if a hunk needs re-anchoring.