Skip to content

Commit

Permalink
Many fixes and improvements to chunk prioritization
Browse files Browse the repository at this point in the history
I believe this brings us back to stable. A lot of complexity was
learned about juggling priorities.

We were essentially promoting more chunks to urgent than really
needed to be urgent.

So this commit adds a lot more logic to juggle neighbor priorities
and demote their priority once they meet the requirements needed of
them.

This greatly improves the performance of "urgent" chunks".

Fixes PaperMC#3410
Fixes PaperMC#3426
Fixes PaperMC#3425
Fixes PaperMC#3416
  • Loading branch information
aikar committed May 22, 2020
1 parent 281181c commit 4d38ee1
Show file tree
Hide file tree
Showing 11 changed files with 316 additions and 208 deletions.
34 changes: 21 additions & 13 deletions Spigot-Server-Patches/0390-Asynchronous-chunk-IO-and-loading.patch
Original file line number Diff line number Diff line change
Expand Up @@ -1847,10 +1847,10 @@ index 0000000000000000000000000000000000000000..1dfa8abfd869ca97e4cc566d44e509b4
+}
diff --git a/src/main/java/com/destroystokyo/paper/io/chunk/ChunkTaskManager.java b/src/main/java/com/destroystokyo/paper/io/chunk/ChunkTaskManager.java
new file mode 100644
index 0000000000000000000000000000000000000000..d9580eb998801edd34c610ced3f82f9627c6685b
index 0000000000000000000000000000000000000000..a5f4cdaf06bfbb0dd957db9a1335c17b073d646d
--- /dev/null
+++ b/src/main/java/com/destroystokyo/paper/io/chunk/ChunkTaskManager.java
@@ -0,0 +1,502 @@
@@ -0,0 +1,509 @@
+package com.destroystokyo.paper.io.chunk;
+
+import com.destroystokyo.paper.io.PaperFileIOThread;
Expand All @@ -1870,6 +1870,8 @@ index 0000000000000000000000000000000000000000..d9580eb998801edd34c610ced3f82f96
+import org.spigotmc.AsyncCatcher;
+
+import java.util.ArrayDeque;
+import java.util.HashSet;
+import java.util.Set;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentLinkedQueue;
Expand Down Expand Up @@ -1940,6 +1942,7 @@ index 0000000000000000000000000000000000000000..d9580eb998801edd34c610ced3f82f96
+ }
+
+ PaperFileIOThread.LOGGER.log(Level.ERROR, "Chunk wait task info below: ");
+ Set<PlayerChunk> seenChunks = new HashSet<>();
+
+ for (final ChunkInfo chunkInfo : WAITING_CHUNKS) {
+ final long key = IOUtil.getCoordinateKey(chunkInfo.chunkX, chunkInfo.chunkZ);
Expand All @@ -1952,15 +1955,19 @@ index 0000000000000000000000000000000000000000..d9580eb998801edd34c610ced3f82f96
+ // log current status of chunk to indicate whether we're waiting on generation or loading
+ net.minecraft.server.PlayerChunk chunkHolder = chunkInfo.world.getChunkProvider().playerChunkMap.getVisibleChunk(key);
+
+ dumpChunkInfo(chunkHolder, chunkInfo.chunkX, chunkInfo.chunkZ);
+ dumpChunkInfo(seenChunks, chunkHolder, chunkInfo.chunkX, chunkInfo.chunkZ);
+ }
+ }
+ }
+
+ static void dumpChunkInfo(PlayerChunk chunkHolder, int x, int z) {
+ dumpChunkInfo(chunkHolder, x, z, 0);
+ static void dumpChunkInfo(Set<PlayerChunk> seenChunks, PlayerChunk chunkHolder, int x, int z) {
+ dumpChunkInfo(seenChunks, chunkHolder, x, z, 0);
+ }
+ static void dumpChunkInfo(PlayerChunk chunkHolder, int x, int z, int indent) {
+ static void dumpChunkInfo(Set<PlayerChunk> seenChunks, PlayerChunk chunkHolder, int x, int z, int indent) {
+ if (seenChunks.contains(chunkHolder)) {
+ return;
+ }
+ seenChunks.add(chunkHolder);
+ String indentStr = StringUtils.repeat(" ", indent);
+ if (chunkHolder == null) {
+ PaperFileIOThread.LOGGER.log(Level.ERROR, indentStr + "Chunk Holder - null for (" + x +"," + z +")");
Expand Down Expand Up @@ -2354,10 +2361,10 @@ index 0000000000000000000000000000000000000000..d9580eb998801edd34c610ced3f82f96
+
+}
diff --git a/src/main/java/net/minecraft/server/ChunkProviderServer.java b/src/main/java/net/minecraft/server/ChunkProviderServer.java
index 4c9c8e483974f8869d6711626620cfd7d814d956..54325d9305953fa7520feec6c80e2931888141c2 100644
index 4c9c8e483974f8869d6711626620cfd7d814d956..a88e8598aab55ac769a5f186507f362e4f99cef4 100644
--- a/src/main/java/net/minecraft/server/ChunkProviderServer.java
+++ b/src/main/java/net/minecraft/server/ChunkProviderServer.java
@@ -299,11 +299,137 @@ public class ChunkProviderServer extends IChunkProvider {
@@ -299,11 +299,138 @@ public class ChunkProviderServer extends IChunkProvider {
return playerChunk.getAvailableChunkNow();

}
Expand All @@ -2380,7 +2387,8 @@ index 4c9c8e483974f8869d6711626620cfd7d814d956..54325d9305953fa7520feec6c80e2931
+ }
+
+ if (!com.destroystokyo.paper.PaperConfig.asyncChunks) {
+ Chunk chunk = getChunkAt(x, z, gen);
+ world.getWorld().loadChunk(x, z, gen);
+ Chunk chunk = getChunkAtIfLoadedMainThread(x, z);
+ return CompletableFuture.completedFuture(chunk != null ? Either.left(chunk) : PlayerChunk.UNLOADED_CHUNK_ACCESS);
+ }
+
Expand Down Expand Up @@ -2419,7 +2427,7 @@ index 4c9c8e483974f8869d6711626620cfd7d814d956..54325d9305953fa7520feec6c80e2931
+ IChunkAccess current = this.getChunkAtImmediately(x, z); // we want to bypass ticket restrictions
+ if (current != null) {
+ if (!(current instanceof ProtoChunkExtension) && !(current instanceof net.minecraft.server.Chunk)) {
+ return CompletableFuture.completedFuture(Either.left(null));
+ return CompletableFuture.completedFuture(PlayerChunk.UNLOADED_CHUNK_ACCESS);
+ }
+ // we know the chunk is at full status here (either in read-only mode or the real thing)
+ return this.bringToFullStatusAsync(x, z, chunkPos, isUrgent);
Expand Down Expand Up @@ -2495,7 +2503,7 @@ index 4c9c8e483974f8869d6711626620cfd7d814d956..54325d9305953fa7520feec6c80e2931
if (Thread.currentThread() != this.serverThread) {
return (IChunkAccess) CompletableFuture.supplyAsync(() -> {
return this.getChunkAt(i, j, chunkstatus, flag);
@@ -326,11 +452,16 @@ public class ChunkProviderServer extends IChunkProvider {
@@ -326,11 +453,16 @@ public class ChunkProviderServer extends IChunkProvider {
}

gameprofilerfiller.c("getChunkCacheMiss");
Expand All @@ -2513,7 +2521,7 @@ index 4c9c8e483974f8869d6711626620cfd7d814d956..54325d9305953fa7520feec6c80e2931
this.world.timings.syncChunkLoad.stopTiming(); // Paper
} // Paper
ichunkaccess = (IChunkAccess) ((Either) completablefuture.join()).map((ichunkaccess1) -> {
@@ -396,6 +527,11 @@ public class ChunkProviderServer extends IChunkProvider {
@@ -396,6 +528,11 @@ public class ChunkProviderServer extends IChunkProvider {
}

private CompletableFuture<Either<IChunkAccess, PlayerChunk.Failure>> getChunkFutureMainThread(int i, int j, ChunkStatus chunkstatus, boolean flag) {
Expand All @@ -2525,7 +2533,7 @@ index 4c9c8e483974f8869d6711626620cfd7d814d956..54325d9305953fa7520feec6c80e2931
ChunkCoordIntPair chunkcoordintpair = new ChunkCoordIntPair(i, j);
long k = chunkcoordintpair.pair();
int l = 33 + ChunkStatus.a(chunkstatus);
@@ -835,11 +971,12 @@ public class ChunkProviderServer extends IChunkProvider {
@@ -835,11 +972,12 @@ public class ChunkProviderServer extends IChunkProvider {
protected boolean executeNext() {
// CraftBukkit start - process pending Chunk loadCallback() and unloadCallback() after each run task
try {
Expand Down
4 changes: 2 additions & 2 deletions Spigot-Server-Patches/0392-Reduce-sync-loads.patch
Original file line number Diff line number Diff line change
Expand Up @@ -286,10 +286,10 @@ index 0000000000000000000000000000000000000000..59aec103295f747793fdc0a52eb45f41
+ }
+}
diff --git a/src/main/java/net/minecraft/server/ChunkProviderServer.java b/src/main/java/net/minecraft/server/ChunkProviderServer.java
index 54325d9305953fa7520feec6c80e2931888141c2..32bea1dea9ebb05ed94f5b47e6ad2145f6319431 100644
index a88e8598aab55ac769a5f186507f362e4f99cef4..21e444e6ad78081353a7330b60c74164e4596d61 100644
--- a/src/main/java/net/minecraft/server/ChunkProviderServer.java
+++ b/src/main/java/net/minecraft/server/ChunkProviderServer.java
@@ -459,6 +459,7 @@ public class ChunkProviderServer extends IChunkProvider {
@@ -460,6 +460,7 @@ public class ChunkProviderServer extends IChunkProvider {
this.world.asyncChunkTaskManager.raisePriority(x, z, com.destroystokyo.paper.io.PrioritizedTaskQueue.HIGHEST_PRIORITY);
com.destroystokyo.paper.io.chunk.ChunkTaskManager.pushChunkWait(this.world, x, z);
// Paper end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -545,10 +545,10 @@ index 0000000000000000000000000000000000000000..4f13d3ff8391793a99f067189f854078
+ }
+}
diff --git a/src/main/java/net/minecraft/server/ChunkProviderServer.java b/src/main/java/net/minecraft/server/ChunkProviderServer.java
index 32bea1dea9ebb05ed94f5b47e6ad2145f6319431..7a4e2c350e78b22dc035471ad0d7191dfd7afede 100644
index 21e444e6ad78081353a7330b60c74164e4596d61..927abc78e973e6f0f87e12303409fd808b3cf6ab 100644
--- a/src/main/java/net/minecraft/server/ChunkProviderServer.java
+++ b/src/main/java/net/minecraft/server/ChunkProviderServer.java
@@ -746,7 +746,22 @@ public class ChunkProviderServer extends IChunkProvider {
@@ -747,7 +747,22 @@ public class ChunkProviderServer extends IChunkProvider {
this.world.timings.countNaturalMobs.startTiming(); // Paper - timings
int l = this.chunkMapDistance.b();
EnumCreatureType[] aenumcreaturetype = EnumCreatureType.values();
Expand All @@ -572,7 +572,7 @@ index 32bea1dea9ebb05ed94f5b47e6ad2145f6319431..7a4e2c350e78b22dc035471ad0d7191d

this.world.timings.countNaturalMobs.stopTiming(); // Paper - timings
this.world.getMethodProfiler().exit();
@@ -814,8 +829,23 @@ public class ChunkProviderServer extends IChunkProvider {
@@ -815,8 +830,23 @@ public class ChunkProviderServer extends IChunkProvider {
if (enumcreaturetype != EnumCreatureType.MISC && (!enumcreaturetype.c() || this.allowAnimals) && (enumcreaturetype.c() || this.allowMonsters) && (!enumcreaturetype.d() || flag2)) {
int k1 = limit * l / ChunkProviderServer.b; // CraftBukkit - use per-world limits

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,10 @@ bypass the need to get a player chunk, then get the either,
then unwrap it...

diff --git a/src/main/java/net/minecraft/server/ChunkProviderServer.java b/src/main/java/net/minecraft/server/ChunkProviderServer.java
index 7a4e2c350e78b22dc035471ad0d7191dfd7afede..4f65c3aca4e1c299114c03339605e0749a969653 100644
index 927abc78e973e6f0f87e12303409fd808b3cf6ab..6688b1340e2cc8fb13a7e80c9b7c37b8822dcecd 100644
--- a/src/main/java/net/minecraft/server/ChunkProviderServer.java
+++ b/src/main/java/net/minecraft/server/ChunkProviderServer.java
@@ -435,6 +435,12 @@ public class ChunkProviderServer extends IChunkProvider {
@@ -436,6 +436,12 @@ public class ChunkProviderServer extends IChunkProvider {
return this.getChunkAt(i, j, chunkstatus, flag);
}, this.serverThreadQueue).join();
} else {
Expand All @@ -23,7 +23,7 @@ index 7a4e2c350e78b22dc035471ad0d7191dfd7afede..4f65c3aca4e1c299114c03339605e074
GameProfilerFiller gameprofilerfiller = this.world.getMethodProfiler();

gameprofilerfiller.c("getChunk");
@@ -485,39 +491,7 @@ public class ChunkProviderServer extends IChunkProvider {
@@ -486,39 +492,7 @@ public class ChunkProviderServer extends IChunkProvider {
if (Thread.currentThread() != this.serverThread) {
return null;
} else {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,10 +57,10 @@ index 0000000000000000000000000000000000000000..f6ff4d8132a95895680f5bc81f8f873e
+ }
+}
diff --git a/src/main/java/net/minecraft/server/ChunkProviderServer.java b/src/main/java/net/minecraft/server/ChunkProviderServer.java
index fd998e4fb1534690a2ef8c1bca55e0ae9fe855f9..8f849d83d08b39f1cd9184f484a2089a7a3124ef 100644
index d53b34ba552771bf271131ce0a56ebb992ccc84c..a1b5e6b90fc93f83186cf3ebf3e158767008c69a 100644
--- a/src/main/java/net/minecraft/server/ChunkProviderServer.java
+++ b/src/main/java/net/minecraft/server/ChunkProviderServer.java
@@ -755,7 +755,7 @@ public class ChunkProviderServer extends IChunkProvider {
@@ -756,7 +756,7 @@ public class ChunkProviderServer extends IChunkProvider {
entityPlayer.playerNaturallySpawnedEvent.callEvent();
};
// Paper end
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,26 +56,26 @@ index f1b41e16c8ce8323a896339c5d822f8ff7d8f7e6..f8f225e18fa38cad917f52a379233e0a
+ }
}
diff --git a/src/main/java/net/minecraft/server/ChunkProviderServer.java b/src/main/java/net/minecraft/server/ChunkProviderServer.java
index 8f849d83d08b39f1cd9184f484a2089a7a3124ef..377d554553ce81f66207541d963f826867e66592 100644
index a1b5e6b90fc93f83186cf3ebf3e158767008c69a..2ef8506f10426b8a5877e30986c105c0d95be774 100644
--- a/src/main/java/net/minecraft/server/ChunkProviderServer.java
+++ b/src/main/java/net/minecraft/server/ChunkProviderServer.java
@@ -688,6 +688,7 @@ public class ChunkProviderServer extends IChunkProvider {
@@ -689,6 +689,7 @@ public class ChunkProviderServer extends IChunkProvider {
this.world.getMethodProfiler().enter("purge");
this.world.timings.doChunkMap.startTiming(); // Spigot
this.chunkMapDistance.purgeTickets();
+ this.world.getMinecraftServer().midTickLoadChunks(); // Paper
this.tickDistanceManager();
this.world.timings.doChunkMap.stopTiming(); // Spigot
this.world.getMethodProfiler().exitEnter("chunks");
@@ -697,6 +698,7 @@ public class ChunkProviderServer extends IChunkProvider {
@@ -698,6 +699,7 @@ public class ChunkProviderServer extends IChunkProvider {
this.world.timings.doChunkUnload.startTiming(); // Spigot
this.world.getMethodProfiler().exitEnter("unload");
this.playerChunkMap.unloadChunks(booleansupplier);
+ this.world.getMinecraftServer().midTickLoadChunks(); // Paper
this.world.timings.doChunkUnload.stopTiming(); // Spigot
this.world.getMethodProfiler().exit();
this.clearCache();
@@ -755,7 +757,7 @@ public class ChunkProviderServer extends IChunkProvider {
@@ -756,7 +758,7 @@ public class ChunkProviderServer extends IChunkProvider {
entityPlayer.playerNaturallySpawnedEvent.callEvent();
};
// Paper end
Expand All @@ -84,15 +84,15 @@ index 8f849d83d08b39f1cd9184f484a2089a7a3124ef..377d554553ce81f66207541d963f8268
Optional<Chunk> optional = ((Either) playerchunk.b().getNow(PlayerChunk.UNLOADED_CHUNK)).left();

if (optional.isPresent()) {
@@ -838,6 +840,7 @@ public class ChunkProviderServer extends IChunkProvider {
@@ -839,6 +841,7 @@ public class ChunkProviderServer extends IChunkProvider {
this.world.timings.chunkTicks.startTiming(); // Spigot // Paper
this.world.a(chunk, k);
this.world.timings.chunkTicks.stopTiming(); // Spigot // Paper
+ if (chunksTicked[0]++ % 10 == 0) this.world.getMinecraftServer().midTickLoadChunks(); // Paper
}
}
});
@@ -979,6 +982,41 @@ public class ChunkProviderServer extends IChunkProvider {
@@ -980,6 +983,41 @@ public class ChunkProviderServer extends IChunkProvider {
super.executeTask(runnable);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,10 @@ so inline where possible, and avoid the abstraction of the
Either class.

diff --git a/src/main/java/net/minecraft/server/ChunkProviderServer.java b/src/main/java/net/minecraft/server/ChunkProviderServer.java
index 377d554553ce81f66207541d963f826867e66592..9afbec260a1d586152073b2adda32959453ab8c9 100644
index 2ef8506f10426b8a5877e30986c105c0d95be774..722db939971fe395d8250c388fbd7f3b5e87804d 100644
--- a/src/main/java/net/minecraft/server/ChunkProviderServer.java
+++ b/src/main/java/net/minecraft/server/ChunkProviderServer.java
@@ -606,27 +606,37 @@ public class ChunkProviderServer extends IChunkProvider {
@@ -607,27 +607,37 @@ public class ChunkProviderServer extends IChunkProvider {

public final boolean isInEntityTickingChunk(Entity entity) { return this.a(entity); } // Paper - OBFHELPER
@Override public boolean a(Entity entity) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ This successfully fixed a reoccurring and highly reproduceable crash
for heightmaps.

diff --git a/src/main/java/net/minecraft/server/ChunkProviderServer.java b/src/main/java/net/minecraft/server/ChunkProviderServer.java
index 9afbec260a1d586152073b2adda32959453ab8c9..e89683b4f1e3cac60b88a5c7317e525c46950b17 100644
index 722db939971fe395d8250c388fbd7f3b5e87804d..ba99e9949c6fdfc4f49b6b6716100eb51697227d 100644
--- a/src/main/java/net/minecraft/server/ChunkProviderServer.java
+++ b/src/main/java/net/minecraft/server/ChunkProviderServer.java
@@ -1039,6 +1039,7 @@ public class ChunkProviderServer extends IChunkProvider {
@@ -1040,6 +1040,7 @@ public class ChunkProviderServer extends IChunkProvider {
return super.executeNext() || execChunkTask; // Paper
}
} finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,10 +77,10 @@ index 4e0ea454f00c69f03023f01c1d4bd2eda5553a02..353b186060b2c0417a49ab3865ea5972

public String c() {
diff --git a/src/main/java/net/minecraft/server/ChunkProviderServer.java b/src/main/java/net/minecraft/server/ChunkProviderServer.java
index e89683b4f1e3cac60b88a5c7317e525c46950b17..0a99b347d8497f097ef1da6560a5d0adc1374f25 100644
index ba99e9949c6fdfc4f49b6b6716100eb51697227d..7a275bf3260f9fbefc41883c5ebdc1eb2196daf0 100644
--- a/src/main/java/net/minecraft/server/ChunkProviderServer.java
+++ b/src/main/java/net/minecraft/server/ChunkProviderServer.java
@@ -724,6 +724,36 @@ public class ChunkProviderServer extends IChunkProvider {
@@ -725,6 +725,36 @@ public class ChunkProviderServer extends IChunkProvider {
boolean flag1 = this.world.getGameRules().getBoolean(GameRules.DO_MOB_SPAWNING) && !world.getPlayers().isEmpty(); // CraftBukkit

if (!flag) {
Expand Down Expand Up @@ -117,7 +117,7 @@ index e89683b4f1e3cac60b88a5c7317e525c46950b17..0a99b347d8497f097ef1da6560a5d0ad
this.world.getMethodProfiler().enter("pollingChunks");
int k = this.world.getGameRules().getInt(GameRules.RANDOM_TICK_SPEED);
BlockPosition blockposition = this.world.getSpawn();
@@ -758,15 +788,7 @@ public class ChunkProviderServer extends IChunkProvider {
@@ -759,15 +789,7 @@ public class ChunkProviderServer extends IChunkProvider {

this.world.timings.countNaturalMobs.stopTiming(); // Paper - timings
this.world.getMethodProfiler().exit();
Expand All @@ -134,7 +134,7 @@ index e89683b4f1e3cac60b88a5c7317e525c46950b17..0a99b347d8497f097ef1da6560a5d0ad
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();

@@ -780,10 +802,10 @@ public class ChunkProviderServer extends IChunkProvider {
@@ -781,10 +803,10 @@ public class ChunkProviderServer extends IChunkProvider {
this.world.getMethodProfiler().exit();
ChunkCoordIntPair chunkcoordintpair = playerchunk.i();

Expand Down
Loading

0 comments on commit 4d38ee1

Please sign in to comment.