feat(perf): Unload non ticking chunks - #14
Conversation
|
@remminiscent Ready? |
currently, it works. however it needs proper testing for different scenarios. will do later tonight |
| } | ||
|
|
||
| public function onChunkUnloaded(int $chunkX, int $chunkZ, Chunk $chunk) : void{ | ||
| $hash = World::chunkHash($chunkX, $chunkZ); |
There was a problem hiding this comment.
The early return can't tell a deliberate unload from a forced one. A plugin calling $world->unloadChunk gets swallowed: chunk stays SENT, orderChunks never re-queues it, client keeps stale terrain forever. Needs an explicit flag on the deliberate path, not inference from ticking state.
| } | ||
|
|
||
| return false; | ||
| } |
There was a problem hiding this comment.
Full scan of usedChunks on every orderChunks(), with a getChunkEntities() per entry. That's the set this PR makes large. updateTickingChunkRegistrations() above works so this should too afaik
| @@ -988,6 +992,35 @@ public function doFirstSpawn() : void{ | |||
| } | |||
| } | |||
There was a problem hiding this comment.
Why a 1-chunk margin? Presumably edge ticking needs loaded neighbours. Put it in a comment.
| $this->loadQueue = $newOrder; | ||
|
|
||
| $this->updateTickingChunkRegistrations($this->tickingChunks, $tickingChunks); | ||
| $oldTickingChunks = $this->tickingChunks; |
There was a problem hiding this comment.
$oldTickingChunks used once as an arg. Drop it.
| ){} | ||
|
|
||
| public function retain(int $chunkX, int $chunkZ) : void{ | ||
| $chunkHash = World::chunkHash($chunkX, $chunkZ); | ||
| $this->usageCounts[$chunkHash] = ($this->usageCounts[$chunkHash] ?? 0) + 1; | ||
| } |
There was a problem hiding this comment.
Document those functions please
| return; | ||
| } | ||
| $worldId = $world->getId(); | ||
| $chunkHash = World::chunkHash($chunkX, $chunkZ); |
There was a problem hiding this comment.
On world change this calls getInstance($oldWorld, ...). If the old world is already unloaded it reconstructs an instance and registers an addOnUnloadCallback that never fires, leaking self::$instances. Probably unreachable since Server::unloadWorld() evacuates first, but don't depend on that ordering.
xRookieFight
changed the title
|
We should merge this into "dev" branch |
2 participants
Related issues & PRs
Behavioural changes
Great ram usage decrease in servers that have large open worlds, and high render distances
Tests
Tested in game, works.
Loaded chunks at view-distance 16:
Loaded chunks at view-distance 24:
In both tests, the ticking chunks remained at 36 (chunk radius: 3)