Improve Optimize Memory use logic to make iterator safer and fix bad plugins like P2

Accessing world state async is bad mkay. But we know you didn't do it city <3
This commit is contained in:
Aikar 2020-04-08 21:07:59 -04:00
parent d33ba160a4
commit 38c626229d
No known key found for this signature in database
GPG key ID: 401ADFC9891FAAFE

View file

@ -1,4 +1,4 @@
From d4a80e85b8785b079769629ecf004f73fde66efd Mon Sep 17 00:00:00 2001
From cee599dd7b5c962429d797a091ab740da42179a3 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
@ -12,7 +12,7 @@ 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/net/minecraft/server/ChunkProviderServer.java b/src/main/java/net/minecraft/server/ChunkProviderServer.java
index 1dcd0980ec..59055cccc5 100644
index 1dcd0980ec..e627440c41 100644
--- a/src/main/java/net/minecraft/server/ChunkProviderServer.java
+++ b/src/main/java/net/minecraft/server/ChunkProviderServer.java
@@ -669,7 +669,7 @@ public class ChunkProviderServer extends IChunkProvider {
@ -20,7 +20,7 @@ index 1dcd0980ec..59055cccc5 100644
};
// Paper end
- this.playerChunkMap.f().forEach((playerchunk) -> {
+ this.playerChunkMap.visibleChunks.values().forEach((playerchunk) -> { // Paper - no need to wrap iterator
+ this.playerChunkMap.forEachVisibleChunk((playerchunk) -> { // Paper - safe iterator incase chunk loads, also no wrapping
Optional<Chunk> optional = ((Either) playerchunk.b().getNow(PlayerChunk.UNLOADED_CHUNK)).left();
if (optional.isPresent()) {
@ -38,7 +38,7 @@ 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..0aa6487d5b 100644
index e1e4ea793a..ce645a69bd 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 {
@ -47,7 +47,7 @@ index e1e4ea793a..0aa6487d5b 100644
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 volatile Long2ObjectLinkedOpenHashMap<PlayerChunk> visibleChunksClone; // Paper - remove copying, make mt safe
+ public transient Long2ObjectLinkedOpenHashMap<PlayerChunk> visibleChunksClone; // Paper - remove copying, make mt safe
private final Long2ObjectLinkedOpenHashMap<PlayerChunk> pendingUnload;
final LongSet loadedChunks; // Paper - private -> package
public final WorldServer world;
@ -60,12 +60,29 @@ index e1e4ea793a..0aa6487d5b 100644
this.pendingUnload = new Long2ObjectLinkedOpenHashMap();
this.loadedChunks = new LongOpenHashSet();
this.unloadQueue = new LongOpenHashSet();
@@ -262,8 +263,32 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
@@ -262,8 +263,49 @@ 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;
+ 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()) {
+ consumer.accept(value);
+ }
+ } finally {
+ this.isIterating = prev;
+ if (!this.isIterating && updatingChunksModified && !wasUpdating) {
+ this.updateVisibleChunks();
+ }
+ }
+ }
+ public Long2ObjectLinkedOpenHashMap<PlayerChunk> getVisibleChunks() {
+ if (Thread.currentThread() == this.world.serverThread) {
+ return this.visibleChunks;
@ -93,7 +110,7 @@ index e1e4ea793a..0aa6487d5b 100644
return (PlayerChunk) this.visibleChunks.get(i);
}
@@ -444,8 +469,9 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
@@ -444,8 +486,9 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
// Paper end
protected void save(boolean flag) {
@ -104,7 +121,7 @@ index e1e4ea793a..0aa6487d5b 100644
MutableBoolean mutableboolean = new MutableBoolean();
do {
@@ -473,7 +499,7 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
@@ -473,7 +516,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 {
@ -113,8 +130,14 @@ index e1e4ea793a..0aa6487d5b 100644
IChunkAccess ichunkaccess = (IChunkAccess) playerchunk.getChunkSave().getNow(null); // CraftBukkit - decompile error
if (ichunkaccess instanceof ProtoChunkExtension || ichunkaccess instanceof Chunk) {
@@ -643,7 +669,14 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
if (!this.updatingChunksModified) {
@@ -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
return false;
} else {
- this.visibleChunks = this.updatingChunks.clone();
@ -129,7 +152,7 @@ index e1e4ea793a..0aa6487d5b 100644
this.updatingChunksModified = false;
return true;
}
@@ -1104,12 +1137,12 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
@@ -1104,12 +1155,12 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
}
protected Iterable<PlayerChunk> f() {
@ -145,30 +168,60 @@ index e1e4ea793a..0aa6487d5b 100644
while (objectbidirectionaliterator.hasNext()) {
Entry<PlayerChunk> entry = (Entry) objectbidirectionaliterator.next();
diff --git a/src/main/java/org/bukkit/craftbukkit/CraftWorld.java b/src/main/java/org/bukkit/craftbukkit/CraftWorld.java
index 051506fce8..c0e8eb85d7 100644
index 051506fce8..630d6470a4 100644
--- a/src/main/java/org/bukkit/craftbukkit/CraftWorld.java
+++ b/src/main/java/org/bukkit/craftbukkit/CraftWorld.java
@@ -290,6 +290,7 @@ public class CraftWorld implements World {
@@ -74,6 +74,7 @@ import net.minecraft.server.GameRules;
import net.minecraft.server.GroupDataEntity;
import net.minecraft.server.IBlockData;
import net.minecraft.server.IChunkAccess;
+import net.minecraft.server.MCUtil;
import net.minecraft.server.MinecraftKey;
import net.minecraft.server.MovingObjectPosition;
import net.minecraft.server.PacketPlayOutCustomSoundEffect;
@@ -290,6 +291,7 @@ public class CraftWorld implements World {
return ret;
}
public int getTileEntityCount() {
+ org.spigotmc.AsyncCatcher.catchOp("getTileEntityCount");
+ return MCUtil.ensureMain(() -> {
// We don't use the full world tile entity list, so we must iterate chunks
Long2ObjectLinkedOpenHashMap<PlayerChunk> chunks = world.getChunkProvider().playerChunkMap.visibleChunks;
int size = 0;
@@ -306,6 +307,7 @@ public class CraftWorld implements World {
@@ -301,11 +303,13 @@ public class CraftWorld implements World {
size += chunk.tileEntities.size();
}
return size;
+ });
}
public int getTickableTileEntityCount() {
return world.tileEntityListTick.size();
}
public int getChunkCount() {
+ org.spigotmc.AsyncCatcher.catchOp("getChunkCount");
+ return MCUtil.ensureMain(() -> {
int ret = 0;
for (PlayerChunk chunkHolder : world.getChunkProvider().playerChunkMap.visibleChunks.values()) {
@@ -432,6 +434,7 @@ public class CraftWorld implements World {
@@ -314,7 +318,7 @@ public class CraftWorld implements World {
}
}
- return ret;
+ return ret; });
}
public int getPlayerCount() {
return world.players.size();
@@ -432,6 +436,14 @@ public class CraftWorld implements World {
@Override
public Chunk[] getLoadedChunks() {
+ org.spigotmc.AsyncCatcher.catchOp("getLoadedChunks"); // Paper
+ // Paper start
+ if (Thread.currentThread() != world.getMinecraftWorld().serverThread) {
+ synchronized (world.getChunkProvider().playerChunkMap.visibleChunks) {
+ Long2ObjectLinkedOpenHashMap<PlayerChunk> chunks = world.getChunkProvider().playerChunkMap.visibleChunks;
+ return chunks.values().stream().map(PlayerChunk::getFullChunk).filter(Objects::nonNull).map(net.minecraft.server.Chunk::getBukkitChunk).toArray(Chunk[]::new);
+ }
+ }
+ // Paper end
Long2ObjectLinkedOpenHashMap<PlayerChunk> chunks = world.getChunkProvider().playerChunkMap.visibleChunks;
return chunks.values().stream().map(PlayerChunk::getFullChunk).filter(Objects::nonNull).map(net.minecraft.server.Chunk::getBukkitChunk).toArray(Chunk[]::new);
}