From 47af5acd7a350771d383487b7b4d56981c173378 Mon Sep 17 00:00:00 2001 From: Aikar Date: Fri, 29 May 2020 03:30:58 -0400 Subject: [PATCH] Drop remove leaked chunk patch - causing many issues I'm hoping the other fix in 324 for the level map getting corrupted fixes the real issue and this isn't needed anymore, but i suspect it is will wait until more study can be done though. Fixes #3469 --- ...te-operations-for-updating-light-dat.patch | 21 +- .../Unload-leaked-Cached-Chunks.patch | 223 ------------------ 2 files changed, 14 insertions(+), 230 deletions(-) delete mode 100644 Spigot-Server-Patches/Unload-leaked-Cached-Chunks.patch diff --git a/Spigot-Server-Patches/Stop-copy-on-write-operations-for-updating-light-dat.patch b/Spigot-Server-Patches/Stop-copy-on-write-operations-for-updating-light-dat.patch index b47accc3d3..c4208fa80a 100644 --- a/Spigot-Server-Patches/Stop-copy-on-write-operations-for-updating-light-dat.patch +++ b/Spigot-Server-Patches/Stop-copy-on-write-operations-for-updating-light-dat.patch @@ -71,11 +71,13 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 private final NibbleArray[] c = new NibbleArray[2]; private boolean d; - protected final Long2ObjectOpenHashMap a; -+ protected final com.destroystokyo.paper.util.map.QueuedChangesMapLong2Object data; // Paper - avoid copying light data -+ protected final boolean isVisible; // Paper - avoid copying light data - +- - protected LightEngineStorageArray(Long2ObjectOpenHashMap long2objectopenhashmap) { - this.a = long2objectopenhashmap; ++ protected final com.destroystokyo.paper.util.map.QueuedChangesMapLong2Object data; // Paper - avoid copying light data ++ protected final boolean isVisible; // Paper - avoid copying light data ++ java.util.function.Function lookup; // Paper - faster branchless lookup ++ + // Paper start - avoid copying light data + protected LightEngineStorageArray(com.destroystokyo.paper.util.map.QueuedChangesMapLong2Object data, boolean isVisible) { + if (isVisible) { @@ -83,6 +85,11 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 + } + this.data = data; + this.isVisible = isVisible; ++ if (isVisible) { ++ lookup = data::getVisibleAsync; ++ } else { ++ lookup = data::getUpdating; ++ } + // Paper end - avoid copying light data this.c(); this.d = true; @@ -99,11 +106,12 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 public boolean b(long i) { - return this.a.containsKey(i); -+ return this.isVisible ? this.data.visibleContainsKeyAsync(i) : this.data.updatingContainsKey(i); // Paper - avoid copying light data ++ return lookup.apply(i) != null; // Paper - avoid copying light data } @Nullable - public NibbleArray c(long i) { +- public NibbleArray c(long i) { ++ public final NibbleArray c(long i) { // Paper - final + // Paper start - remove cache - not thread safe + /* if (this.d) { @@ -117,10 +125,9 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 + // Paper end - NibbleArray nibblearray = (NibbleArray) this.a.get(i); -+ NibbleArray nibblearray = (NibbleArray) (this.isVisible ? this.data.getVisibleAsync(i) : this.data.getUpdating(i)); // Paper - avoid copying light data ++ return lookup.apply(i); // Paper - avoid copying light data + // Paper start - remove cache - not thread safe -+ return nibblearray; + /* if (nibblearray == null) { return null; diff --git a/Spigot-Server-Patches/Unload-leaked-Cached-Chunks.patch b/Spigot-Server-Patches/Unload-leaked-Cached-Chunks.patch deleted file mode 100644 index c777c55541..0000000000 --- a/Spigot-Server-Patches/Unload-leaked-Cached-Chunks.patch +++ /dev/null @@ -1,223 +0,0 @@ -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 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 ---- a/src/main/java/net/minecraft/server/ChunkMapDistance.java -+++ b/src/main/java/net/minecraft/server/ChunkMapDistance.java -@@ -0,0 +0,0 @@ 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 -@@ -0,0 +0,0 @@ public abstract class ChunkMapDistance { - this.e = i; - } - -+ // Paper start - check diff below -+ public boolean isChunkLoaded(long i) { -+ return this.c(this.c(i)); -+ } -+ // Paper end -+ - private void a(long i, int j, boolean flag, boolean flag1) { - if (flag != flag1) { - Ticket ticket = new Ticket<>(TicketType.PLAYER, 33, new ChunkCoordIntPair(i)); // Paper - no-tick view distance -@@ -0,0 +0,0 @@ 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 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 ---- a/src/main/java/net/minecraft/server/ChunkProviderServer.java -+++ b/src/main/java/net/minecraft/server/ChunkProviderServer.java -@@ -0,0 +0,0 @@ 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)); -@@ -0,0 +0,0 @@ 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(); - -@@ -0,0 +0,0 @@ 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) { -@@ -0,0 +0,0 @@ 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*5) && -+ 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; -+ Chunk chunk = playerchunk.getChunk(); -+ if ((chunk != null && chunk.isAnyNeighborsLoaded()) || !playerchunk.neighborPriorities.isEmpty() || !playerchunk.dependendedOnBy.isEmpty()) { -+ return; -+ } -+ long key = playerchunk.location.pair(); -+ if (playerChunkMap.playerViewDistanceNoTickMap.getObjectsInRange(key) != null) { -+ return; -+ } -+ PlayerChunkMap.a distanceManager = playerChunkMap.chunkDistanceManager; -+ ArraySetSorted> tickets = distanceManager.tickets.get(key); -+ if ((tickets == null || tickets.isEmpty()) && !distanceManager.getLevelTracker().isChunkLoaded(key)) { -+ playerChunkMap.unloadQueue.add(key); -+ } -+ } -+ } -+ // 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 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 ---- a/src/main/java/net/minecraft/server/PlayerChunk.java -+++ b/src/main/java/net/minecraft/server/PlayerChunk.java -@@ -0,0 +0,0 @@ public class PlayerChunk { - - long lastAutoSaveTime; // Paper - incremental autosave - long inactiveTimeStart; // Paper - incremental autosave -+ long lastActivity; // Paper - fix chunk leak -+ java.util.concurrent.ConcurrentHashMap dependendedOnBy = new java.util.concurrent.ConcurrentHashMap<>(); // Paper - - // Paper start - optimise isOutsideOfRange - // cached here to avoid a map lookup -@@ -0,0 +0,0 @@ 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 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 ---- a/src/main/java/net/minecraft/server/PlayerChunkMap.java -+++ b/src/main/java/net/minecraft/server/PlayerChunkMap.java -@@ -0,0 +0,0 @@ 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); -@@ -0,0 +0,0 @@ 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)); - }); - } -@@ -0,0 +0,0 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d { - while (longiterator.hasNext()) { // Spigot - long j = longiterator.nextLong(); - longiterator.remove(); // Spigot -+ ArraySetSorted> tickets = chunkDistanceManager.tickets.get(j); // Paper - chunk leak -+ if (tickets != null && !tickets.isEmpty()) continue; // Paper - ticket got added, don't remove - PlayerChunk playerchunk = (PlayerChunk) this.updatingChunks.remove(j); - - if (playerchunk != null) { -@@ -0,0 +0,0 @@ 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; -@@ -0,0 +0,0 @@ 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 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 ---- a/src/main/java/net/minecraft/server/StructureGenerator.java -+++ b/src/main/java/net/minecraft/server/StructureGenerator.java -@@ -0,0 +0,0 @@ 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);