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
This commit is contained in:
Aikar 2020-05-29 03:30:58 -04:00
parent 50557cebdb
commit 47af5acd7a
2 changed files with 14 additions and 230 deletions

View File

@ -71,11 +71,13 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000
private final NibbleArray[] c = new NibbleArray[2];
private boolean d;
- protected final Long2ObjectOpenHashMap<NibbleArray> a;
+ protected final com.destroystokyo.paper.util.map.QueuedChangesMapLong2Object<NibbleArray> data; // Paper - avoid copying light data
+ protected final boolean isVisible; // Paper - avoid copying light data
-
- protected LightEngineStorageArray(Long2ObjectOpenHashMap<NibbleArray> long2objectopenhashmap) {
- this.a = long2objectopenhashmap;
+ protected final com.destroystokyo.paper.util.map.QueuedChangesMapLong2Object<NibbleArray> data; // Paper - avoid copying light data
+ protected final boolean isVisible; // Paper - avoid copying light data
+ java.util.function.Function<Long, NibbleArray> lookup; // Paper - faster branchless lookup
+
+ // Paper start - avoid copying light data
+ protected LightEngineStorageArray(com.destroystokyo.paper.util.map.QueuedChangesMapLong2Object<NibbleArray> 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;

View File

@ -1,223 +0,0 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Aikar <aikar@aikar.co>
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<ArraySetSorted<Ticket<?>>> 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<PlayerChunk> pendingChunkUpdates = new java.util.ArrayDeque<PlayerChunk>() {
@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<Either<IChunkAccess, PlayerChunk.Failure>> 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<Chunk> 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<Ticket<?>> 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<ChunkCoordIntPair, Boolean> 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<Either<IChunkAccess, PlayerChunk.Failure>> 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<Ticket<?>> 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<Either<IChunkAccess, PlayerChunk.Failure>> 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<C extends WorldGenFeatureConfiguration>
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);