Refactor Lighting Queue System

may help #284

Cleans up the lighting queue system, reducing diff and improving implementation.

We no longer stop chunk unloads due to lighting updates, and instead simply flush the lighting queue.
The cost of forcing the chunk (and its neighbors!) to stay loaded waiting for its
lighting work to finish is much greater than simply taking the hit and doing the work.

This change also helps reduce the diff and avoid bugs with missed diffs by removing
duplicated logic.

Also switches to a more effecient data structure (ArrayDeque instead of LinkedList) for the queue itself.

Spigot-Server-Patches/Avoid-hopper-searches-if-there-are-no-items.patch

@@ -18,8 +18,8 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000
 --- a/src/main/java/net/minecraft/server/Chunk.java
 +++ b/src/main/java/net/minecraft/server/Chunk.java
 @@ -0,0 +0,0 @@ public class Chunk {
+     public boolean d;
      protected gnu.trove.map.hash.TObjectIntHashMap<Class> entityCount = new gnu.trove.map.hash.TObjectIntHashMap<Class>(); // Spigot
-     public int lightUpdates; // Paper - Number of queued light updates for this chunk
  
 +    // Paper start
 +    // Track the number of minecarts and items

Spigot-Server-Patches/Lighting-Queue.patch

@@ -3,19 +3,29 @@ From: Byteflux <byte@byteflux.net>
 Date: Wed, 2 Mar 2016 00:52:31 -0600
 Subject: [PATCH] Lighting Queue
 
+This provides option to queue lighting updates to ensure they do not cause the server lag
 
-diff --git a/src/main/java/co/aikar/timings/MinecraftTimings.java b/src/main/java/co/aikar/timings/MinecraftTimings.java
+diff --git a/src/main/java/co/aikar/timings/WorldTimingsHandler.java b/src/main/java/co/aikar/timings/WorldTimingsHandler.java
 index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644
---- a/src/main/java/co/aikar/timings/MinecraftTimings.java
+--- a/src/main/java/co/aikar/timings/WorldTimingsHandler.java
-+++ b/src/main/java/co/aikar/timings/MinecraftTimings.java
++++ b/src/main/java/co/aikar/timings/WorldTimingsHandler.java
-@@ -0,0 +0,0 @@ public final class MinecraftTimings {
+@@ -0,0 +0,0 @@ public class WorldTimingsHandler {
-     public static final Timing timeUpdateTimer = Timings.ofSafe("Time Update");
+     public final Timing syncChunkLoadTileTicksTimer;
-     public static final Timing serverCommandTimer = Timings.ofSafe("Server Command");
+     public final Timing syncChunkLoadPostTimer;
-     public static final Timing worldSaveTimer = Timings.ofSafe("World Save");
-+    public static final Timing lightingQueueTimer = Timings.ofSafe("Lighting Queue");
  
-     public static final Timing tickEntityTimer = Timings.ofSafe("## tickEntity");
++    public final Timing lightingQueueTimer;
-     public static final Timing tickTileEntityTimer = Timings.ofSafe("## tickTileEntity");
++
+     public WorldTimingsHandler(World server) {
+         String name = server.worldData.getName() +" - ";
+ 
+@@ -0,0 +0,0 @@ public class WorldTimingsHandler {
+         tracker2 = Timings.ofSafe(name + "tracker stage 2");
+         doTick = Timings.ofSafe(name + "doTick");
+         tickEntities = Timings.ofSafe(name + "tickEntities");
++
++        lightingQueueTimer = Timings.ofSafe(name + "Lighting Queue");
+     }
+ }
 diff --git a/src/main/java/com/destroystokyo/paper/PaperWorldConfig.java b/src/main/java/com/destroystokyo/paper/PaperWorldConfig.java
 index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644
 --- a/src/main/java/com/destroystokyo/paper/PaperWorldConfig.java
@@ -36,27 +46,18 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000
 --- a/src/main/java/net/minecraft/server/Chunk.java
 +++ b/src/main/java/net/minecraft/server/Chunk.java
 @@ -0,0 +0,0 @@ public class Chunk {
-     private ConcurrentLinkedQueue<BlockPosition> y;
+     private boolean m;
-     public boolean d;
+     public final Map<BlockPosition, TileEntity> tileEntities;
-     protected gnu.trove.map.hash.TObjectIntHashMap<Class> entityCount = new gnu.trove.map.hash.TObjectIntHashMap<Class>(); // Spigot
+     public final List<Entity>[] entitySlices; // Spigot
-+    public int lightUpdates; // Paper - Number of queued light updates for this chunk
++    final PaperLightingQueue.LightingQueue lightingQueue = new PaperLightingQueue.LightingQueue(this); // Paper
- 
+     private boolean done;
-     // CraftBukkit start - Neighbor loaded cache for chunk lighting and entity ticking
+     private boolean lit;
-     private int neighbors = 0x1 << 12;
+     private boolean r;
 @@ -0,0 +0,0 @@ public class Chunk {
      private void h(boolean flag) {
          this.world.methodProfiler.a("recheckGaps");
          if (this.world.areChunksLoaded(new BlockPosition(this.locX * 16 + 8, 0, this.locZ * 16 + 8), 16)) {
-+            // Paper start - Queue light update
++            lightingQueue.add(() -> recheckGaps(flag)); // Paper - Queue light update
-+            if (!world.paperConfig.queueLightUpdates) {
-+                recheckGaps(flag);
-+            } else {
-+                ++lightUpdates;
-+                world.getServer().getServer().lightingQueue.add(() -> {
-+                    recheckGaps(flag);
-+                    --lightUpdates;
-+                });
-+            }
 +        }
 +    }
 +
@@ -71,7 +72,7 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000
                  if (flag) {
                      this.initLighting();
 -                } else {
-+                } else if (!world.paperConfig.queueLightUpdates) { // Paper
++                } else { lightingQueue.add(() -> { // Paper - Queue light update
                      int j1 = iblockdata.c();
                      int k1 = iblockdata1.c();
  
@@ -79,132 +80,147 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000
                      if (j1 != k1 && (j1 < k1 || this.getBrightness(EnumSkyBlock.SKY, blockposition) > 0 || this.getBrightness(EnumSkyBlock.BLOCK, blockposition) > 0)) {
                          this.d(i, k);
                      }
-+                    // Paper start - Queue light update
++                    }); // Paper
-+                } else {
-+                    int j1 = iblockdata.c();
-+                    int k1 = iblockdata1.c();
-+
-+                    ++lightUpdates;
-+                    world.getServer().getServer().lightingQueue.add(() -> {
-+                        if (j1 > 0) {
-+                            if (j >= i1) {
-+                                this.c(i, j + 1, k);
-+                            }
-+                        } else if (j == i1 - 1) {
-+                            this.c(i, j, k);
-+                        }
-+
-+                        if (j1 != k1 && (j1 < k1 || this.getBrightness(EnumSkyBlock.SKY, blockposition) > 0 || this.getBrightness(EnumSkyBlock.BLOCK, blockposition) > 0)) {
-+                            this.d(i, k);
-+                        }
-+
-+                        --lightUpdates;
-+                    });
-+                    // Paper end
                  }
  
                  TileEntity tileentity;
-@@ -0,0 +0,0 @@ public class Chunk {
- 
-         private EnumTileEntityState() {}
-     }
-+
-+    // Paper start
-+    public boolean hasLightUpdates() {
-+        if (world.paperConfig.queueLightUpdates) {
-+            if (lightUpdates > 0) {
-+                return true;
-+            }
-+
-+            for (int x = locX - 2; x <= locX + 2; ++x) {
-+                for (int z = locZ - 2; z <= locZ + 2; ++z) {
-+                    if ((x == 0 && z == 0) || (x == locX && z == locZ)) {
-+                        continue;
-+                    }
-+
-+                    Chunk chunk = MCUtil.getLoadedChunkWithoutMarkingActive(world, x, z);
-+                    if (chunk != null && chunk.lightUpdates > 0) {
-+                        return true;
-+                    }
-+                }
-+            }
-+        }
-+
-+        return false;
-+    }
-+    // Paper end
- }
 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 implements IChunkProvider {
-                     Chunk chunk = (Chunk) this.chunks.get(olong);
+                         if (event.isCancelled()) {
+                             continue;
+                         }
++                        chunk.lightingQueue.processUnload(); // Paper
  
-                     if (chunk != null && chunk.d) {
+                         // Update neighbor counts
-+                        if (chunk.hasLightUpdates()) continue; // Paper - Don't unload chunks with pending light updates.
+                         for (int x = -2; x < 3; x++) {
-                         // CraftBukkit start
-                         ChunkUnloadEvent event = new ChunkUnloadEvent(chunk.bukkitChunk);
-                         this.world.getServer().getPluginManager().callEvent(event);
 diff --git a/src/main/java/net/minecraft/server/MinecraftServer.java b/src/main/java/net/minecraft/server/MinecraftServer.java
 index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644
 --- a/src/main/java/net/minecraft/server/MinecraftServer.java
 +++ b/src/main/java/net/minecraft/server/MinecraftServer.java
-@@ -0,0 +0,0 @@ import org.bukkit.craftbukkit.CraftServer;
- // CraftBukkit end
- import co.aikar.timings.MinecraftTimings; // Paper
- 
-+// Paper start
-+import java.util.LinkedList;
-+import java.util.Queue;
-+// Paper end
-+
- public abstract class MinecraftServer implements Runnable, ICommandListener, IAsyncTaskHandler, IMojangStatistics {
- 
-     public static final Logger LOGGER = LogManager.getLogger();
 @@ -0,0 +0,0 @@ public abstract class MinecraftServer implements Runnable, ICommandListener, IAs
-     public final Thread primaryThread;
-     public java.util.Queue<Runnable> processQueue = new java.util.concurrent.ConcurrentLinkedQueue<Runnable>();
-     public int autosavePeriod;
-+    public final Queue<Runnable> lightingQueue = new LinkedList<Runnable>(); // Paper - Queued light updates
-     // CraftBukkit end
  
-     public MinecraftServer(OptionSet options, Proxy proxy, DataConverterManager dataconvertermanager, YggdrasilAuthenticationService yggdrasilauthenticationservice, MinecraftSessionService minecraftsessionservice, GameProfileRepository gameprofilerepository, UserCache usercache) {
+     protected void C() throws ExceptionWorldConflict { // CraftBukkit - added throws
+         co.aikar.timings.TimingsManager.FULL_SERVER_TICK.startTiming(); // Paper
+-        long i = System.nanoTime();
++        long i = System.nanoTime(); long startTime = i; // Paper
+ 
+         ++this.ticks;
+         if (this.S) {
 @@ -0,0 +0,0 @@ public abstract class MinecraftServer implements Runnable, ICommandListener, IAs
          this.methodProfiler.b();
-         this.methodProfiler.b();
  
-+        // Paper start - Flush light updates
+         org.spigotmc.WatchdogThread.tick(); // Spigot
-+        if (!lightingQueue.isEmpty()) {
++        PaperLightingQueue.processQueue(startTime); // Paper
-+            MinecraftTimings.lightingQueueTimer.startTiming();
+         co.aikar.timings.TimingsManager.FULL_SERVER_TICK.stopTiming(); // Paper
+     }
+ 
+diff --git a/src/main/java/net/minecraft/server/PaperLightingQueue.java b/src/main/java/net/minecraft/server/PaperLightingQueue.java
+new file mode 100644
+index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000
+--- /dev/null
++++ b/src/main/java/net/minecraft/server/PaperLightingQueue.java
+@@ -0,0 +0,0 @@
++package net.minecraft.server;
 +
-+            int updatesThisTick = 0;
++import co.aikar.timings.Timing;
-+            long cachedTime = System.currentTimeMillis();
++import java.util.ArrayDeque;
-+            long startTime = cachedTime - (this.h[this.ticks % 100] / 1000000);
-+            int maxTickTimeCap = MathHelper.floor((TICK_TIME / 1000000) * 0.8);
-+            int maxTickTime = Math.max(0, (int) (maxTickTimeCap - (cachedTime - startTime)));
-+            Runnable lightUpdate;
 +
-+            while (maxTickTime > 0 && (lightUpdate = lightingQueue.poll()) != null) {
++class PaperLightingQueue {
-+                lightUpdate.run();
++    private static final long MAX_TIME = (long) (1000000000 / 20 * .95);
-+                if (++updatesThisTick % 10 == 0) {
++    private static int updatesThisTick;
-+                    long currentTime = System.currentTimeMillis();
++
-+                    if (currentTime - cachedTime > maxTickTime) {
++
-+                        break;
++    static void processQueue(long curTime) {
++        updatesThisTick = 0;
++
++        final long startTime = System.nanoTime();
++        final long maxTickTime = MAX_TIME - (startTime - curTime);
++
++        START:
++        for (World world : MinecraftServer.getServer().worlds) {
++            if (!world.paperConfig.queueLightUpdates) {
++                continue;
++            }
++
++            for (Chunk chunk : ((WorldServer) world).getChunkProviderServer().chunks.values()) {
++                if (chunk.lightingQueue.processQueue(startTime, maxTickTime)) {
++                    break START;
++                }
++            }
++        }
++    }
++
++    static class LightingQueue extends ArrayDeque<Runnable> {
++        final private Chunk chunk;
++
++        LightingQueue(Chunk chunk) {
++            super();
++            this.chunk = chunk;
++        }
++
++        @Override
++        public boolean add(Runnable runnable) {
++            if (chunk.world.paperConfig.queueLightUpdates) {
++                return super.add(runnable);
++            }
++            runnable.run();
++            return true;
++        }
++
++        /**
++         * Processes the lighting queue for this chunk
++         *
++         * @param startTime If start Time is 0, we will not limit execution time
++         * @param maxTickTime Maximum time to spend processing lighting updates
++         * @return true to abort processing furthur lighting updates
++         */
++        private boolean processQueue(long startTime, long maxTickTime) {
++            if (this.isEmpty()) {
++                return false;
++            }
++            try (Timing ignored = chunk.world.timings.lightingQueueTimer.startTiming()) {
++                Runnable lightUpdate;
++                while ((lightUpdate = this.poll()) != null) {
++                    lightUpdate.run();
++                    if (startTime > 0 && ++PaperLightingQueue.updatesThisTick % 10 == 0 && PaperLightingQueue.updatesThisTick > 10) {
++                        if (System.nanoTime() - startTime > maxTickTime) {
++                            return true;
++                        }
 +                    }
-+
-+                    cachedTime = currentTime;
-+                    maxTickTime = Math.max(0, (int) (maxTickTimeCap - (currentTime - startTime)));
 +                }
 +            }
 +
-+            MinecraftTimings.lightingQueueTimer.stopTiming();
++            return false;
 +        }
-+        // Paper end
 +
-         org.spigotmc.WatchdogThread.tick(); // Spigot
++        /**
-         co.aikar.timings.TimingsManager.FULL_SERVER_TICK.stopTiming(); // Paper
++         * Flushes lighting updates to unload the chunk
-     }
++         */
++        void processUnload() {
++            if (!chunk.world.paperConfig.queueLightUpdates) {
++                return;
++            }
++            processQueue(0, 0); // No timeout
++
++            final int radius = 1; // TODO: bitflip, why should this ever be 2?
++            for (int x = chunk.locX - radius; x <= chunk.locX + radius; ++x) {
++                for (int z = chunk.locZ - radius; z <= chunk.locZ + radius; ++z) {
++                    if (x == chunk.locX && z == chunk.locZ) {
++                        continue;
++                    }
++
++                    Chunk neighbor = MCUtil.getLoadedChunkWithoutMarkingActive(chunk.world, x, z);
++                    if (neighbor != null) {
++                        neighbor.lightingQueue.processQueue(0, 0); // No timeout
++                    }
++                }
++            }
++        }
++    }
++}
 diff --git a/src/main/java/net/minecraft/server/World.java b/src/main/java/net/minecraft/server/World.java
 index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644
 --- a/src/main/java/net/minecraft/server/World.java
@@ -214,17 +230,7 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000
                  if (iblockdata.c() != iblockdata1.c() || iblockdata.d() != iblockdata1.d()) {
                      this.methodProfiler.a("checkLight");
 -                    this.w(blockposition);
-+                    // Paper start - Queue light update
++                    chunk.lightingQueue.add(() -> this.w(blockposition)); // Paper - Queue light update
-+                    if (!paperConfig.queueLightUpdates) {
-+                        this.w(blockposition);
-+                    } else {
-+                        ++chunk.lightUpdates;
-+                        getMinecraftServer().lightingQueue.add(() -> {
-+                            this.w(blockposition);
-+                            --chunk.lightUpdates;
-+                        });
-+                    }
-+                    // Paper end
                      this.methodProfiler.b();
                  }
  

Spigot-Server-Patches/Unmark-chunk-as-unloading-when-unload-is-cancelled.patch

@@ -15,8 +15,8 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000
 -    public boolean d;
 +    public boolean d;public void setShouldUnload(boolean unload) { this.d = unload; } // Paper // OBFHELPER
      protected gnu.trove.map.hash.TObjectIntHashMap<Class> entityCount = new gnu.trove.map.hash.TObjectIntHashMap<Class>(); // Spigot
-     public int lightUpdates; // Paper - Number of queued light updates for this chunk
  
+     // Paper start
 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
@@ -26,7 +26,7 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000
  
                      if (chunk != null && chunk.d) {
 +                        chunk.setShouldUnload(false); // Paper
-                         if (chunk.hasLightUpdates()) continue; // Paper - Don't unload chunks with pending light updates.
                          // CraftBukkit start
                          ChunkUnloadEvent event = new ChunkUnloadEvent(chunk.bukkitChunk);
+                         this.world.getServer().getPluginManager().callEvent(event);
 --

Spigot-Server-Patches/remove-null-possibility-for-getServer-singleton.patch

@@ -9,7 +9,7 @@ diff --git a/src/main/java/net/minecraft/server/MinecraftServer.java b/src/main/
 index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644
 --- a/src/main/java/net/minecraft/server/MinecraftServer.java
 +++ b/src/main/java/net/minecraft/server/MinecraftServer.java
-@@ -0,0 +0,0 @@ import java.util.Queue;
+@@ -0,0 +0,0 @@ import co.aikar.timings.MinecraftTimings; // Paper
  
  public abstract class MinecraftServer implements Runnable, ICommandListener, IAsyncTaskHandler, IMojangStatistics {