From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Aikar Date: Mon, 25 May 2020 11:02:42 -0400 Subject: [PATCH] Unload leaked Cached Chunks Due to some complexity in mojangs complicated chain of juggling whether or not a chunk should be unloaded when the last ticket is removed, many chunks are remaining around in the cache. These chunks are never being targetted for unload because they are vastly out of view distance range and have no reason to be looked at. This is a huge issue for performance because we have to iterate these chunks EVERY TICK... This is what's been leading to high SELF time in Ticking Chunks timings/profiler results. We will now detect these chunks in that iteration, and automatically add it to the unload queue when the chunk is found without any tickets. diff --git a/src/main/java/net/minecraft/server/ChunkMapDistance.java b/src/main/java/net/minecraft/server/ChunkMapDistance.java index 9805361e2d49fa1cfecf0c5811187fc503d0ad8e..62ed8e653c2060c314b407f698842e9c7c3312c0 100644 --- a/src/main/java/net/minecraft/server/ChunkMapDistance.java +++ b/src/main/java/net/minecraft/server/ChunkMapDistance.java @@ -33,7 +33,7 @@ public abstract class ChunkMapDistance { public final Long2ObjectOpenHashMap>> tickets = new Long2ObjectOpenHashMap(); private final ChunkMapDistance.a e = new ChunkMapDistance.a(); public static final int MOB_SPAWN_RANGE = 8; //private final ChunkMapDistance.b f = new ChunkMapDistance.b(8); // Paper - no longer used - private final ChunkMapDistance.c g = new ChunkMapDistance.c(33); + private final ChunkMapDistance.c g = new ChunkMapDistance.c(33); public final ChunkMapDistance.c getLevelTracker() { return g; } // Paper // Paper start use a queue, but still keep unique requirement public final java.util.Queue pendingChunkUpdates = new java.util.ArrayDeque() { @Override @@ -478,7 +478,7 @@ public abstract class ChunkMapDistance { if (flag1) { ChunkMapDistance.this.j.a(ChunkTaskQueueSorter.a(() -> { // CraftBukkit - decompile error ChunkMapDistance.this.m.execute(() -> { - if (this.c(this.c(i))) { + if (this.c(this.c(i))) { // Paper - diff above isChunkLoaded ChunkMapDistance.this.addTicket(i, ticket); ChunkMapDistance.this.l.add(i); } else { diff --git a/src/main/java/net/minecraft/server/ChunkProviderServer.java b/src/main/java/net/minecraft/server/ChunkProviderServer.java index 54e89c9cc6c47ff2c4f4dd5d4c22a391f8a3d6e0..144e91b303cbd9c58c9e6d598e9c9334f2a75c73 100644 --- a/src/main/java/net/minecraft/server/ChunkProviderServer.java +++ b/src/main/java/net/minecraft/server/ChunkProviderServer.java @@ -560,6 +560,7 @@ public class ChunkProviderServer extends IChunkProvider { } } // Paper start + if (playerchunk != null) playerchunk.lastActivity = world.getTime(); // Paper CompletableFuture> future = this.a(playerchunk, l) ? PlayerChunk.UNLOADED_CHUNK_ACCESS_FUTURE : playerchunk.a(chunkstatus, this.playerChunkMap); if (isUrgent) { future.thenAccept(either -> this.chunkMapDistance.clearUrgent(chunkcoordintpair)); @@ -812,6 +813,7 @@ public class ChunkProviderServer extends IChunkProvider { this.world.timings.countNaturalMobs.stopTiming(); // Paper - timings this.world.getMethodProfiler().exit(); // Paper - replaced by above + final long time = world.getTime(); // Paper final int[] chunksTicked = {0}; this.playerChunkMap.forEachVisibleChunk((playerchunk) -> { // Paper - safe iterator incase chunk loads, also no wrapping Optional optional = ((Either) playerchunk.b().getNow(PlayerChunk.UNLOADED_CHUNK)).left(); @@ -897,7 +899,7 @@ public class ChunkProviderServer extends IChunkProvider { this.world.timings.chunkTicks.stopTiming(); // Spigot // Paper if (chunksTicked[0]++ % 10 == 0) this.world.getMinecraftServer().midTickLoadChunks(); // Paper } - } + } else { checkInactiveChunk(playerchunk, time); } // Paper - check inaccessible chunks }); this.world.getMethodProfiler().enter("customSpawners"); if (flag1) { @@ -913,6 +915,30 @@ public class ChunkProviderServer extends IChunkProvider { this.playerChunkMap.g(); } + // Paper start - remove inaccessible chunks leaked + private void checkInactiveChunk(PlayerChunk playerchunk, long time) { + int ticketLevel = playerchunk.getTicketLevel(); + if (ticketLevel > 33 && ticketLevel == playerchunk.oldTicketLevel && + (playerchunk.lastActivity == 0 || time - playerchunk.lastActivity > 20*120) && + playerchunk.location.pair() % 20 == 0 && playerChunkMap.unloadQueue.size() < 100 + ) { + ChunkStatus chunkHolderStatus = playerchunk.getChunkHolderStatus(); + ChunkStatus desiredStatus = PlayerChunk.getChunkStatus(ticketLevel); + if (chunkHolderStatus != null && !chunkHolderStatus.isAtLeastStatus(desiredStatus)) { + return; + } + if (playerchunk.lastActivity == 0) { + playerchunk.lastActivity = time; + return; + } + playerchunk.lastActivity = time; + if (playerchunk.shouldBeUnloaded()) { + playerChunkMap.unloadQueue.add(playerchunk.location.pair()); + } + } + } + // Paper end + @Override public String getName() { return "ServerChunkCache: " + this.h(); diff --git a/src/main/java/net/minecraft/server/PlayerChunk.java b/src/main/java/net/minecraft/server/PlayerChunk.java index b8fe42e8123e972b1ec97b048c35d90118076e66..18f9a2590f7fa5dfc9070fc5e13121d77f06e79f 100644 --- a/src/main/java/net/minecraft/server/PlayerChunk.java +++ b/src/main/java/net/minecraft/server/PlayerChunk.java @@ -44,6 +44,22 @@ public class PlayerChunk { long lastAutoSaveTime; // Paper - incremental autosave long inactiveTimeStart; // Paper - incremental autosave + // Paper start - unload leaked chunks + long lastActivity; + java.util.concurrent.ConcurrentHashMap dependendedOnBy = new java.util.concurrent.ConcurrentHashMap<>(); + public boolean shouldBeUnloaded() { + if (!neighborPriorities.isEmpty() || !dependendedOnBy.isEmpty()) { + return false; + } + long key = location.pair(); + if (chunkMap.playerViewDistanceNoTickMap.getObjectsInRange(key) != null) { + return false; + } + PlayerChunkMap.a distanceManager = chunkMap.chunkDistanceManager; + ArraySetSorted> tickets = distanceManager.tickets.get(key); + return (tickets == null || tickets.isEmpty()) TODO: Figure out level map; + } + // Paper end // Paper start - optimise isOutsideOfRange // cached here to avoid a map lookup @@ -562,6 +578,7 @@ public class PlayerChunk { protected void a(PlayerChunkMap playerchunkmap) { ChunkStatus chunkstatus = getChunkStatus(this.oldTicketLevel); ChunkStatus chunkstatus1 = getChunkStatus(this.ticketLevel); + if (oldTicketLevel != ticketLevel) lastActivity = chunkMap.world.getTime(); // Paper - chunk leak boolean flag = this.oldTicketLevel <= PlayerChunkMap.GOLDEN_TICKET; boolean flag1 = this.ticketLevel <= PlayerChunkMap.GOLDEN_TICKET; // Paper - diff on change: (flag1 = new ticket level is in loadable range) PlayerChunk.State playerchunk_state = getChunkState(this.oldTicketLevel); diff --git a/src/main/java/net/minecraft/server/PlayerChunkMap.java b/src/main/java/net/minecraft/server/PlayerChunkMap.java index f42507f5a17f9388db738218f58ca76f863274ff..9ee13c741fdcc6b40d175e375e61bffa4c14f45f 100644 --- a/src/main/java/net/minecraft/server/PlayerChunkMap.java +++ b/src/main/java/net/minecraft/server/PlayerChunkMap.java @@ -639,6 +639,7 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d { } })); } + playerchunk.lastActivity = world.getTime(); // Paper - chunk leak ChunkStatus chunkstatus = (ChunkStatus) intfunction.apply(j1); CompletableFuture> completablefuture = playerchunk.a(chunkstatus, this); @@ -646,6 +647,7 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d { if (requestingNeighbor != null && requestingNeighbor != playerchunk && !completablefuture.isDone()) { requestingNeighbor.onNeighborRequest(playerchunk, chunkstatus); completablefuture.thenAccept(either -> { + playerchunk.lastActivity = world.getTime(); // Paper - chunk leak requestingNeighbor.onNeighborDone(playerchunk, chunkstatus, either.left().orElse(null)); }); } @@ -874,6 +876,12 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d { PlayerChunk playerchunk = (PlayerChunk) this.updatingChunks.remove(j); if (playerchunk != null) { + // Paper start - don't unload chunks that should be loaded + if (!playerchunk.shouldBeUnloaded()) { + this.updatingChunks.put(j, playerchunk); + continue; + } + // Paper end this.pendingUnload.put(j, playerchunk); this.updatingChunksModified = true; this.a(j, playerchunk); // Paper - Move up - don't leak chunks @@ -1147,9 +1155,27 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d { return completablefuture.thenComposeAsync((either) -> { return either.map((list) -> { // Paper - Shut up. try { + // Paper start + list.forEach(chunk -> { + PlayerChunk updatingChunk = getUpdatingChunk(chunk.getPos().pair()); + if (updatingChunk != null) { + updatingChunk.dependendedOnBy.put(playerchunk.location, true); + updatingChunk.lastActivity = world.getTime(); + } + }); + // Paper end CompletableFuture> completablefuture1 = chunkstatus.a(this.world, this.chunkGenerator, this.definedStructureManager, this.lightEngine, (ichunkaccess) -> { return this.c(playerchunk); }, list); + // Paper start + completablefuture1.whenComplete((unused, unused2) -> list.forEach(chunk -> { + PlayerChunk updatingChunk = getUpdatingChunk(chunk.getPos().pair()); + if (updatingChunk != null) { + updatingChunk.dependendedOnBy.remove(playerchunk.location); + updatingChunk.lastActivity = world.getTime(); + } + })); + // Paper end this.worldLoadListener.a(chunkcoordintpair, chunkstatus); return completablefuture1; @@ -1167,6 +1193,7 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d { return CompletableFuture.completedFuture(Either.right(playerchunk_failure)); }); }, (runnable) -> { + playerchunk.lastActivity = world.getTime(); // Paper this.mailboxWorldGen.a(ChunkTaskQueueSorter.a(playerchunk, runnable)); // CraftBukkit - decompile error }); } diff --git a/src/main/java/net/minecraft/server/StructureGenerator.java b/src/main/java/net/minecraft/server/StructureGenerator.java index acfe732af5b9f63fc2f6b78499defabe2e73ee45..25b19346fc1c702cc37275d0ec16abbbfacfb418 100644 --- a/src/main/java/net/minecraft/server/StructureGenerator.java +++ b/src/main/java/net/minecraft/server/StructureGenerator.java @@ -160,7 +160,7 @@ public abstract class StructureGenerator while (longiterator.hasNext()) { long k = longiterator.nextLong(); IChunkAccess ichunkaccess1 = generatoraccess.getChunkAt(ChunkCoordIntPair.getX(k), ChunkCoordIntPair.getZ(k), ChunkStatus.STRUCTURE_STARTS, false); // CraftBukkit - don't load chunks - StructureStart structurestart = ichunkaccess1.a(this.b()); + StructureStart structurestart = ichunkaccess1 != null ? ichunkaccess1.a(this.b()) : null; // Paper - make sure not null if (structurestart != null) { list.add(structurestart);