206 lines
		
	
	
	
		
			10 KiB
			
		
	
	
	
		
			Diff
		
	
	
	
	
	
			
		
		
	
	
			206 lines
		
	
	
	
		
			10 KiB
			
		
	
	
	
		
			Diff
		
	
	
	
	
	
From 208077ba7ae7f41d3bdfcc4f5bc577faa8095905 Mon Sep 17 00:00:00 2001
 | 
						|
From: Aikar <aikar@aikar.co>
 | 
						|
Date: Fri, 4 Mar 2016 18:18:37 -0600
 | 
						|
Subject: [PATCH] Chunk save queue improvements
 | 
						|
 | 
						|
For some unknown reason, Minecraft is sleeping 10ms between every single chunk being saved to disk.
 | 
						|
Under high chunk load/unload activity (lots of movement / teleporting), this causes the chunk unload queue
 | 
						|
to build up in size.
 | 
						|
 | 
						|
This has multiple impacts:
 | 
						|
1) Performance of the unload queue itself - The save thread is pretty ineffecient for how it accesses it
 | 
						|
   By letting the queue get larger, checking and popping work off the queue can get less performant.
 | 
						|
2) Performance of chunk loading - As with #1, chunk loads also have to check this queue when loading
 | 
						|
   chunk data so that it doesn't load stale data if new data is pending write to disk.
 | 
						|
3) Memory Usage - The entire chunk has been serialized to NBT, and now sits in this queue. This leads to
 | 
						|
   elevated memory usage, and then the objects used in the serialization sit around longer than needed,
 | 
						|
   resulting in promotion to Old Generation instead of dying young.
 | 
						|
 | 
						|
To optimize this, we change the entire unload queue to be a proper queue. This improves the behavior of popping
 | 
						|
the first queued chunk off, instead of abusing iterators like Mojang was doing.
 | 
						|
 | 
						|
This also improves reliability of chunk saving, as the previous hack job had a race condition that could
 | 
						|
fail to save some chunks.
 | 
						|
 | 
						|
Then finally, Sleeping will by default be removed, but due to known issues with 1.9, a config option was added.
 | 
						|
But if sleeps are to remain enabled, we at least lower the sleep interval so it doesn't have as much negative impact.
 | 
						|
 | 
						|
diff --git a/src/main/java/com/destroystokyo/paper/PaperConfig.java b/src/main/java/com/destroystokyo/paper/PaperConfig.java
 | 
						|
index cfcc244672..4e932ea235 100644
 | 
						|
--- a/src/main/java/com/destroystokyo/paper/PaperConfig.java
 | 
						|
+++ b/src/main/java/com/destroystokyo/paper/PaperConfig.java
 | 
						|
@@ -217,4 +217,10 @@ public class PaperConfig {
 | 
						|
                 " - Interval: " + timeSummary(Timings.getHistoryInterval() / 20) +
 | 
						|
                 " - Length: " + timeSummary(Timings.getHistoryLength() / 20));
 | 
						|
     }
 | 
						|
+
 | 
						|
+    public static boolean enableFileIOThreadSleep;
 | 
						|
+    private static void enableFileIOThreadSleep() {
 | 
						|
+        enableFileIOThreadSleep = getBoolean("settings.sleep-between-chunk-saves", false);
 | 
						|
+        if (enableFileIOThreadSleep) Bukkit.getLogger().info("Enabled sleeping between chunk saves, beware of memory issues");
 | 
						|
+    }
 | 
						|
 }
 | 
						|
diff --git a/src/main/java/net/minecraft/server/ChunkCoordIntPair.java b/src/main/java/net/minecraft/server/ChunkCoordIntPair.java
 | 
						|
index b0c004b1f2..d2cece2651 100644
 | 
						|
--- a/src/main/java/net/minecraft/server/ChunkCoordIntPair.java
 | 
						|
+++ b/src/main/java/net/minecraft/server/ChunkCoordIntPair.java
 | 
						|
@@ -20,6 +20,7 @@ public class ChunkCoordIntPair {
 | 
						|
         this.z = (int) (i >> 32);
 | 
						|
     }
 | 
						|
 
 | 
						|
+    public long asLong() { return a(); } // Paper
 | 
						|
     public long a() {
 | 
						|
         return a(this.x, this.z);
 | 
						|
     }
 | 
						|
diff --git a/src/main/java/net/minecraft/server/ChunkRegionLoader.java b/src/main/java/net/minecraft/server/ChunkRegionLoader.java
 | 
						|
index 35976a26f3..21ee154a57 100644
 | 
						|
--- a/src/main/java/net/minecraft/server/ChunkRegionLoader.java
 | 
						|
+++ b/src/main/java/net/minecraft/server/ChunkRegionLoader.java
 | 
						|
@@ -20,6 +20,7 @@ import java.util.Map.Entry;
 | 
						|
 import java.util.function.Consumer;
 | 
						|
 import java.util.function.Function;
 | 
						|
 import javax.annotation.Nullable;
 | 
						|
+import java.util.concurrent.ConcurrentLinkedQueue; // Paper
 | 
						|
 import org.apache.logging.log4j.LogManager;
 | 
						|
 import org.apache.logging.log4j.Logger;
 | 
						|
 // Spigot start
 | 
						|
@@ -29,8 +30,28 @@ import org.spigotmc.SupplierUtils;
 | 
						|
 
 | 
						|
 public class ChunkRegionLoader implements IChunkLoader, IAsyncChunkSaver {
 | 
						|
 
 | 
						|
+    // Paper start - Chunk queue improvements
 | 
						|
+    private static class QueuedChunk {
 | 
						|
+        public ChunkCoordIntPair coords;
 | 
						|
+        public Supplier<NBTTagCompound> compoundSupplier;
 | 
						|
+        public Runnable onSave;
 | 
						|
+
 | 
						|
+        public QueuedChunk(Runnable run) {
 | 
						|
+            this.coords = null;
 | 
						|
+            this.compoundSupplier = null;
 | 
						|
+            this.onSave = run;
 | 
						|
+        }
 | 
						|
+
 | 
						|
+        public QueuedChunk(ChunkCoordIntPair coords, Supplier<NBTTagCompound> compoundSupplier) {
 | 
						|
+            this.coords = coords;
 | 
						|
+            this.compoundSupplier = compoundSupplier;
 | 
						|
+        }
 | 
						|
+    }
 | 
						|
+    final private ConcurrentLinkedQueue<QueuedChunk> queue = new ConcurrentLinkedQueue<>();
 | 
						|
+    // Paper end
 | 
						|
+
 | 
						|
     private static final Logger a = LogManager.getLogger();
 | 
						|
-    private final Map<ChunkCoordIntPair, Supplier<NBTTagCompound>> b = Maps.newHashMap();
 | 
						|
+    private final it.unimi.dsi.fastutil.longs.Long2ObjectMap<Supplier<NBTTagCompound>> saveMap = it.unimi.dsi.fastutil.longs.Long2ObjectMaps.synchronize(new it.unimi.dsi.fastutil.longs.Long2ObjectOpenHashMap<>()); // Paper
 | 
						|
     private final File c;
 | 
						|
     private final DataFixer d;
 | 
						|
     private PersistentStructureLegacy e;
 | 
						|
@@ -86,7 +107,7 @@ public class ChunkRegionLoader implements IChunkLoader, IAsyncChunkSaver {
 | 
						|
             return null;
 | 
						|
         }
 | 
						|
         // CraftBukkit end
 | 
						|
-        NBTTagCompound nbttagcompound = SupplierUtils.getIfExists(this.b.get(new ChunkCoordIntPair(i, j))); // Spigot
 | 
						|
+        NBTTagCompound nbttagcompound = SupplierUtils.getIfExists(this.saveMap.get(ChunkCoordIntPair.asLong(i, j))); // Spigot // Paper
 | 
						|
 
 | 
						|
         if (nbttagcompound != null) {
 | 
						|
             return nbttagcompound;
 | 
						|
@@ -314,7 +335,7 @@ public class ChunkRegionLoader implements IChunkLoader, IAsyncChunkSaver {
 | 
						|
                 };
 | 
						|
             }
 | 
						|
 
 | 
						|
-            this.a(chunkcoordintpair, SupplierUtils.createUnivaluedSupplier(completion, unloaded && this.b.size() < SAVE_QUEUE_TARGET_SIZE));
 | 
						|
+            this.a(chunkcoordintpair, SupplierUtils.createUnivaluedSupplier(completion, unloaded)); // Paper - Remove save queue target size
 | 
						|
             // Spigot end
 | 
						|
         } catch (Exception exception) {
 | 
						|
             ChunkRegionLoader.a.error("Failed to save chunk", exception);
 | 
						|
@@ -323,7 +344,8 @@ public class ChunkRegionLoader implements IChunkLoader, IAsyncChunkSaver {
 | 
						|
     }
 | 
						|
 
 | 
						|
     protected void a(ChunkCoordIntPair chunkcoordintpair, Supplier<NBTTagCompound> nbttagcompound) { // Spigot
 | 
						|
-        this.b.put(chunkcoordintpair, nbttagcompound);
 | 
						|
+        this.saveMap.put(chunkcoordintpair.asLong(), nbttagcompound); // Paper
 | 
						|
+        queue.add(new QueuedChunk(chunkcoordintpair, nbttagcompound)); // Paper - Chunk queue improvements
 | 
						|
         FileIOThread.a().a(this);
 | 
						|
     }
 | 
						|
 
 | 
						|
@@ -333,20 +355,24 @@ public class ChunkRegionLoader implements IChunkLoader, IAsyncChunkSaver {
 | 
						|
     }
 | 
						|
 
 | 
						|
     private boolean processSaveQueueEntry(boolean logCompletion) {
 | 
						|
-        Iterator<Entry<ChunkCoordIntPair, Supplier<NBTTagCompound>>> iterator = this.b.entrySet().iterator(); // Spigot
 | 
						|
-
 | 
						|
-        if (!iterator.hasNext()) {
 | 
						|
+        // Paper start - Chunk queue improvements
 | 
						|
+        QueuedChunk chunk = queue.poll();
 | 
						|
+        if (chunk == null) {
 | 
						|
+        // Paper - end
 | 
						|
             if (logCompletion) { // CraftBukkit
 | 
						|
                 ChunkRegionLoader.a.info("ThreadedAnvilChunkStorage ({}): All chunks are saved", this.c.getName());
 | 
						|
             }
 | 
						|
 
 | 
						|
             return false;
 | 
						|
         } else {
 | 
						|
-            Entry<ChunkCoordIntPair, NBTTagCompound> entry = (Entry) iterator.next();
 | 
						|
-
 | 
						|
-            iterator.remove();
 | 
						|
-            ChunkCoordIntPair chunkcoordintpair = (ChunkCoordIntPair) entry.getKey();
 | 
						|
-            Supplier<NBTTagCompound> nbttagcompound = (Supplier<NBTTagCompound>) entry.getValue(); // Spigot
 | 
						|
+            // Paper start
 | 
						|
+            if (chunk.onSave != null) {
 | 
						|
+                chunk.onSave.run();
 | 
						|
+                return true;
 | 
						|
+            }
 | 
						|
+            // Paper end
 | 
						|
+            ChunkCoordIntPair chunkcoordintpair = chunk.coords; // Paper - Chunk queue improvements
 | 
						|
+            Supplier<NBTTagCompound> nbttagcompound = chunk.compoundSupplier; // Spigot // Paper
 | 
						|
 
 | 
						|
             if (nbttagcompound == null) {
 | 
						|
                 return true;
 | 
						|
@@ -355,6 +381,15 @@ public class ChunkRegionLoader implements IChunkLoader, IAsyncChunkSaver {
 | 
						|
                     // CraftBukkit start
 | 
						|
                     RegionFileCache.write(this.c, chunkcoordintpair.x, chunkcoordintpair.z, SupplierUtils.getIfExists(nbttagcompound)); // Spigot
 | 
						|
 
 | 
						|
+                    // Paper start remove from map only if this was the latest version of the chunk
 | 
						|
+                    synchronized (this.saveMap) {
 | 
						|
+                        long k = chunkcoordintpair.asLong();
 | 
						|
+                        // This will not equal if a newer version is still pending - wait until newest is saved to remove
 | 
						|
+                        if (this.saveMap.get(k) == chunk.compoundSupplier) {
 | 
						|
+                            this.saveMap.remove(k);
 | 
						|
+                        }
 | 
						|
+                    }
 | 
						|
+                    // Paper end
 | 
						|
                     /*
 | 
						|
                     NBTCompressedStreamTools.a(nbttagcompound, (DataOutput) dataoutputstream);
 | 
						|
                     dataoutputstream.close();
 | 
						|
diff --git a/src/main/java/net/minecraft/server/FileIOThread.java b/src/main/java/net/minecraft/server/FileIOThread.java
 | 
						|
index 8c3537ab8d..3c688f546c 100644
 | 
						|
--- a/src/main/java/net/minecraft/server/FileIOThread.java
 | 
						|
+++ b/src/main/java/net/minecraft/server/FileIOThread.java
 | 
						|
@@ -38,20 +38,21 @@ public class FileIOThread implements Runnable {
 | 
						|
             IAsyncChunkSaver iasyncchunksaver = (IAsyncChunkSaver) this.c.get(i);
 | 
						|
             boolean flag;
 | 
						|
 
 | 
						|
-            synchronized (iasyncchunksaver) {
 | 
						|
+            //synchronized (iasyncchunksaver) { // Paper - remove synchronized
 | 
						|
                 flag = iasyncchunksaver.a();
 | 
						|
-            }
 | 
						|
+            //} // Paper
 | 
						|
 
 | 
						|
             if (!flag) {
 | 
						|
                 this.c.remove(i--);
 | 
						|
                 ++this.e;
 | 
						|
             }
 | 
						|
 
 | 
						|
+            if (com.destroystokyo.paper.PaperConfig.enableFileIOThreadSleep) { // Paper
 | 
						|
             try {
 | 
						|
-                Thread.sleep(this.f ? 0L : 10L);
 | 
						|
+                Thread.sleep(this.f ? 0L : 1L); // Paper
 | 
						|
             } catch (InterruptedException interruptedexception) {
 | 
						|
                 interruptedexception.printStackTrace();
 | 
						|
-            }
 | 
						|
+            }} // Paper
 | 
						|
         }
 | 
						|
 
 | 
						|
         if (this.c.isEmpty()) {
 | 
						|
-- 
 | 
						|
2.21.0
 | 
						|
 |