Chunk save queue improvements

This commit is contained in:
Aikar 2016-03-04 20:14:27 -06:00
parent 9fc52783a2
commit cedb1c3686
2 changed files with 167 additions and 39 deletions

View file

@ -0,0 +1,167 @@
From 0000000000000000000000000000000000000000 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.
If there is work to do, then the thread should be doing its work, and only sleep when it is done.
Additionally, optimize the ChunkRegionLoader queue to reduce lock contention (issue with 1.9)
diff --git a/src/main/java/com/destroystokyo/paper/PaperConfig.java b/src/main/java/com/destroystokyo/paper/PaperConfig.java
index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644
--- a/src/main/java/com/destroystokyo/paper/PaperConfig.java
+++ b/src/main/java/com/destroystokyo/paper/PaperConfig.java
@@ -0,0 +0,0 @@ public class PaperConfig {
}
return time;
}
+
+ 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/ChunkRegionLoader.java b/src/main/java/net/minecraft/server/ChunkRegionLoader.java
index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644
--- a/src/main/java/net/minecraft/server/ChunkRegionLoader.java
+++ b/src/main/java/net/minecraft/server/ChunkRegionLoader.java
@@ -0,0 +0,0 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.ConcurrentLinkedQueue; // Paper
import org.apache.logging.log4j.LogManager;
import org.apache.logging.log4j.Logger;
public class ChunkRegionLoader implements IChunkLoader, IAsyncChunkSaver {
+ private ConcurrentLinkedQueue<QueuedChunk> queue = new ConcurrentLinkedQueue<QueuedChunk>(); // Paper - Chunk queue improvements
+ private final Object lock = new Object(); // Paper - Chunk queue improvements
private static final Logger a = LogManager.getLogger();
private Map<ChunkCoordIntPair, NBTTagCompound> b = new ConcurrentHashMap();
- private Set<ChunkCoordIntPair> c = Collections.newSetFromMap(new ConcurrentHashMap());
+ //private Set<ChunkCoordIntPair> c = Collections.newSetFromMap(new ConcurrentHashMap()); // Paper - Chunk queue improvements
private final File d;
private final DataConverterManager e;
private boolean f = false;
@@ -0,0 +0,0 @@ public class ChunkRegionLoader implements IChunkLoader, IAsyncChunkSaver {
public boolean chunkExists(World world, int i, int j) {
ChunkCoordIntPair chunkcoordintpair = new ChunkCoordIntPair(i, j);
- if (this.c.contains(chunkcoordintpair)) {
+ //if (this.c.contains(chunkcoordintpair)) { // Paper - Chunk queue improvements
if (this.b.containsKey(chunkcoordintpair)) {
return true;
}
- }
+ //} // Paper - Chunk queue improvements
// Paper start - Don't create region files when checking that they exist
final RegionFile region = RegionFileCache.a(this.d, i, j, false);
@@ -0,0 +0,0 @@ public class ChunkRegionLoader implements IChunkLoader, IAsyncChunkSaver {
}
protected void a(ChunkCoordIntPair chunkcoordintpair, NBTTagCompound nbttagcompound) {
- if (!this.c.contains(chunkcoordintpair)) {
+ synchronized (lock) { // Paper - Chunk queue improvements
this.b.put(chunkcoordintpair, nbttagcompound);
}
+ queue.add(new QueuedChunk(chunkcoordintpair, nbttagcompound)); // Paper - Chunk queue improvements
FileIOThread.a().a(this);
}
public boolean c() {
- if (this.b.isEmpty()) {
+ // Paper start - Chunk queue improvements
+ QueuedChunk chunk = queue.poll();
+ if (chunk == null) {
+ // Paper - end
if (this.f) {
ChunkRegionLoader.a.info("ThreadedAnvilChunkStorage ({}): All chunks are saved", new Object[] { this.d.getName()});
}
return false;
} else {
- ChunkCoordIntPair chunkcoordintpair = (ChunkCoordIntPair) this.b.keySet().iterator().next();
+ ChunkCoordIntPair chunkcoordintpair = chunk.coords; // Paper - Chunk queue improvements
boolean flag;
try {
- this.c.add(chunkcoordintpair);
- NBTTagCompound nbttagcompound = (NBTTagCompound) this.b.remove(chunkcoordintpair);
+ //this.c.add(chunkcoordintpair);
+ NBTTagCompound nbttagcompound = chunk.compound; // Paper - Chunk queue improvements
if (nbttagcompound != null) {
try {
@@ -0,0 +0,0 @@ public class ChunkRegionLoader implements IChunkLoader, IAsyncChunkSaver {
ChunkRegionLoader.a.error("Failed to save chunk", exception);
}
}
+ synchronized (lock) { if (this.b.get(chunkcoordintpair) == nbttagcompound) { this.b.remove(chunkcoordintpair); } }// Paper - This will not equal if a newer version is still pending
flag = true;
} finally {
- this.c.remove(chunkcoordintpair);
+ //this.c.remove(chunkcoordintpair); // Paper
}
return flag;
@@ -0,0 +0,0 @@ public class ChunkRegionLoader implements IChunkLoader, IAsyncChunkSaver {
return entity;
}
}
+
+ // Paper start - Chunk queue improvements
+ private static class QueuedChunk {
+ public ChunkCoordIntPair coords;
+ public NBTTagCompound compound;
+
+ public QueuedChunk(ChunkCoordIntPair coords, NBTTagCompound compound) {
+ this.coords = coords;
+ this.compound = compound;
+ }
+ }
+ // Paper end
}
diff --git a/src/main/java/net/minecraft/server/FileIOThread.java b/src/main/java/net/minecraft/server/FileIOThread.java
index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644
--- a/src/main/java/net/minecraft/server/FileIOThread.java
+++ b/src/main/java/net/minecraft/server/FileIOThread.java
@@ -0,0 +0,0 @@ public class FileIOThread implements Runnable {
++this.d;
}
- try {
- Thread.sleep(this.e ? 0L : 10L);
- } catch (InterruptedException interruptedexception) {
- interruptedexception.printStackTrace();
+ // Paper start - Add toggle
+ if (com.destroystokyo.paper.PaperConfig.enableFileIOThreadSleep) {
+ try {
+ Thread.sleep(this.e ? 0L : 10L);
+ } catch (InterruptedException interruptedexception) {
+ interruptedexception.printStackTrace();
+ }
}
+ // Paper end
}
if (this.b.isEmpty()) {
--

View file

@ -1,39 +0,0 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Aikar <aikar@aikar.co>
Date: Wed, 2 Mar 2016 23:58:29 -0600
Subject: [PATCH] Don't sleep between chunk saves
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.
If there is work to do, then the thread should be doing its work, and only sleep when it is done.
diff --git a/src/main/java/net/minecraft/server/FileIOThread.java b/src/main/java/net/minecraft/server/FileIOThread.java
index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644
--- a/src/main/java/net/minecraft/server/FileIOThread.java
+++ b/src/main/java/net/minecraft/server/FileIOThread.java
@@ -0,0 +0,0 @@ public class FileIOThread implements Runnable {
++this.d;
}
+ /* // Paper start - don't sleep in between chunks so we unload faster.
try {
Thread.sleep(this.e ? 0L : 10L);
} catch (InterruptedException interruptedexception) {
interruptedexception.printStackTrace();
- }
+ } */ // Paper end
}
if (this.b.isEmpty()) {
--