mirrors/papermc
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Fix issues with 167 causing crashes due to missing chunks - Fixes #3122

Browse source 
Forgot to flip the pending boolean back to false, causing it to copy
empty data on the next tick if nothing else triggered a load.

haven't managed to actually reproduce the crash others got, but did
verify that the bad copy was occurring erasing the data.

also fixed a bug with chunk load callback not executing before
another one was scheduled.
This commit is contained in:
Aikar 2020-04-10 14:11:46 -04:00
parent 23ee0a8a88
commit 0ed6da7ab2
No known key found for this signature in database
GPG key ID: 401ADFC9891FAAFE
2 changed files with 79 additions and 159 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

101
Spigot-Server-Patches/0459-Optimize-PlayerChunkMap-memory-use-for-visibleChunks.patch
View file

@ -1,4 +1,4 @@
From cee599dd7b5c962429d797a091ab740da42179a3 Mon Sep 17 00:00:00 2001
From 4134905d5d67b984c29785aa279cd0fb91c73140 Mon Sep 17 00:00:00 2001
From: Aikar <aikar@aikar.co>
Date: Wed, 8 Apr 2020 03:06:30 -0400
Subject: [PATCH] Optimize PlayerChunkMap memory use for visibleChunks
@ -11,6 +11,44 @@ as before with only 2 small objects created (FastIterator/MapEntry)
This should result in siginificant memory use reduction and improved GC behavior.
diff --git a/src/main/java/com/destroystokyo/paper/util/map/Long2ObjectLinkedOpenHashMapFastCopy.java b/src/main/java/com/destroystokyo/paper/util/map/Long2ObjectLinkedOpenHashMapFastCopy.java
new file mode 100644
index 0000000000..e0ad725b2e
--- /dev/null
+++ b/src/main/java/com/destroystokyo/paper/util/map/Long2ObjectLinkedOpenHashMapFastCopy.java
@@ -0,0 +1,32 @@
+package com.destroystokyo.paper.util.map;
+
+import it.unimi.dsi.fastutil.longs.Long2ObjectLinkedOpenHashMap;
+
+public class Long2ObjectLinkedOpenHashMapFastCopy<V> extends Long2ObjectLinkedOpenHashMap<V> {
+
+ public void copyFrom(Long2ObjectLinkedOpenHashMapFastCopy<V> map) {
+ if (key.length < map.key.length) {
+ key = null;
+ key = new long[map.key.length];
+ }
+ if (value.length < map.value.length) {
+ value = null;
+ //noinspection unchecked
+ value = (V[]) new Object[map.value.length];
+ }
+ if (link.length < map.link.length) {
+ link = null;
+ link = new long[map.link.length];
+ }
+ System.arraycopy(map.key, 0, this.key, 0, map.key.length);
+ System.arraycopy(map.value, 0, this.value, 0, map.value.length);
+ System.arraycopy(map.link, 0, this.link, 0, map.link.length);
+ this.size = map.size;
+ this.mask = map.mask;
+ this.first = map.first;
+ this.last = map.last;
+ this.n = map.n;
+ this.maxFill = map.maxFill;
+ this.containsNullKey = map.containsNullKey;
+ }
+}
diff --git a/src/main/java/net/minecraft/server/ChunkProviderServer.java b/src/main/java/net/minecraft/server/ChunkProviderServer.java
index 1dcd0980ec..e627440c41 100644
--- a/src/main/java/net/minecraft/server/ChunkProviderServer.java
@ -38,20 +76,23 @@ index b9d5844520..9980e4c277 100644
List<PlayerChunk> allChunks = new ArrayList<>(visibleChunks.values());
List<EntityPlayer> players = world.players;
diff --git a/src/main/java/net/minecraft/server/PlayerChunkMap.java b/src/main/java/net/minecraft/server/PlayerChunkMap.java
index e1e4ea793a..ce645a69bd 100644
index e1e4ea793a..e61ddeb1ff 100644
--- a/src/main/java/net/minecraft/server/PlayerChunkMap.java
+++ b/src/main/java/net/minecraft/server/PlayerChunkMap.java
@@ -56,7 +56,8 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
@@ -55,8 +55,10 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
private static final Logger LOGGER = LogManager.getLogger();
public static final int GOLDEN_TICKET = 33 + ChunkStatus.b();
public final Long2ObjectLinkedOpenHashMap<PlayerChunk> updatingChunks = new Long2ObjectLinkedOpenHashMap();
- public final Long2ObjectLinkedOpenHashMap<PlayerChunk> updatingChunks = new Long2ObjectLinkedOpenHashMap();
- public volatile Long2ObjectLinkedOpenHashMap<PlayerChunk> visibleChunks;
+ public final Long2ObjectLinkedOpenHashMap<PlayerChunk> visibleChunks = new Long2ObjectLinkedOpenHashMap(); // Paper - remove copying, make mt safe
+ public transient Long2ObjectLinkedOpenHashMap<PlayerChunk> visibleChunksClone; // Paper - remove copying, make mt safe
+ public final com.destroystokyo.paper.util.map.Long2ObjectLinkedOpenHashMapFastCopy<PlayerChunk> updatingChunks = new com.destroystokyo.paper.util.map.Long2ObjectLinkedOpenHashMapFastCopy<>(); // Paper - faster copying
+ public final com.destroystokyo.paper.util.map.Long2ObjectLinkedOpenHashMapFastCopy<PlayerChunk> visibleChunks = new com.destroystokyo.paper.util.map.Long2ObjectLinkedOpenHashMapFastCopy<>(); // Paper - faster copying
+ public final com.destroystokyo.paper.util.map.Long2ObjectLinkedOpenHashMapFastCopy<PlayerChunk> pendingVisibleChunks = new com.destroystokyo.paper.util.map.Long2ObjectLinkedOpenHashMapFastCopy<>(); // Paper - this is used if the visible chunks is updated while iterating only
+ public transient Long2ObjectLinkedOpenHashMap<PlayerChunk> visibleChunksClone; // Paper - used for async access of visible chunks, clone and cache only when needed
private final Long2ObjectLinkedOpenHashMap<PlayerChunk> pendingUnload;
final LongSet loadedChunks; // Paper - private -> package
public final WorldServer world;
@@ -170,7 +171,7 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
@@ -170,7 +172,7 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
public PlayerChunkMap(WorldServer worldserver, File file, DataFixer datafixer, DefinedStructureManager definedstructuremanager, Executor executor, IAsyncTaskHandler<Runnable> iasynctaskhandler, ILightAccess ilightaccess, ChunkGenerator<?> chunkgenerator, WorldLoadListener worldloadlistener, Supplier<WorldPersistentData> supplier, int i) {
super(new File(worldserver.getWorldProvider().getDimensionManager().a(file), "region"), datafixer);
@ -60,17 +101,17 @@ index e1e4ea793a..ce645a69bd 100644
this.pendingUnload = new Long2ObjectLinkedOpenHashMap();
this.loadedChunks = new LongOpenHashSet();
this.unloadQueue = new LongOpenHashSet();
@@ -262,8 +263,49 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
@@ -262,9 +264,52 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
return (PlayerChunk) this.updatingChunks.get(i);
}
+ // Paper start - remove cloning of visible chunks unless accessed as a collection async
+ private static final boolean DEBUG_ASYNC_VISIBLE_CHUNKS = Boolean.getBoolean("paper.debug-async-visible-chunks");
+ private boolean isIterating = false;
+ private boolean hasPendingVisibleUpdate = false;
+ public void forEachVisibleChunk(java.util.function.Consumer<PlayerChunk> consumer) {
+ org.spigotmc.AsyncCatcher.catchOp("forEachVisibleChunk");
+ boolean prev = isIterating;
+ boolean wasUpdating = updatingChunksModified;
+ isIterating = true;
+ try {
+ for (PlayerChunk value : this.visibleChunks.values()) {
@ -78,8 +119,10 @@ index e1e4ea793a..ce645a69bd 100644
+ }
+ } finally {
+ this.isIterating = prev;
+ if (!this.isIterating && updatingChunksModified && !wasUpdating) {
+ this.updateVisibleChunks();
+ if (!this.isIterating && this.hasPendingVisibleUpdate) {
+ this.visibleChunks.copyFrom(this.pendingVisibleChunks);
+ this.pendingVisibleChunks.clear();
+ this.hasPendingVisibleUpdate = false;
+ }
+ }
+ }
@ -90,7 +133,7 @@ index e1e4ea793a..ce645a69bd 100644
+ synchronized (this.visibleChunks) {
+ if (DEBUG_ASYNC_VISIBLE_CHUNKS) new Throwable("Async getVisibleChunks").printStackTrace();
+ if (this.visibleChunksClone == null) {
+ this.visibleChunksClone = this.visibleChunks.clone();
+ this.visibleChunksClone = this.hasPendingVisibleUpdate ? this.pendingVisibleChunks.clone() : this.visibleChunks.clone();
+ }
+ return this.visibleChunksClone;
+ }
@ -100,17 +143,19 @@ index e1e4ea793a..ce645a69bd 100644
+
@Nullable
public PlayerChunk getVisibleChunk(long i) { // Paper - protected -> public
- return (PlayerChunk) this.visibleChunks.get(i);
+ // Paper start - mt safe get
+ if (Thread.currentThread() != this.world.serverThread) {
+ synchronized (this.visibleChunks) {
+ return (PlayerChunk) this.visibleChunks.get(i);
+ return (PlayerChunk) (this.hasPendingVisibleUpdate ? this.pendingVisibleChunks.get(i) : this.visibleChunks.get(i));
+ }
+ }
+ return (PlayerChunk) (this.hasPendingVisibleUpdate ? this.pendingVisibleChunks.get(i) : this.visibleChunks.get(i));
+ // Paper end
return (PlayerChunk) this.visibleChunks.get(i);
}
@@ -444,8 +486,9 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
protected IntSupplier c(long i) {
@@ -444,8 +489,9 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
// Paper end
protected void save(boolean flag) {
@ -121,7 +166,7 @@ index e1e4ea793a..ce645a69bd 100644
MutableBoolean mutableboolean = new MutableBoolean();
do {
@@ -473,7 +516,7 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
@@ -473,7 +519,7 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
// this.i(); // Paper - nuke IOWorker
PlayerChunkMap.LOGGER.info("ThreadedAnvilChunkStorage ({}): All chunks are saved", this.w.getName());
} else {
@ -130,29 +175,29 @@ index e1e4ea793a..ce645a69bd 100644
IChunkAccess ichunkaccess = (IChunkAccess) playerchunk.getChunkSave().getNow(null); // CraftBukkit - decompile error
if (ichunkaccess instanceof ProtoChunkExtension || ichunkaccess instanceof Chunk) {
@@ -639,11 +682,19 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
});
}
+ protected boolean updateVisibleChunks() { return b(); } // Paper - OBFHELPER
protected boolean b() {
- if (!this.updatingChunksModified) {
+ if (!this.updatingChunksModified || this.isIterating) { // Paper
@@ -643,7 +689,20 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
if (!this.updatingChunksModified) {
return false;
} else {
- this.visibleChunks = this.updatingChunks.clone();
+ // Paper start - stop cloning visibleChunks
+ synchronized (this.visibleChunks) {
+ this.visibleChunks.clear();
+ this.visibleChunks.putAll(this.updatingChunks);
+ this.visibleChunksClone = null;
+ if (isIterating) {
+ hasPendingVisibleUpdate = true;
+ this.pendingVisibleChunks.copyFrom(this.updatingChunks);
+ } else {
+ hasPendingVisibleUpdate = false;
+ this.pendingVisibleChunks.clear();
+ this.visibleChunks.copyFrom(this.updatingChunks);
+ this.visibleChunksClone = null;
+ }
+ }
+ // Paper end
+
this.updatingChunksModified = false;
return true;
}
@@ -1104,12 +1155,12 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
@@ -1104,12 +1163,12 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
}
protected Iterable<PlayerChunk> f() {

137
Spigot-Server-Patches/0462-Speed-up-processing-of-chunk-loads-and-generation.patch
View file

@ -1,4 +1,4 @@
From e245683a13582175b68dbfa9babb6bef2b3d2fd5 Mon Sep 17 00:00:00 2001
From 665aad14dce0eb101630440132805dccc9f2aefe Mon Sep 17 00:00:00 2001
From: Aikar <aikar@aikar.co>
Date: Thu, 9 Apr 2020 00:09:26 -0400
Subject: [PATCH] Speed up processing of chunk loads and generation
@ -44,46 +44,8 @@ index 69e26a8267..434833d50e 100644
public static final Timing playerListTimer = Timings.ofSafe("Player List");
public static final Timing commandFunctionsTimer = Timings.ofSafe("Command Functions");
public static final Timing connectionTimer = Timings.ofSafe("Connection Handler");
diff --git a/src/main/java/com/destroystokyo/paper/util/map/Long2ObjectLinkedOpenHashMapFastCopy.java b/src/main/java/com/destroystokyo/paper/util/map/Long2ObjectLinkedOpenHashMapFastCopy.java
new file mode 100644
index 0000000000..e0ad725b2e
--- /dev/null
+++ b/src/main/java/com/destroystokyo/paper/util/map/Long2ObjectLinkedOpenHashMapFastCopy.java
@@ -0,0 +1,32 @@
+package com.destroystokyo.paper.util.map;
+
+import it.unimi.dsi.fastutil.longs.Long2ObjectLinkedOpenHashMap;
+
+public class Long2ObjectLinkedOpenHashMapFastCopy<V> extends Long2ObjectLinkedOpenHashMap<V> {
+
+ public void copyFrom(Long2ObjectLinkedOpenHashMapFastCopy<V> map) {
+ if (key.length < map.key.length) {
+ key = null;
+ key = new long[map.key.length];
+ }
+ if (value.length < map.value.length) {
+ value = null;
+ //noinspection unchecked
+ value = (V[]) new Object[map.value.length];
+ }
+ if (link.length < map.link.length) {
+ link = null;
+ link = new long[map.link.length];
+ }
+ System.arraycopy(map.key, 0, this.key, 0, map.key.length);
+ System.arraycopy(map.value, 0, this.value, 0, map.value.length);
+ System.arraycopy(map.link, 0, this.link, 0, map.link.length);
+ this.size = map.size;
+ this.mask = map.mask;
+ this.first = map.first;
+ this.last = map.last;
+ this.n = map.n;
+ this.maxFill = map.maxFill;
+ this.containsNullKey = map.containsNullKey;
+ }
+}
diff --git a/src/main/java/net/minecraft/server/ChunkProviderServer.java b/src/main/java/net/minecraft/server/ChunkProviderServer.java
index e627440c41..15450f36a4 100644
index e627440c41..bacfc4cba6 100644
--- a/src/main/java/net/minecraft/server/ChunkProviderServer.java
+++ b/src/main/java/net/minecraft/server/ChunkProviderServer.java
@@ -602,6 +602,7 @@ public class ChunkProviderServer extends IChunkProvider {
@ -119,7 +81,7 @@ index e627440c41..15450f36a4 100644
}
}
});
@@ -893,6 +896,39 @@ public class ChunkProviderServer extends IChunkProvider {
@@ -893,6 +896,38 @@ public class ChunkProviderServer extends IChunkProvider {
super.executeTask(runnable);
}
@ -132,8 +94,7 @@ index e627440c41..15450f36a4 100644
+ try {
+ // always try to load chunks as long as we aren't falling behind, restrain generation/other updates only.
+ boolean execChunkTask = com.destroystokyo.paper.io.chunk.ChunkTaskManager.pollChunkWaitQueue() || ChunkProviderServer.this.world.asyncChunkTaskManager.pollNextChunkTask(); // Paper
+ ChunkProviderServer.this.tickDistanceManager();
+ if (execChunkTask) {
+ if (ChunkProviderServer.this.tickDistanceManager() || execChunkTask) {
+ continue;
+ }
+ long now = System.nanoTime();
@ -202,96 +163,10 @@ index 06c395000f..936434110c 100644
protected TickTask postToMainThread(Runnable runnable) {
return new TickTask(this.ticks, runnable);
diff --git a/src/main/java/net/minecraft/server/PlayerChunkMap.java b/src/main/java/net/minecraft/server/PlayerChunkMap.java
index ce645a69bd..32fed8a39a 100644
index e61ddeb1ff..92c9ab43d7 100644
--- a/src/main/java/net/minecraft/server/PlayerChunkMap.java
+++ b/src/main/java/net/minecraft/server/PlayerChunkMap.java
@@ -55,8 +55,9 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
private static final Logger LOGGER = LogManager.getLogger();
public static final int GOLDEN_TICKET = 33 + ChunkStatus.b();
- public final Long2ObjectLinkedOpenHashMap<PlayerChunk> updatingChunks = new Long2ObjectLinkedOpenHashMap();
- public final Long2ObjectLinkedOpenHashMap<PlayerChunk> visibleChunks = new Long2ObjectLinkedOpenHashMap(); // Paper - remove copying, make mt safe
+ public final com.destroystokyo.paper.util.map.Long2ObjectLinkedOpenHashMapFastCopy<PlayerChunk> updatingChunks = new com.destroystokyo.paper.util.map.Long2ObjectLinkedOpenHashMapFastCopy<>();
+ public final com.destroystokyo.paper.util.map.Long2ObjectLinkedOpenHashMapFastCopy<PlayerChunk> visibleChunks = new com.destroystokyo.paper.util.map.Long2ObjectLinkedOpenHashMapFastCopy<>(); // Paper - remove copying, make mt safe
+ public final com.destroystokyo.paper.util.map.Long2ObjectLinkedOpenHashMapFastCopy<PlayerChunk> pendingVisibleChunks = new com.destroystokyo.paper.util.map.Long2ObjectLinkedOpenHashMapFastCopy<>(); // Paper - remove copying, this is used if the visible chunks is updated while iterating only
public transient Long2ObjectLinkedOpenHashMap<PlayerChunk> visibleChunksClone; // Paper - remove copying, make mt safe
private final Long2ObjectLinkedOpenHashMap<PlayerChunk> pendingUnload;
final LongSet loadedChunks; // Paper - private -> package
@@ -266,10 +267,10 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
// Paper start - remove cloning of visible chunks unless accessed as a collection async
private static final boolean DEBUG_ASYNC_VISIBLE_CHUNKS = Boolean.getBoolean("paper.debug-async-visible-chunks");
private boolean isIterating = false;
+ private boolean hasPendingVisibleUpdate = false;
public void forEachVisibleChunk(java.util.function.Consumer<PlayerChunk> consumer) {
org.spigotmc.AsyncCatcher.catchOp("forEachVisibleChunk");
boolean prev = isIterating;
- boolean wasUpdating = updatingChunksModified;
isIterating = true;
try {
for (PlayerChunk value : this.visibleChunks.values()) {
@@ -277,8 +278,9 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
}
} finally {
this.isIterating = prev;
- if (!this.isIterating && updatingChunksModified && !wasUpdating) {
- this.updateVisibleChunks();
+ if (!this.isIterating && hasPendingVisibleUpdate) {
+ this.visibleChunks.copyFrom(this.pendingVisibleChunks);
+ this.pendingVisibleChunks.clear();
}
}
}
@@ -289,7 +291,7 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
synchronized (this.visibleChunks) {
if (DEBUG_ASYNC_VISIBLE_CHUNKS) new Throwable("Async getVisibleChunks").printStackTrace();
if (this.visibleChunksClone == null) {
- this.visibleChunksClone = this.visibleChunks.clone();
+ this.visibleChunksClone = this.hasPendingVisibleUpdate ? this.pendingVisibleChunks.clone() : this.visibleChunks.clone();
}
return this.visibleChunksClone;
}
@@ -302,11 +304,11 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
// Paper start - mt safe get
if (Thread.currentThread() != this.world.serverThread) {
synchronized (this.visibleChunks) {
- return (PlayerChunk) this.visibleChunks.get(i);
+ return (PlayerChunk) (this.hasPendingVisibleUpdate ? this.pendingVisibleChunks.get(i) : this.visibleChunks.get(i));
}
}
+ return (PlayerChunk) (this.hasPendingVisibleUpdate ? this.pendingVisibleChunks.get(i) : this.visibleChunks.get(i));
// Paper end
- return (PlayerChunk) this.visibleChunks.get(i);
}
protected IntSupplier c(long i) {
@@ -682,16 +684,21 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
});
}
- protected boolean updateVisibleChunks() { return b(); } // Paper - OBFHELPER
protected boolean b() {
- if (!this.updatingChunksModified || this.isIterating) { // Paper
+ if (!this.updatingChunksModified) {
return false;
} else {
// Paper start - stop cloning visibleChunks
synchronized (this.visibleChunks) {
- this.visibleChunks.clear();
- this.visibleChunks.putAll(this.updatingChunks);
- this.visibleChunksClone = null;
+ if (isIterating) {
+ hasPendingVisibleUpdate = true;
+ this.pendingVisibleChunks.copyFrom(this.updatingChunks);
+ } else {
+ hasPendingVisibleUpdate = false;
+ this.pendingVisibleChunks.clear();
+ this.visibleChunks.copyFrom(this.updatingChunks);
+ this.visibleChunksClone = null;
+ }
}
// Paper end
@@ -785,7 +792,7 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
@@ -793,7 +793,7 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
CompletableFuture<Either<IChunkAccess, PlayerChunk.Failure>> ret = new CompletableFuture<>();
Consumer<ChunkRegionLoader.InProgressChunkHolder> chunkHolderConsumer = (ChunkRegionLoader.InProgressChunkHolder holder) -> {