From bf2043b339efd42a0374374d2cb04a8f9e96dc2d Mon Sep 17 00:00:00 2001 From: Spottedleaf Date: Sun, 16 Jun 2019 20:52:34 -0700 Subject: [PATCH] Fix World#isChunkGenerated calls (#2186) * Fix World#isChunkGenerated calls Optimize World#loadChunk() too This patch also adds a chunk status cache on region files (note that its only purpose is to cache the status on DISK) * Ensure correct regionfile usage This also bumps the minimum region file cache to 4 files given readChunkData can load potentially 4 files. * Fix closed check * Better checks for invalid regionfile usage --- .../Configurable-Player-Collision.patch | 4 +- .../Fix-World-isChunkGenerated-calls.patch | 334 ++++++++++++++++++ Spigot-Server-Patches/MC-Dev-fixes.patch | 13 + ...egionFileCache-and-make-configurable.patch | 4 +- 4 files changed, 351 insertions(+), 4 deletions(-) create mode 100644 Spigot-Server-Patches/Fix-World-isChunkGenerated-calls.patch diff --git a/Spigot-Server-Patches/Configurable-Player-Collision.patch b/Spigot-Server-Patches/Configurable-Player-Collision.patch index 37eea3e5af..8a2db3e3c6 100644 --- a/Spigot-Server-Patches/Configurable-Player-Collision.patch +++ b/Spigot-Server-Patches/Configurable-Player-Collision.patch @@ -5,12 +5,12 @@ Subject: [PATCH] Configurable Player Collision diff --git a/src/main/java/com/destroystokyo/paper/PaperConfig.java b/src/main/java/com/destroystokyo/paper/PaperConfig.java -index 4424f7ef18..69bfe62416 100644 +index 0cef1853f5..dc3438890b 100644 --- a/src/main/java/com/destroystokyo/paper/PaperConfig.java +++ b/src/main/java/com/destroystokyo/paper/PaperConfig.java @@ -0,0 +0,0 @@ public class PaperConfig { private static void regionFileCacheSize() { - regionFileCacheSize = getInt("settings.region-file-cache-size", 256); + regionFileCacheSize = Math.max(getInt("settings.region-file-cache-size", 256), 4); } + + public static boolean enablePlayerCollisions = true; diff --git a/Spigot-Server-Patches/Fix-World-isChunkGenerated-calls.patch b/Spigot-Server-Patches/Fix-World-isChunkGenerated-calls.patch new file mode 100644 index 0000000000..05810c71cd --- /dev/null +++ b/Spigot-Server-Patches/Fix-World-isChunkGenerated-calls.patch @@ -0,0 +1,334 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Spottedleaf +Date: Sat, 15 Jun 2019 08:54:33 -0700 +Subject: [PATCH] Fix World#isChunkGenerated calls + +Optimize World#loadChunk() too +This patch also adds a chunk status cache on region files (note that +its only purpose is to cache the status on DISK) + +diff --git a/src/main/java/net/minecraft/server/ChunkProviderServer.java b/src/main/java/net/minecraft/server/ChunkProviderServer.java +index d83eec0d76..459bd619ee 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 { + private final WorldServer world; + private final Thread serverThread; + private final LightEngineThreaded lightEngine; +- private final ChunkProviderServer.a serverThreadQueue; ++ public final ChunkProviderServer.a serverThreadQueue; // Paper private -> public + public final PlayerChunkMap playerChunkMap; + private final WorldPersistentData worldPersistentData; + private long lastTickTime; +@@ -0,0 +0,0 @@ public class ChunkProviderServer extends IChunkProvider { + + return playerChunk.getFullChunk(); + } ++ ++ @Nullable ++ public IChunkAccess getChunkAtImmediately(int x, int z) { ++ if (Thread.currentThread() != this.serverThread) { ++ return CompletableFuture.supplyAsync(() -> { ++ return this.getChunkAtImmediately(x, z); ++ }, this.serverThreadQueue).join(); ++ } ++ ++ long k = ChunkCoordIntPair.pair(x, z); ++ ++ IChunkAccess ichunkaccess; ++ ++ for (int l = 0; l < 4; ++l) { ++ if (k == this.cachePos[l]) { ++ ichunkaccess = this.cacheChunk[l]; ++ if (ichunkaccess != null) { // CraftBukkit - the chunk can become accessible in the meantime TODO for non-null chunks it might also make sense to check that the chunk's state hasn't changed in the meantime ++ return ichunkaccess; ++ } ++ } ++ } ++ ++ PlayerChunk playerChunk = this.getChunk(k); ++ if (playerChunk == null) { ++ return null; ++ } ++ ++ return playerChunk.getAvailableChunkNow(); ++ ++ } + // Paper end + + @Nullable +diff --git a/src/main/java/net/minecraft/server/ChunkRegionLoader.java b/src/main/java/net/minecraft/server/ChunkRegionLoader.java +index 2f749fe26a..4906530d83 100644 +--- a/src/main/java/net/minecraft/server/ChunkRegionLoader.java ++++ b/src/main/java/net/minecraft/server/ChunkRegionLoader.java +@@ -0,0 +0,0 @@ public class ChunkRegionLoader { + return nbttagcompound; + } + ++ // Paper start ++ public static ChunkStatus getStatus(NBTTagCompound compound) { ++ if (compound == null) { ++ return null; ++ } ++ ++ // Note: Copied from below ++ return ChunkStatus.getStatus(compound.getCompound("Level").getString("Status")); ++ } ++ // Paper end ++ + public static ChunkStatus.Type a(@Nullable NBTTagCompound nbttagcompound) { + if (nbttagcompound != null) { + ChunkStatus chunkstatus = ChunkStatus.a(nbttagcompound.getCompound("Level").getString("Status")); +diff --git a/src/main/java/net/minecraft/server/ChunkStatus.java b/src/main/java/net/minecraft/server/ChunkStatus.java +index dd1822d6ff..e324989b46 100644 +--- a/src/main/java/net/minecraft/server/ChunkStatus.java ++++ b/src/main/java/net/minecraft/server/ChunkStatus.java +@@ -0,0 +0,0 @@ public class ChunkStatus { + return this.s; + } + ++ public ChunkStatus getPreviousStatus() { return this.e(); } // Paper - OBFHELPER + public ChunkStatus e() { + return this.u; + } +@@ -0,0 +0,0 @@ public class ChunkStatus { + return this.y; + } + ++ // Paper start ++ public static ChunkStatus getStatus(String name) { ++ try { ++ // We need this otherwise we return EMPTY for invalid names ++ MinecraftKey key = new MinecraftKey(name); ++ return IRegistry.CHUNK_STATUS.getOptional(key).orElse(null); ++ } catch (Exception ex) { ++ return null; // invalid name ++ } ++ } ++ // Paper end + public static ChunkStatus a(String s) { + return (ChunkStatus) IRegistry.CHUNK_STATUS.get(MinecraftKey.a(s)); + } +diff --git a/src/main/java/net/minecraft/server/PlayerChunk.java b/src/main/java/net/minecraft/server/PlayerChunk.java +index 806d225aaa..97040528a0 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 { + Either either = (Either) statusFuture.getNow(null); + return either == null ? null : (Chunk) either.left().orElse(null); + } ++ ++ public IChunkAccess getAvailableChunkNow() { ++ // TODO can we just getStatusFuture(EMPTY)? ++ for (ChunkStatus curr = ChunkStatus.FULL, next = curr.getPreviousStatus(); curr != next; curr = next, next = next.getPreviousStatus()) { ++ CompletableFuture> future = this.getStatusFutureUnchecked(curr); ++ Either either = future.getNow(null); ++ if (either == null || !either.left().isPresent()) { ++ continue; ++ } ++ return either.left().get(); ++ } ++ return null; ++ } + // Paper end + + public CompletableFuture> getStatusFutureUnchecked(ChunkStatus chunkstatus) { +diff --git a/src/main/java/net/minecraft/server/PlayerChunkMap.java b/src/main/java/net/minecraft/server/PlayerChunkMap.java +index 86831c3526..f4bdb2eda3 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 { + } + + @Nullable +- private NBTTagCompound readChunkData(ChunkCoordIntPair chunkcoordintpair) throws IOException { ++ public NBTTagCompound readChunkData(ChunkCoordIntPair chunkcoordintpair) throws IOException { // Paper - private -> public + NBTTagCompound nbttagcompound = this.read(chunkcoordintpair); + +- return nbttagcompound == null ? null : this.getChunkData(this.world.getWorldProvider().getDimensionManager(), this.m, nbttagcompound, chunkcoordintpair, world); // CraftBukkit ++ // Paper start - Cache chunk status on disk ++ if (nbttagcompound == null) { ++ return null; ++ } ++ ++ nbttagcompound = this.getChunkData(this.world.getWorldProvider().getDimensionManager(), this.m, nbttagcompound, chunkcoordintpair, world); // CraftBukkit ++ if (nbttagcompound == null) { ++ return null; ++ } ++ ++ this.getRegionFile(chunkcoordintpair, false).setStatus(chunkcoordintpair.x, chunkcoordintpair.z, ChunkRegionLoader.getStatus(nbttagcompound)); ++ ++ return nbttagcompound; ++ // Paper end + } + + // Spigot Start +diff --git a/src/main/java/net/minecraft/server/RegionFile.java b/src/main/java/net/minecraft/server/RegionFile.java +index 2e14d84657..d610253b95 100644 +--- a/src/main/java/net/minecraft/server/RegionFile.java ++++ b/src/main/java/net/minecraft/server/RegionFile.java +@@ -0,0 +0,0 @@ public class RegionFile implements AutoCloseable { + private final int[] d = new int[1024];private int[] timestamps = d; // Paper - OBFHELPER + private final List e; private List getFreeSectors() { return this.e; } // Paper - OBFHELPER + ++ // Paper start - Cache chunk status ++ private final ChunkStatus[] statuses = new ChunkStatus[32 * 32]; ++ ++ private boolean closed; ++ ++ // invoked on write/read ++ public void setStatus(int x, int z, ChunkStatus status) { ++ if (this.closed) { ++ // We've used an invalid region file. ++ throw new IllegalStateException("RegionFile is closed"); ++ } ++ this.statuses[this.getChunkLocation(new ChunkCoordIntPair(x, z))] = status; ++ } ++ ++ public ChunkStatus getStatusIfCached(int x, int z) { ++ if (this.closed) { ++ // We've used an invalid region file. ++ throw new IllegalStateException("RegionFile is closed"); ++ } ++ final int location = this.getChunkLocation(new ChunkCoordIntPair(x, z)); ++ return this.statuses[location]; ++ } ++ ++ public ChunkStatus getStatus(int x, int z, PlayerChunkMap playerChunkMap) throws IOException { ++ if (this.closed) { ++ // We've used an invalid region file. ++ throw new java.io.EOFException("RegionFile is closed"); ++ } ++ ChunkCoordIntPair chunkPos = new ChunkCoordIntPair(x, z); ++ int location = this.getChunkLocation(chunkPos); ++ ChunkStatus cached = this.statuses[location]; ++ if (cached != null) { ++ return cached; ++ } ++ ++ playerChunkMap.readChunkData(chunkPos); // This will set our status (yes it's disgusting) ++ ++ return this.statuses[location]; ++ } ++ // Paper end ++ + public RegionFile(File file) throws IOException { + this.b = new RandomAccessFile(file, "rw"); + this.file = file; // Spigot // Paper - We need this earlier +@@ -0,0 +0,0 @@ public class RegionFile implements AutoCloseable { + this.writeInt(i); // Paper - Avoid 3 io write calls + } + ++ private final int getChunkLocation(ChunkCoordIntPair chunkcoordintpair) { return this.f(chunkcoordintpair); } // Paper - OBFHELPER + private int f(ChunkCoordIntPair chunkcoordintpair) { + return chunkcoordintpair.j() + chunkcoordintpair.k() * 32; + } +@@ -0,0 +0,0 @@ public class RegionFile implements AutoCloseable { + } + + public void close() throws IOException { ++ this.closed = true; // Paper + this.b.close(); + } + +diff --git a/src/main/java/net/minecraft/server/RegionFileCache.java b/src/main/java/net/minecraft/server/RegionFileCache.java +index 6f34d8aea0..d1323891fa 100644 +--- a/src/main/java/net/minecraft/server/RegionFileCache.java ++++ b/src/main/java/net/minecraft/server/RegionFileCache.java +@@ -0,0 +0,0 @@ public abstract class RegionFileCache implements AutoCloseable { + try { + NBTCompressedStreamTools.writeNBT(nbttagcompound, out); + out.close(); ++ regionfile.setStatus(chunk.x, chunk.z, ChunkRegionLoader.getStatus(nbttagcompound)); // Paper - cache status on disk + regionfile.setOversized(chunkX, chunkZ, false); + } catch (RegionFile.ChunkTooLargeException ignored) { + printOversizedLog("ChunkTooLarge! Someone is trying to duplicate.", regionfile.file, chunkX, chunkZ); +@@ -0,0 +0,0 @@ public abstract class RegionFileCache implements AutoCloseable { + if (SIZE_THRESHOLD == OVERZEALOUS_THRESHOLD) { + resetFilterThresholds(); + } ++ regionfile.setStatus(chunk.x, chunk.z, ChunkRegionLoader.getStatus(nbttagcompound)); // Paper - cache status on disk + } catch (RegionFile.ChunkTooLargeException e) { + printOversizedLog("ChunkTooLarge even after reduction. Trying in overzealous mode.", regionfile.file, chunkX, chunkZ); + // Eek, major fail. We have retry logic, so reduce threshholds and fall back +diff --git a/src/main/java/org/bukkit/craftbukkit/CraftWorld.java b/src/main/java/org/bukkit/craftbukkit/CraftWorld.java +index 0cdf723480..4026edabc2 100644 +--- a/src/main/java/org/bukkit/craftbukkit/CraftWorld.java ++++ b/src/main/java/org/bukkit/craftbukkit/CraftWorld.java +@@ -0,0 +0,0 @@ public class CraftWorld implements World { + + @Override + public boolean isChunkGenerated(int x, int z) { ++ // Paper start - Fix this method ++ if (!Bukkit.isPrimaryThread()) { ++ return CompletableFuture.supplyAsync(() -> { ++ return CraftWorld.this.isChunkGenerated(x, z); ++ }, world.getChunkProvider().serverThreadQueue).join(); ++ } ++ IChunkAccess chunk = world.getChunkProvider().getChunkAtImmediately(x, z); ++ if (chunk != null) { ++ return chunk instanceof ProtoChunkExtension || chunk instanceof net.minecraft.server.Chunk; ++ } + try { +- return world.getChunkProvider().getChunkAtIfCachedImmediately(x, z) != null || world.getChunkProvider().playerChunkMap.chunkExists(new ChunkCoordIntPair(x, z)); // Paper ++ return world.getChunkProvider().playerChunkMap.getRegionFile(new ChunkCoordIntPair(x, z), false) ++ .getStatus(x, z, world.getChunkProvider().playerChunkMap) == ChunkStatus.FULL; ++ // Paper end + } catch (IOException ex) { + throw new RuntimeException(ex); + } +@@ -0,0 +0,0 @@ public class CraftWorld implements World { + @Override + public boolean loadChunk(int x, int z, boolean generate) { + org.spigotmc.AsyncCatcher.catchOp( "chunk load"); // Spigot +- IChunkAccess chunk = world.getChunkProvider().getChunkAt(x, z, generate || isChunkGenerated(x, z) ? ChunkStatus.FULL : ChunkStatus.EMPTY, true); +- +- // If generate = false, but the chunk already exists, we will get this back. +- if (chunk instanceof ProtoChunkExtension) { +- // We then cycle through again to get the full chunk immediately, rather than after the ticket addition +- chunk = world.getChunkProvider().getChunkAt(x, z, ChunkStatus.FULL, true); +- } +- +- if (chunk instanceof net.minecraft.server.Chunk) { +- world.getChunkProvider().addTicket(TicketType.PLUGIN, new ChunkCoordIntPair(x, z), 1, Unit.INSTANCE); +- return true; ++ // Paper - Optimize this method ++ if (!generate && !isChunkGenerated(x, z)) { ++ return false; + } +- +- return false; ++ world.getChunkProvider().addTicket(TicketType.PLUGIN, new ChunkCoordIntPair(x, z), 1, Unit.INSTANCE); ++ world.getChunkProvider().getChunkAt(x, z, ChunkStatus.FULL, true); ++ return true; ++ // Paper end + } + + @Override +@@ -0,0 +0,0 @@ public class CraftWorld implements World { + + // Paper start + private Chunk getChunkAtGen(int x, int z, boolean gen) { +- // copied from loadChunk() +- // this function is identical except we do not add a plugin ticket +- IChunkAccess chunk = world.getChunkProvider().getChunkAt(x, z, gen || isChunkGenerated(x, z) ? ChunkStatus.FULL : ChunkStatus.EMPTY, true); +- +- // If generate = false, but the chunk already exists, we will get this back. +- if (chunk instanceof ProtoChunkExtension) { +- // We then cycle through again to get the full chunk immediately, rather than after the ticket addition +- chunk = world.getChunkProvider().getChunkAt(x, z, ChunkStatus.FULL, true); +- } +- +- if (chunk instanceof net.minecraft.server.Chunk) { +- return ((net.minecraft.server.Chunk)chunk).bukkitChunk; ++ // Note: Copied from loadChunk() ++ if (!gen && !isChunkGenerated(x, z)) { ++ return null; + } +- +- return null; ++ return ((net.minecraft.server.Chunk)world.getChunkProvider().getChunkAt(x, z, ChunkStatus.FULL, true)).bukkitChunk; + } + + @Override +-- \ No newline at end of file diff --git a/Spigot-Server-Patches/MC-Dev-fixes.patch b/Spigot-Server-Patches/MC-Dev-fixes.patch index 2d10653961..65eee9df8a 100644 --- a/Spigot-Server-Patches/MC-Dev-fixes.patch +++ b/Spigot-Server-Patches/MC-Dev-fixes.patch @@ -160,6 +160,19 @@ index c973ab6076..30701fd7f3 100644 + return this.blockIds.a(iblockdata); // Paper - decompile fix } } +diff --git a/src/main/java/net/minecraft/server/ChunkStatus.java b/src/main/java/net/minecraft/server/ChunkStatus.java +index 26db8f135f..dd1822d6ff 100644 +--- a/src/main/java/net/minecraft/server/ChunkStatus.java ++++ b/src/main/java/net/minecraft/server/ChunkStatus.java +@@ -0,0 +0,0 @@ public class ChunkStatus { + return (CompletableFuture) function.apply(ichunkaccess); + }); + private static final List q = ImmutableList.of(ChunkStatus.FULL, ChunkStatus.FEATURES, ChunkStatus.LIQUID_CARVERS, ChunkStatus.STRUCTURE_STARTS, ChunkStatus.STRUCTURE_STARTS, ChunkStatus.STRUCTURE_STARTS, ChunkStatus.STRUCTURE_STARTS, ChunkStatus.STRUCTURE_STARTS, ChunkStatus.STRUCTURE_STARTS, ChunkStatus.STRUCTURE_STARTS, ChunkStatus.STRUCTURE_STARTS); +- private static final IntList r = (IntList) SystemUtils.a((Object) (new IntArrayList(a().size())), (intarraylist) -> { ++ private static final IntList r = (IntList) SystemUtils.a((new IntArrayList(a().size())), (java.util.function.Consumer)(IntArrayList intarraylist) -> { // Paper - decompile fix + int i = 0; + + for (int j = a().size() - 1; j >= 0; --j) { diff --git a/src/main/java/net/minecraft/server/EntityFox.java b/src/main/java/net/minecraft/server/EntityFox.java index a79262631c..5184e52626 100644 --- a/src/main/java/net/minecraft/server/EntityFox.java diff --git a/Spigot-Server-Patches/Sanitise-RegionFileCache-and-make-configurable.patch b/Spigot-Server-Patches/Sanitise-RegionFileCache-and-make-configurable.patch index 752af18cbb..793c4d8516 100644 --- a/Spigot-Server-Patches/Sanitise-RegionFileCache-and-make-configurable.patch +++ b/Spigot-Server-Patches/Sanitise-RegionFileCache-and-make-configurable.patch @@ -11,7 +11,7 @@ The implementation uses a LinkedHashMap as an LRU cache (modified from HashMap). The maximum size of the RegionFileCache is also made configurable. diff --git a/src/main/java/com/destroystokyo/paper/PaperConfig.java b/src/main/java/com/destroystokyo/paper/PaperConfig.java -index 6cc99ffe43..4424f7ef18 100644 +index 6cc99ffe43..0cef1853f5 100644 --- a/src/main/java/com/destroystokyo/paper/PaperConfig.java +++ b/src/main/java/com/destroystokyo/paper/PaperConfig.java @@ -0,0 +0,0 @@ public class PaperConfig { @@ -21,7 +21,7 @@ index 6cc99ffe43..4424f7ef18 100644 + + public static int regionFileCacheSize = 256; + private static void regionFileCacheSize() { -+ regionFileCacheSize = getInt("settings.region-file-cache-size", 256); ++ regionFileCacheSize = Math.max(getInt("settings.region-file-cache-size", 256), 4); + } } diff --git a/src/main/java/net/minecraft/server/RegionFileCache.java b/src/main/java/net/minecraft/server/RegionFileCache.java