From e4c179e8277302b74877532781fb9df58823d2ca Mon Sep 17 00:00:00 2001 From: Aikar Date: Sun, 15 May 2016 18:48:39 -0400 Subject: [PATCH] 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. --- ...opper-searches-if-there-are-no-items.patch | 2 +- Spigot-Server-Patches/Lighting-Queue.patch | 284 +++++++++--------- ...s-unloading-when-unload-is-cancelled.patch | 4 +- ...-possibility-for-getServer-singleton.patch | 2 +- 4 files changed, 149 insertions(+), 143 deletions(-) diff --git a/Spigot-Server-Patches/Avoid-hopper-searches-if-there-are-no-items.patch b/Spigot-Server-Patches/Avoid-hopper-searches-if-there-are-no-items.patch index 75eb14015..8aca1cec6 100644 --- a/Spigot-Server-Patches/Avoid-hopper-searches-if-there-are-no-items.patch +++ b/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 entityCount = new gnu.trove.map.hash.TObjectIntHashMap(); // Spigot - public int lightUpdates; // Paper - Number of queued light updates for this chunk + // Paper start + // Track the number of minecarts and items diff --git a/Spigot-Server-Patches/Lighting-Queue.patch b/Spigot-Server-Patches/Lighting-Queue.patch index 426ce8acf..bbedfff37 100644 --- a/Spigot-Server-Patches/Lighting-Queue.patch +++ b/Spigot-Server-Patches/Lighting-Queue.patch @@ -3,19 +3,29 @@ From: Byteflux 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 -+++ b/src/main/java/co/aikar/timings/MinecraftTimings.java -@@ -0,0 +0,0 @@ public final class MinecraftTimings { - public static final Timing timeUpdateTimer = Timings.ofSafe("Time Update"); - public static final Timing serverCommandTimer = Timings.ofSafe("Server Command"); - public static final Timing worldSaveTimer = Timings.ofSafe("World Save"); -+ public static final Timing lightingQueueTimer = Timings.ofSafe("Lighting Queue"); +--- a/src/main/java/co/aikar/timings/WorldTimingsHandler.java ++++ b/src/main/java/co/aikar/timings/WorldTimingsHandler.java +@@ -0,0 +0,0 @@ public class WorldTimingsHandler { + public final Timing syncChunkLoadTileTicksTimer; + public final Timing syncChunkLoadPostTimer; - public static final Timing tickEntityTimer = Timings.ofSafe("## tickEntity"); - public static final Timing tickTileEntityTimer = Timings.ofSafe("## tickTileEntity"); ++ public final Timing lightingQueueTimer; ++ + 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 y; - public boolean d; - protected gnu.trove.map.hash.TObjectIntHashMap entityCount = new gnu.trove.map.hash.TObjectIntHashMap(); // Spigot -+ public int lightUpdates; // Paper - Number of queued light updates for this chunk - - // CraftBukkit start - Neighbor loaded cache for chunk lighting and entity ticking - private int neighbors = 0x1 << 12; + private boolean m; + public final Map tileEntities; + public final List[] entitySlices; // Spigot ++ final PaperLightingQueue.LightingQueue lightingQueue = new PaperLightingQueue.LightingQueue(this); // Paper + private boolean done; + private boolean lit; + 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 -+ if (!world.paperConfig.queueLightUpdates) { -+ recheckGaps(flag); -+ } else { -+ ++lightUpdates; -+ world.getServer().getServer().lightingQueue.add(() -> { -+ recheckGaps(flag); -+ --lightUpdates; -+ }); -+ } ++ lightingQueue.add(() -> recheckGaps(flag)); // Paper - Queue light update + } + } + @@ -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 -+ } 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 ++ }); // Paper } 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) { -+ 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); + // Update neighbor counts + for (int x = -2; x < 3; x++) { 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 processQueue = new java.util.concurrent.ConcurrentLinkedQueue(); - public int autosavePeriod; -+ public final Queue lightingQueue = new LinkedList(); // 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 -+ if (!lightingQueue.isEmpty()) { -+ MinecraftTimings.lightingQueueTimer.startTiming(); + org.spigotmc.WatchdogThread.tick(); // Spigot ++ PaperLightingQueue.processQueue(startTime); // Paper + 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; -+ long cachedTime = System.currentTimeMillis(); -+ 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; ++import co.aikar.timings.Timing; ++import java.util.ArrayDeque; + -+ while (maxTickTime > 0 && (lightUpdate = lightingQueue.poll()) != null) { -+ lightUpdate.run(); -+ if (++updatesThisTick % 10 == 0) { -+ long currentTime = System.currentTimeMillis(); -+ if (currentTime - cachedTime > maxTickTime) { -+ break; ++class PaperLightingQueue { ++ private static final long MAX_TIME = (long) (1000000000 / 20 * .95); ++ private static int updatesThisTick; ++ ++ ++ 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 { ++ 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 -+ if (!paperConfig.queueLightUpdates) { -+ this.w(blockposition); -+ } else { -+ ++chunk.lightUpdates; -+ getMinecraftServer().lightingQueue.add(() -> { -+ this.w(blockposition); -+ --chunk.lightUpdates; -+ }); -+ } -+ // Paper end ++ chunk.lightingQueue.add(() -> this.w(blockposition)); // Paper - Queue light update this.methodProfiler.b(); } diff --git a/Spigot-Server-Patches/Unmark-chunk-as-unloading-when-unload-is-cancelled.patch b/Spigot-Server-Patches/Unmark-chunk-as-unloading-when-unload-is-cancelled.patch index 5e6d40558..32d0f815e 100644 --- a/Spigot-Server-Patches/Unmark-chunk-as-unloading-when-unload-is-cancelled.patch +++ 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 entityCount = new gnu.trove.map.hash.TObjectIntHashMap(); // 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); -- \ No newline at end of file diff --git a/Spigot-Server-Patches/remove-null-possibility-for-getServer-singleton.patch b/Spigot-Server-Patches/remove-null-possibility-for-getServer-singleton.patch index 5bc182b7f..daea82d23 100644 --- a/Spigot-Server-Patches/remove-null-possibility-for-getServer-singleton.patch +++ b/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 {