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"

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));

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);
++        Chunk chunk = this.func_186026_b(p_186028_1_, p_186028_2_);
-+        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_);
-+
          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))
+-            if (chunk != null)
-+            {
++            long pos = ChunkPos.func_77272_a(p_186028_1_, p_186028_2_);
-+                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());
-+            }
 +            chunk = net.minecraftforge.common.ForgeChunkManager.fetchDormantChunk(pos, this.field_73251_h);
- 
++            if (chunk != null || !(this.field_73247_e instanceof net.minecraft.world.chunk.storage.AnvilChunkLoader))
-+            if (chunk == null)
-+                chunk = this.func_73239_e(p_186025_1_, p_186025_2_);
-+
-             if (chunk != null)
              {
--                this.field_73244_f.put(ChunkPos.func_77272_a(p_186028_1_, p_186028_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());
-+                this.field_73244_f.put(ChunkPos.func_77272_a(p_186025_1_, p_186025_2_), chunk);
++                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<Long> 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;

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;
+    private static final int BASE_THREADS = 1;
-    static final int PLAYERS_PER_THREAD = 50;
+    private static final int PLAYERS_PER_THREAD = 50;
 
     private static final Map<QueuedChunk, ChunkIOProvider> 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);
+        QueuedChunk key = new QueuedChunk(x, z, world);
-        ChunkIOProvider task = tasks.get(key);
+        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(Math.max(BASE_THREADS, players / PLAYERS_PER_THREAD));
-        pool.setCorePoolSize(size);
     }
 
     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();
         }
     }
-}
+}

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<Runnable> callbacks = new ConcurrentLinkedQueue<Runnable>();
+    private final ConcurrentLinkedQueue<Runnable> callbacks = new ConcurrentLinkedQueue<Runnable>();
     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();
     }
-}
+}

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 World world;
-    final net.minecraft.world.World world;
-    final net.minecraft.world.gen.ChunkProviderServer provider;
 
-    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);

src/main/resources/forge.exc

@@ -22,8 +22,7 @@ net/minecraft/world/biome/Biome.<init>(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.loadChunk(IILjava/lang/Runnable;)Lnet/minecraft/world/chunk/Chunk;=|p_186028_1_,p_186028_2_,runnable
-net/minecraft/world/gen/ChunkProviderServer.originalLoadChunk(II)Lnet/minecraft/world/chunk/Chunk;=|p_186025_1_,p_186025_2_
 
 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