From 279380b4f1aa02a4df5e2b323c11ca6ab1531431 Mon Sep 17 00:00:00 2001 From: Minecrell Date: Sat, 4 Jun 2016 11:51:27 +0200 Subject: [PATCH] Fix asynchronous chunk loading (#2946) Since the update to Minecraft 1.9.4 chunks were actually never loaded asynchronously because a sync request was always made from the PlayerChunkMap shortly after the chunk had been queued. - PlayerChunkMapEntry now only loads chunks synchronously *after* the chunk failed to load asynchronously. - Fixed some minor bugs that caused "Attempted to dequeue chunk" messages - Simplified ChunkProviderServer patch. loadChunk no longer generates chunks, so there is no need to handle that. - Moved loader and provider to ChunkIOProvider so there is no need for "hashCode abuse" --- .../management/PlayerChunkMapEntry.java.patch | 26 +++-- .../world/gen/ChunkProviderServer.java.patch | 96 +++++++------------ .../common/chunkio/ChunkIOExecutor.java | 36 +++---- .../common/chunkio/ChunkIOProvider.java | 30 +++--- .../common/chunkio/QueuedChunk.java | 10 +- src/main/resources/forge.exc | 3 +- 6 files changed, 93 insertions(+), 108 deletions(-) diff --git a/patches/minecraft/net/minecraft/server/management/PlayerChunkMapEntry.java.patch b/patches/minecraft/net/minecraft/server/management/PlayerChunkMapEntry.java.patch index 443d0d914..1b4dd9bdf 100644 --- a/patches/minecraft/net/minecraft/server/management/PlayerChunkMapEntry.java.patch +++ b/patches/minecraft/net/minecraft/server/management/PlayerChunkMapEntry.java.patch @@ -1,6 +1,6 @@ --- ../src-base/minecraft/net/minecraft/server/management/PlayerChunkMapEntry.java +++ ../src-work/minecraft/net/minecraft/server/management/PlayerChunkMapEntry.java -@@ -32,12 +32,19 @@ +@@ -32,12 +32,21 @@ private int field_187288_h; private long field_187289_i; private boolean field_187290_j; @@ -9,8 +9,10 @@ + public void run() + { + PlayerChunkMapEntry.this.field_187286_f = PlayerChunkMapEntry.this.field_187282_b.func_72688_a().func_72863_F().func_186028_c(PlayerChunkMapEntry.this.field_187284_d.field_77276_a, PlayerChunkMapEntry.this.field_187284_d.field_77275_b); ++ PlayerChunkMapEntry.this.loading = false; + } + }; ++ private boolean loading = true; public PlayerChunkMapEntry(PlayerChunkMap p_i1518_1_, int p_i1518_2_, int p_i1518_3_) { @@ -21,7 +23,7 @@ } public ChunkPos func_187264_a() -@@ -71,6 +78,20 @@ +@@ -71,6 +80,20 @@ { if (this.field_187283_c.contains(p_187277_1_)) { @@ -32,7 +34,7 @@ + + if (this.field_187283_c.isEmpty()) + { -+ net.minecraftforge.common.chunkio.ChunkIOExecutor.dropQueuedChunkLoad(this.field_187282_b.func_72688_a(), this.field_187284_d.field_77276_a, this.field_187284_d.field_77275_b, this.loadedRunnable); ++ if (this.loading) net.minecraftforge.common.chunkio.ChunkIOExecutor.dropQueuedChunkLoad(this.field_187282_b.func_72688_a(), this.field_187284_d.field_77276_a, this.field_187284_d.field_77275_b, this.loadedRunnable); + this.field_187282_b.func_187305_b(this); + } + @@ -42,7 +44,7 @@ if (this.field_187290_j) { p_187277_1_.field_71135_a.func_147359_a(new SPacketUnloadChunk(this.field_187284_d.field_77276_a, this.field_187284_d.field_77275_b)); -@@ -78,6 +99,8 @@ +@@ -78,6 +101,8 @@ this.field_187283_c.remove(p_187277_1_); @@ -51,7 +53,15 @@ if (this.field_187283_c.isEmpty()) { this.field_187282_b.func_187305_b(this); -@@ -167,7 +190,7 @@ +@@ -87,6 +112,7 @@ + + public boolean func_187268_a(boolean p_187268_1_) + { ++ if (this.loading) return false; + if (this.field_187286_f != null) + { + return true; +@@ -167,7 +193,7 @@ this.field_187288_h |= 1 << (p_187265_2_ >> 4); @@ -60,7 +70,7 @@ { short short1 = (short)(p_187265_1_ << 12 | p_187265_3_ << 8 | p_187265_2_); -@@ -178,7 +201,8 @@ +@@ -178,7 +204,8 @@ return; } } @@ -70,7 +80,7 @@ this.field_187285_e[this.field_187287_g++] = short1; } } -@@ -195,6 +219,7 @@ +@@ -195,6 +222,7 @@ } } @@ -78,7 +88,7 @@ public void func_187280_d() { if (this.field_187290_j && this.field_187286_f != null) -@@ -208,28 +233,32 @@ +@@ -208,28 +236,32 @@ int k = (this.field_187285_e[0] >> 8 & 15) + this.field_187284_d.field_77275_b * 16; BlockPos blockpos = new BlockPos(i, j, k); this.func_187267_a(new SPacketBlockChange(this.field_187282_b.func_72688_a(), blockpos)); diff --git a/patches/minecraft/net/minecraft/world/gen/ChunkProviderServer.java.patch b/patches/minecraft/net/minecraft/world/gen/ChunkProviderServer.java.patch index 4464d5966..e063f01b7 100644 --- a/patches/minecraft/net/minecraft/world/gen/ChunkProviderServer.java.patch +++ b/patches/minecraft/net/minecraft/world/gen/ChunkProviderServer.java.patch @@ -8,7 +8,7 @@ public ChunkProviderServer(WorldServer p_i46838_1_, IChunkLoader p_i46838_2_, IChunkGenerator p_i46838_3_) { -@@ -82,18 +83,73 @@ +@@ -82,20 +83,50 @@ @Nullable public Chunk func_186028_c(int p_186028_1_, int p_186028_2_) { @@ -17,75 +17,53 @@ + } + @Nullable -+ public Chunk loadChunk(int X, int Z, Runnable runnable) ++ public Chunk loadChunk(int p_186028_1_, int p_186028_2_, Runnable runnable) + { -+ long pos = ChunkPos.func_77272_a(X, Z); -+ this.field_73248_b.remove(Long.valueOf(pos)); -+ Chunk chunk = this.field_73244_f.get(pos); -+ net.minecraft.world.chunk.storage.AnvilChunkLoader loader = null; -+ -+ if (this.field_73247_e instanceof net.minecraft.world.chunk.storage.AnvilChunkLoader) -+ { -+ loader = (net.minecraft.world.chunk.storage.AnvilChunkLoader) this.field_73247_e; -+ } -+ -+ // We can only use the queue for already generated chunks -+ if (chunk == null && loader != null && loader.chunkExists(this.field_73251_h, X, Z)) -+ { -+ if (runnable != null) -+ { -+ net.minecraftforge.common.chunkio.ChunkIOExecutor.queueChunkLoad(this.field_73251_h, loader, this, X, Z, runnable); -+ return null; -+ } -+ else -+ { -+ chunk = net.minecraftforge.common.chunkio.ChunkIOExecutor.syncChunkLoad(this.field_73251_h, loader, this, X, Z); -+ } -+ } -+ else if (chunk == null) -+ { -+ chunk = this.originalLoadChunk(X, Z); -+ } -+ -+ // If we didn't load the chunk async and have a callback run it now -+ if (runnable != null) -+ { -+ runnable.run(); -+ } -+ -+ return chunk; -+ } -+ -+ public Chunk originalLoadChunk(int p_186025_1_, int p_186025_2_) -+ { -+ Chunk chunk = this.func_186026_b(p_186025_1_, p_186025_2_); -+ ++ Chunk chunk = this.func_186026_b(p_186028_1_, p_186028_2_); if (chunk == null) { - chunk = this.func_73239_e(p_186028_1_, p_186028_2_); -+ long pos = ChunkPos.func_77272_a(p_186025_1_, p_186025_2_); -+ if (!loadingChunks.add(pos)) -+ { -+ net.minecraftforge.fml.common.FMLLog.bigWarning("There is an attempt to load a chunk (%d,%d) in di >mension %d that is already being loaded. This will cause weird chunk breakages.", p_186025_1_, p_186025_2_, this.field_73251_h.field_73011_w.getDimension()); -+ } +- +- if (chunk != null) ++ long pos = ChunkPos.func_77272_a(p_186028_1_, p_186028_2_); + chunk = net.minecraftforge.common.ForgeChunkManager.fetchDormantChunk(pos, this.field_73251_h); - -+ if (chunk == null) -+ chunk = this.func_73239_e(p_186025_1_, p_186025_2_); -+ - if (chunk != null) ++ if (chunk != null || !(this.field_73247_e instanceof net.minecraft.world.chunk.storage.AnvilChunkLoader)) { -- this.field_73244_f.put(ChunkPos.func_77272_a(p_186028_1_, p_186028_2_), chunk); -+ this.field_73244_f.put(ChunkPos.func_77272_a(p_186025_1_, p_186025_2_), chunk); ++ if (!loadingChunks.add(pos)) net.minecraftforge.fml.common.FMLLog.bigWarning("There is an attempt to load a chunk (%d,%d) in dimension %d that is already being loaded. This will cause weird chunk breakages.", p_186028_1_, p_186028_2_, this.field_73251_h.field_73011_w.getDimension()); ++ if (chunk == null) chunk = this.func_73239_e(p_186028_1_, p_186028_2_); ++ ++ if (chunk != null) ++ { + this.field_73244_f.put(ChunkPos.func_77272_a(p_186028_1_, p_186028_2_), chunk); chunk.func_76631_c(); chunk.func_186030_a(this, this.field_186029_c); - } ++ } + -+ loadingChunks.remove(pos); ++ loadingChunks.remove(pos); + } ++ else ++ { ++ net.minecraft.world.chunk.storage.AnvilChunkLoader loader = (net.minecraft.world.chunk.storage.AnvilChunkLoader) this.field_73247_e; ++ ++ // We can only use the queue for already generated chunks ++ if (loader.chunkExists(this.field_73251_h, p_186028_1_, p_186028_2_)) ++ { ++ if (runnable != null) ++ { ++ net.minecraftforge.common.chunkio.ChunkIOExecutor.queueChunkLoad(this.field_73251_h, loader, this, p_186028_1_, p_186028_2_, runnable); ++ return null; ++ } ++ else chunk = net.minecraftforge.common.chunkio.ChunkIOExecutor.syncChunkLoad(this.field_73251_h, loader, this, p_186028_1_, p_186028_2_); ++ } ++ } } ++ // If we didn't load the chunk async and have a callback run it now ++ if (runnable != null) runnable.run(); return chunk; -@@ -221,6 +277,11 @@ + } + +@@ -221,6 +252,11 @@ { if (!this.field_73248_b.isEmpty()) { @@ -97,7 +75,7 @@ Iterator iterator = this.field_73248_b.iterator(); for (int i = 0; i < 100 && iterator.hasNext(); iterator.remove()) -@@ -235,6 +296,11 @@ +@@ -235,6 +271,11 @@ this.func_73243_a(chunk); this.field_73244_f.remove(olong); ++i; diff --git a/src/main/java/net/minecraftforge/common/chunkio/ChunkIOExecutor.java b/src/main/java/net/minecraftforge/common/chunkio/ChunkIOExecutor.java index cc710132d..6e8a9727f 100644 --- a/src/main/java/net/minecraftforge/common/chunkio/ChunkIOExecutor.java +++ b/src/main/java/net/minecraftforge/common/chunkio/ChunkIOExecutor.java @@ -2,7 +2,6 @@ package net.minecraftforge.common.chunkio; import java.util.Iterator; import java.util.Map; -import java.util.concurrent.Executors; import java.util.concurrent.LinkedBlockingQueue; import java.util.concurrent.ThreadFactory; import java.util.concurrent.ThreadPoolExecutor; @@ -19,8 +18,8 @@ import net.minecraftforge.fml.common.FMLLog; public class ChunkIOExecutor { - static final int BASE_THREADS = 1; - static final int PLAYERS_PER_THREAD = 50; + private static final int BASE_THREADS = 1; + private static final int PLAYERS_PER_THREAD = 50; private static final Map tasks = Maps.newConcurrentMap(); private static final ThreadPoolExecutor pool = new ThreadPoolExecutor(BASE_THREADS, Integer.MAX_VALUE, 60L, TimeUnit.SECONDS, @@ -41,8 +40,8 @@ public class ChunkIOExecutor //Load the chunk completely in this thread. Dequeue as needed... public static Chunk syncChunkLoad(World world, AnvilChunkLoader loader, ChunkProviderServer provider, int x, int z) { - QueuedChunk key = new QueuedChunk(x, z, loader, world, provider); - ChunkIOProvider task = tasks.get(key); + QueuedChunk key = new QueuedChunk(x, z, world); + ChunkIOProvider task = tasks.remove(key); // Remove task because we will call the sync callbacks directly if (task != null) { if (!pool.remove(task)) // If it wasn't in the pool, and run hasn't finished, then wait for the async thread. @@ -62,10 +61,15 @@ public class ChunkIOExecutor } } } + else + { + // If the task was not run yet we still need to load the chunk + task.run(); + } } else { - task = new ChunkIOProvider(key); + task = new ChunkIOProvider(key, loader, provider); task.run(); } task.syncCallback(); @@ -75,11 +79,11 @@ public class ChunkIOExecutor //Queue the chunk to be loaded, and call the runnable when finished public static void queueChunkLoad(World world, AnvilChunkLoader loader, ChunkProviderServer provider, int x, int z, Runnable runnable) { - QueuedChunk key = new QueuedChunk(x, z, loader, world, provider); + QueuedChunk key = new QueuedChunk(x, z, world); ChunkIOProvider task = tasks.get(key); if (task == null) { - task = new ChunkIOProvider(key); + task = new ChunkIOProvider(key, loader, provider); task.addCallback(runnable); // Add before calling execute for thread safety tasks.put(key, task); pool.execute(task); @@ -90,11 +94,10 @@ public class ChunkIOExecutor } } - // Abuses the fact that hashCode and equals for QueuedChunk only use world and coords // Remove the chunk from the queue if it's in the list. public static void dropQueuedChunkLoad(World world, int x, int z, Runnable runnable) { - QueuedChunk key = new QueuedChunk(x, z, null, world, null); + QueuedChunk key = new QueuedChunk(x, z, world); ChunkIOProvider task = tasks.get(key); if (task == null) { @@ -113,8 +116,7 @@ public class ChunkIOExecutor public static void adjustPoolSize(int players) { - int size = Math.max(BASE_THREADS, (int) Math.ceil(players / PLAYERS_PER_THREAD)); - pool.setCorePoolSize(size); + pool.setCorePoolSize(Math.max(BASE_THREADS, players / PLAYERS_PER_THREAD)); } public static void tick() @@ -123,11 +125,13 @@ public class ChunkIOExecutor while (itr.hasNext()) { ChunkIOProvider task = itr.next(); - if (task.runFinished() && task.hasCallback()) + if (task.runFinished()) { - task.syncCallback(); + if (task.hasCallback()) + task.syncCallback(); + + itr.remove(); } - itr.remove(); } } -} \ No newline at end of file +} diff --git a/src/main/java/net/minecraftforge/common/chunkio/ChunkIOProvider.java b/src/main/java/net/minecraftforge/common/chunkio/ChunkIOProvider.java index 741bdfe3e..2fb71dd1c 100644 --- a/src/main/java/net/minecraftforge/common/chunkio/ChunkIOProvider.java +++ b/src/main/java/net/minecraftforge/common/chunkio/ChunkIOProvider.java @@ -13,15 +13,20 @@ import java.util.concurrent.ConcurrentLinkedQueue; class ChunkIOProvider implements Runnable { - private QueuedChunk chunkInfo; + private final QueuedChunk chunkInfo; + private final AnvilChunkLoader loader; + private final ChunkProviderServer provider; + private Chunk chunk; private NBTTagCompound nbt; - private ConcurrentLinkedQueue callbacks = new ConcurrentLinkedQueue(); + private final ConcurrentLinkedQueue callbacks = new ConcurrentLinkedQueue(); private boolean ran = false; - ChunkIOProvider(QueuedChunk chunk) + ChunkIOProvider(QueuedChunk chunk, AnvilChunkLoader loader, ChunkProviderServer provider) { this.chunkInfo = chunk; + this.loader = loader; + this.provider = provider; } public void addCallback(Runnable callback) @@ -38,11 +43,10 @@ class ChunkIOProvider implements Runnable { synchronized(this) { - AnvilChunkLoader loader = chunkInfo.loader; Object[] data = null; try { - data = loader.loadChunk__Async(chunkInfo.world, chunkInfo.x, chunkInfo.z); + data = this.loader.loadChunk__Async(chunkInfo.world, chunkInfo.x, chunkInfo.z); } catch (IOException e) { @@ -63,30 +67,24 @@ class ChunkIOProvider implements Runnable // sync stuff public void syncCallback() { - ChunkProviderServer provider = this.chunkInfo.provider; if (chunk == null) { - // If the chunk loading failed just do it synchronously (may generate) - this.chunk = provider.originalLoadChunk(this.chunkInfo.x, this.chunkInfo.z); this.runCallbacks(); return; } // Load Entities - this.chunkInfo.loader.loadEntities(this.chunkInfo.world, this.nbt.getCompoundTag("Level"), this.chunk); + this.loader.loadEntities(this.chunkInfo.world, this.nbt.getCompoundTag("Level"), this.chunk); MinecraftForge.EVENT_BUS.post(new ChunkDataEvent.Load(this.chunk, this.nbt)); // Don't call ChunkDataEvent.Load async this.chunk.setLastSaveTime(provider.worldObj.getTotalWorldTime()); + this.provider.chunkGenerator.recreateStructures(this.chunk, this.chunkInfo.x, this.chunkInfo.z); + provider.id2ChunkMap.put(ChunkPos.chunkXZ2Int(this.chunkInfo.x, this.chunkInfo.z), this.chunk); this.chunk.onChunkLoad(); - - if (provider.chunkGenerator != null) - { - provider.chunkGenerator.recreateStructures(this.chunk, this.chunkInfo.x, this.chunkInfo.z); - } - this.chunk.populateChunk(provider, provider.chunkGenerator); + this.runCallbacks(); } @@ -114,4 +112,4 @@ class ChunkIOProvider implements Runnable this.callbacks.clear(); } -} \ No newline at end of file +} diff --git a/src/main/java/net/minecraftforge/common/chunkio/QueuedChunk.java b/src/main/java/net/minecraftforge/common/chunkio/QueuedChunk.java index d813242e6..b914de6e6 100644 --- a/src/main/java/net/minecraftforge/common/chunkio/QueuedChunk.java +++ b/src/main/java/net/minecraftforge/common/chunkio/QueuedChunk.java @@ -1,19 +1,16 @@ package net.minecraftforge.common.chunkio; +import net.minecraft.world.World; class QueuedChunk { final int x; final int z; - final net.minecraft.world.chunk.storage.AnvilChunkLoader loader; - final net.minecraft.world.World world; - final net.minecraft.world.gen.ChunkProviderServer provider; + final World world; - public QueuedChunk(int x, int z, net.minecraft.world.chunk.storage.AnvilChunkLoader loader, net.minecraft.world.World world, net.minecraft.world.gen.ChunkProviderServer provider) { + public QueuedChunk(int x, int z, World world) { this.x = x; this.z = z; - this.loader = loader; this.world = world; - this.provider = provider; } @Override @@ -40,7 +37,6 @@ class QueuedChunk { result.append(this.getClass().getName() + " {" + NEW_LINE); result.append(" x: " + x + NEW_LINE); result.append(" z: " + z + NEW_LINE); - result.append(" loader: " + loader + NEW_LINE ); result.append(" world: " + world.getWorldInfo().getWorldName() + NEW_LINE); result.append(" dimension: " + world.provider.getDimension() + NEW_LINE); result.append(" provider: " + world.provider.getClass().getName() + NEW_LINE); diff --git a/src/main/resources/forge.exc b/src/main/resources/forge.exc index f715ac5f3..5dd6d2365 100644 --- a/src/main/resources/forge.exc +++ b/src/main/resources/forge.exc @@ -22,8 +22,7 @@ net/minecraft/world/biome/Biome.(IZ)V=|p_i1971_1_,register net/minecraft/world/chunk/storage/AnvilChunkLoader.loadChunk__Async(Lnet/minecraft/world/World;II)[Ljava/lang/Object;=|p_75815_1_,p_75815_2_,p_75815_3_ net/minecraft/world/chunk/storage/AnvilChunkLoader.checkedReadChunkFromNBT__Async(Lnet/minecraft/world/World;IILnet/minecraft/nbt/NBTTagCompound;)[Ljava/lang/Object;=|p_75822_1_,p_75822_2_,p_75822_3_,p_75822_4_ net/minecraft/world/chunk/storage/AnvilChunkLoader.loadEntities(Lnet/minecraft/world/World;Lnet/minecraft/nbt/NBTTagCompound;Lnet/minecraft/world/chunk/Chunk;)V=|p_75823_1_,p_75823_2_,chunk -net/minecraft/world/gen/ChunkProviderServer.loadChunk(II;Ljava/lang/Runnable;)Lnet/minecraft/world/chunk/Chunk;=|p_186025_1_,p_186025_2_,runnable -net/minecraft/world/gen/ChunkProviderServer.originalLoadChunk(II)Lnet/minecraft/world/chunk/Chunk;=|p_186025_1_,p_186025_2_ +net/minecraft/world/gen/ChunkProviderServer.loadChunk(IILjava/lang/Runnable;)Lnet/minecraft/world/chunk/Chunk;=|p_186028_1_,p_186028_2_,runnable net/minecraft/block/BlockFire.tryCatchFire(Lnet/minecraft/world/World;Lnet/minecraft/util/math/BlockPos;ILjava/util/Random;ILnet/minecraft/util/EnumFacing;)V=|p_176536_1_,p_176536_2_,p_176536_3_,p_176536_4_,p_176536_5_,face net/minecraft/block/BlockSkull.getDrops(Lnet/minecraft/world/IBlockAccess;Lnet/minecraft/util/math/BlockPos;Lnet/minecraft/block/state/IBlockState;I)Ljava/util/List;=|p_180663_1_,p_180663_2_,p_180663_3_,fortune