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 00570dc4bb
commit 587c1e02a5

View file

@ -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. 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 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 --- a/src/main/java/net/minecraft/server/ChunkProviderServer.java
+++ b/src/main/java/net/minecraft/server/ChunkProviderServer.java +++ b/src/main/java/net/minecraft/server/ChunkProviderServer.java
@@ -0,0 +0,0 @@ public class ChunkProviderServer extends IChunkProvider { @@ -0,0 +0,0 @@ public class ChunkProviderServer extends IChunkProvider {
@ -20,7 +20,7 @@ index 1dcd0980ec..59055cccc5 100644
}; };
// Paper end // Paper end
- this.playerChunkMap.f().forEach((playerchunk) -> { - 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(); Optional<Chunk> optional = ((Either) playerchunk.b().getNow(PlayerChunk.UNLOADED_CHUNK)).left();
if (optional.isPresent()) { if (optional.isPresent()) {
@ -38,7 +38,7 @@ index b9d5844520..9980e4c277 100644
List<PlayerChunk> allChunks = new ArrayList<>(visibleChunks.values()); List<PlayerChunk> allChunks = new ArrayList<>(visibleChunks.values());
List<EntityPlayer> players = world.players; 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 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 --- a/src/main/java/net/minecraft/server/PlayerChunkMap.java
+++ b/src/main/java/net/minecraft/server/PlayerChunkMap.java +++ b/src/main/java/net/minecraft/server/PlayerChunkMap.java
@@ -0,0 +0,0 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d { @@ -0,0 +0,0 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
@ -47,7 +47,7 @@ index e1e4ea793a..0aa6487d5b 100644
public final Long2ObjectLinkedOpenHashMap<PlayerChunk> updatingChunks = new Long2ObjectLinkedOpenHashMap(); public final Long2ObjectLinkedOpenHashMap<PlayerChunk> updatingChunks = new Long2ObjectLinkedOpenHashMap();
- public volatile Long2ObjectLinkedOpenHashMap<PlayerChunk> visibleChunks; - public volatile Long2ObjectLinkedOpenHashMap<PlayerChunk> visibleChunks;
+ public final Long2ObjectLinkedOpenHashMap<PlayerChunk> visibleChunks = new Long2ObjectLinkedOpenHashMap(); // Paper - remove copying, make mt safe + 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; private final Long2ObjectLinkedOpenHashMap<PlayerChunk> pendingUnload;
final LongSet loadedChunks; // Paper - private -> package final LongSet loadedChunks; // Paper - private -> package
public final WorldServer world; public final WorldServer world;
@ -66,6 +66,23 @@ index e1e4ea793a..0aa6487d5b 100644
+ // Paper start - remove cloning of visible chunks unless accessed as a collection async + // 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 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() { + public Long2ObjectLinkedOpenHashMap<PlayerChunk> getVisibleChunks() {
+ if (Thread.currentThread() == this.world.serverThread) { + if (Thread.currentThread() == this.world.serverThread) {
+ return this.visibleChunks; + return this.visibleChunks;
@ -114,7 +131,13 @@ index e1e4ea793a..0aa6487d5b 100644
if (ichunkaccess instanceof ProtoChunkExtension || ichunkaccess instanceof Chunk) { if (ichunkaccess instanceof ProtoChunkExtension || ichunkaccess instanceof Chunk) {
@@ -0,0 +0,0 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d { @@ -0,0 +0,0 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
if (!this.updatingChunksModified) { });
}
+ protected boolean updateVisibleChunks() { return b(); } // Paper - OBFHELPER
protected boolean b() {
- if (!this.updatingChunksModified) {
+ if (!this.updatingChunksModified || this.isIterating) { // Paper
return false; return false;
} else { } else {
- this.visibleChunks = this.updatingChunks.clone(); - this.visibleChunks = this.updatingChunks.clone();
@ -145,30 +168,60 @@ index e1e4ea793a..0aa6487d5b 100644
while (objectbidirectionaliterator.hasNext()) { while (objectbidirectionaliterator.hasNext()) {
Entry<PlayerChunk> entry = (Entry) objectbidirectionaliterator.next(); 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 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 --- a/src/main/java/org/bukkit/craftbukkit/CraftWorld.java
+++ b/src/main/java/org/bukkit/craftbukkit/CraftWorld.java +++ b/src/main/java/org/bukkit/craftbukkit/CraftWorld.java
@@ -0,0 +0,0 @@ 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;
@@ -0,0 +0,0 @@ public class CraftWorld implements World { @@ -0,0 +0,0 @@ public class CraftWorld implements World {
return ret; return ret;
} }
public int getTileEntityCount() { 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 // We don't use the full world tile entity list, so we must iterate chunks
Long2ObjectLinkedOpenHashMap<PlayerChunk> chunks = world.getChunkProvider().playerChunkMap.visibleChunks; Long2ObjectLinkedOpenHashMap<PlayerChunk> chunks = world.getChunkProvider().playerChunkMap.visibleChunks;
int size = 0; int size = 0;
@@ -0,0 +0,0 @@ public class CraftWorld implements World { @@ -0,0 +0,0 @@ public class CraftWorld implements World {
size += chunk.tileEntities.size();
}
return size;
+ });
}
public int getTickableTileEntityCount() {
return world.tileEntityListTick.size(); return world.tileEntityListTick.size();
} }
public int getChunkCount() { public int getChunkCount() {
+ org.spigotmc.AsyncCatcher.catchOp("getChunkCount"); + return MCUtil.ensureMain(() -> {
int ret = 0; int ret = 0;
for (PlayerChunk chunkHolder : world.getChunkProvider().playerChunkMap.visibleChunks.values()) { for (PlayerChunk chunkHolder : world.getChunkProvider().playerChunkMap.visibleChunks.values()) {
@@ -0,0 +0,0 @@ public class CraftWorld implements World { @@ -0,0 +0,0 @@ public class CraftWorld implements World {
}
}
- return ret;
+ return ret; });
}
public int getPlayerCount() {
return world.players.size();
@@ -0,0 +0,0 @@ public class CraftWorld implements World {
@Override @Override
public Chunk[] getLoadedChunks() { 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; 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); return chunks.values().stream().map(PlayerChunk::getFullChunk).filter(Objects::nonNull).map(net.minecraft.server.Chunk::getBukkitChunk).toArray(Chunk[]::new);
} }