From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Aikar Date: Wed, 8 Apr 2020 03:06:30 -0400 Subject: [PATCH] Optimize PlayerChunkMap memory use for visibleChunks No longer clones visible chunks which is causing massive memory allocation issues, likely the source of Humongous Objects on large servers. Instead we just synchronize, clear and rebuild, reusing the same object buffers 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 0000000000000000000000000000000000000000..f6ff4d8132a95895680f5bc81f8f873e78f0bbdb --- /dev/null +++ b/src/main/java/com/destroystokyo/paper/util/map/Long2ObjectLinkedOpenHashMapFastCopy.java @@ -0,0 +1,39 @@ +package com.destroystokyo.paper.util.map; + +import it.unimi.dsi.fastutil.longs.Long2ObjectLinkedOpenHashMap; + +public class Long2ObjectLinkedOpenHashMapFastCopy extends Long2ObjectLinkedOpenHashMap { + + public void copyFrom(Long2ObjectLinkedOpenHashMapFastCopy 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; + } + + @Override + public Long2ObjectLinkedOpenHashMapFastCopy clone() { + Long2ObjectLinkedOpenHashMapFastCopy clone = (Long2ObjectLinkedOpenHashMapFastCopy) super.clone(); + clone.copyFrom(this); + return clone; + } +} diff --git a/src/main/java/net/minecraft/server/ChunkProviderServer.java b/src/main/java/net/minecraft/server/ChunkProviderServer.java index ba9f75bd8f6fe1990d485548f4481bd1762d93af..e14e8bcf235339c1537a1e0a7702a364ee784c93 100644 --- a/src/main/java/net/minecraft/server/ChunkProviderServer.java +++ b/src/main/java/net/minecraft/server/ChunkProviderServer.java @@ -756,7 +756,7 @@ public class ChunkProviderServer extends IChunkProvider { entityPlayer.playerNaturallySpawnedEvent.callEvent(); }; // Paper end - this.playerChunkMap.f().forEach((playerchunk) -> { // Paper - no... just no... + this.playerChunkMap.forEachVisibleChunk((playerchunk) -> { // Paper - safe iterator incase chunk loads, also no wrapping Optional optional = ((Either) playerchunk.a().getNow(PlayerChunk.UNLOADED_CHUNK)).left(); if (optional.isPresent()) { diff --git a/src/main/java/net/minecraft/server/MCUtil.java b/src/main/java/net/minecraft/server/MCUtil.java index b2e8ddc9ff1bf5f519d971455d48a2faad3638f2..2f9c014454cf5fe771c6da84ad4af7e7790fdc7d 100644 --- a/src/main/java/net/minecraft/server/MCUtil.java +++ b/src/main/java/net/minecraft/server/MCUtil.java @@ -598,7 +598,7 @@ public final class MCUtil { WorldServer world = ((org.bukkit.craftbukkit.CraftWorld)bukkitWorld).getHandle(); PlayerChunkMap chunkMap = world.getChunkProvider().playerChunkMap; - Long2ObjectLinkedOpenHashMap visibleChunks = chunkMap.visibleChunks; + Long2ObjectLinkedOpenHashMap visibleChunks = chunkMap.getVisibleChunks(); ChunkMapDistance chunkMapDistance = chunkMap.getChunkMapDistanceManager(); List allChunks = new ArrayList<>(visibleChunks.values()); List players = world.players; diff --git a/src/main/java/net/minecraft/server/PlayerChunkMap.java b/src/main/java/net/minecraft/server/PlayerChunkMap.java index c9be4ae99458863bf91687c3667d67bc6b37b0f0..9d4c5f32036118bf7c5081c55bea71efd606b8f6 100644 --- a/src/main/java/net/minecraft/server/PlayerChunkMap.java +++ b/src/main/java/net/minecraft/server/PlayerChunkMap.java @@ -56,8 +56,33 @@ 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 updatingChunks = new Long2ObjectLinkedOpenHashMap(); - public volatile Long2ObjectLinkedOpenHashMap visibleChunks; + // Paper start - faster copying + public final Long2ObjectLinkedOpenHashMap updatingChunks = new com.destroystokyo.paper.util.map.Long2ObjectLinkedOpenHashMapFastCopy<>(); // Paper - faster copying + public final Long2ObjectLinkedOpenHashMap visibleChunks = new ProtectedVisibleChunksMap(); // Paper - faster copying + + private class ProtectedVisibleChunksMap extends com.destroystokyo.paper.util.map.Long2ObjectLinkedOpenHashMapFastCopy { + @Override + public PlayerChunk put(long k, PlayerChunk playerChunk) { + throw new UnsupportedOperationException("Updating visible Chunks"); + } + + @Override + public PlayerChunk remove(long k) { + throw new UnsupportedOperationException("Removing visible Chunks"); + } + + @Override + public PlayerChunk get(long k) { + return PlayerChunkMap.this.getVisibleChunk(k); + } + + public PlayerChunk safeGet(long k) { + return super.get(k); + } + } + // Paper end + public final com.destroystokyo.paper.util.map.Long2ObjectLinkedOpenHashMapFastCopy pendingVisibleChunks = new com.destroystokyo.paper.util.map.Long2ObjectLinkedOpenHashMapFastCopy(); // Paper - this is used if the visible chunks is updated while iterating only + public transient com.destroystokyo.paper.util.map.Long2ObjectLinkedOpenHashMapFastCopy visibleChunksClone; // Paper - used for async access of visible chunks, clone and cache only when needed private final Long2ObjectLinkedOpenHashMap pendingUnload; final LongSet loadedChunks; // Paper - private -> package public final WorldServer world; @@ -130,7 +155,7 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d { public PlayerChunkMap(WorldServer worldserver, Convertable.ConversionSession convertable_conversionsession, DataFixer datafixer, DefinedStructureManager definedstructuremanager, Executor executor, IAsyncTaskHandler iasynctaskhandler, ILightAccess ilightaccess, ChunkGenerator chunkgenerator, WorldLoadListener worldloadlistener, Supplier supplier, int i, boolean flag) { super(new File(convertable_conversionsession.a(worldserver.getDimensionKey()), "region"), datafixer, flag); - this.visibleChunks = this.updatingChunks.clone(); + //this.visibleChunks = this.updatingChunks.clone(); // Paper - no more cloning this.pendingUnload = new Long2ObjectLinkedOpenHashMap(); this.loadedChunks = new LongOpenHashSet(); this.unloadQueue = new LongOpenHashSet(); @@ -222,9 +247,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 consumer) { + org.spigotmc.AsyncCatcher.catchOp("forEachVisibleChunk"); + boolean prev = isIterating; + isIterating = true; + try { + for (PlayerChunk value : this.visibleChunks.values()) { + consumer.accept(value); + } + } finally { + this.isIterating = prev; + if (!this.isIterating && this.hasPendingVisibleUpdate) { + ((ProtectedVisibleChunksMap)this.visibleChunks).copyFrom(this.pendingVisibleChunks); + this.pendingVisibleChunks.clear(); + this.hasPendingVisibleUpdate = false; + } + } + } + public Long2ObjectLinkedOpenHashMap getVisibleChunks() { + if (Thread.currentThread() == this.world.serverThread) { + return this.visibleChunks; + } else { + synchronized (this.visibleChunks) { + if (DEBUG_ASYNC_VISIBLE_CHUNKS) new Throwable("Async getVisibleChunks").printStackTrace(); + if (this.visibleChunksClone == null) { + this.visibleChunksClone = this.hasPendingVisibleUpdate ? this.pendingVisibleChunks.clone() : ((ProtectedVisibleChunksMap)this.visibleChunks).clone(); + } + return this.visibleChunksClone; + } + } + } + // Paper end + @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.hasPendingVisibleUpdate ? this.pendingVisibleChunks.get(i) : ((ProtectedVisibleChunksMap)this.visibleChunks).safeGet(i)); + } + } + return (PlayerChunk) (this.hasPendingVisibleUpdate ? this.pendingVisibleChunks.get(i) : ((ProtectedVisibleChunksMap)this.visibleChunks).safeGet(i)); + // Paper end } protected IntSupplier c(long i) { @@ -412,8 +480,9 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d { // Paper end protected void save(boolean flag) { + Long2ObjectLinkedOpenHashMap visibleChunks = this.getVisibleChunks(); // Paper remove clone of visible Chunks unless saving off main thread (watchdog kill) if (flag) { - List list = (List) this.visibleChunks.values().stream().filter(PlayerChunk::hasBeenLoaded).peek(PlayerChunk::m).collect(Collectors.toList()); + List list = (List) visibleChunks.values().stream().filter(PlayerChunk::hasBeenLoaded).peek(PlayerChunk::m).collect(Collectors.toList()); // Paper - remove cloning of visible chunks MutableBoolean mutableboolean = new MutableBoolean(); do { @@ -441,7 +510,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 { - this.visibleChunks.values().stream().filter(PlayerChunk::hasBeenLoaded).forEach((playerchunk) -> { + visibleChunks.values().stream().filter(PlayerChunk::hasBeenLoaded).forEach((playerchunk) -> { IChunkAccess ichunkaccess = (IChunkAccess) playerchunk.getChunkSave().getNow(null); // CraftBukkit - decompile error if (ichunkaccess instanceof ProtoChunkExtension || ichunkaccess instanceof Chunk) { @@ -612,7 +681,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) { + if (isIterating) { + hasPendingVisibleUpdate = true; + this.pendingVisibleChunks.copyFrom((com.destroystokyo.paper.util.map.Long2ObjectLinkedOpenHashMapFastCopy)this.updatingChunks); + } else { + hasPendingVisibleUpdate = false; + this.pendingVisibleChunks.clear(); + ((ProtectedVisibleChunksMap)this.visibleChunks).copyFrom((com.destroystokyo.paper.util.map.Long2ObjectLinkedOpenHashMapFastCopy)this.updatingChunks); + this.visibleChunksClone = null; + } + } + // Paper end + this.updatingChunksModified = false; return true; } @@ -1080,12 +1162,12 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d { } protected Iterable f() { - return Iterables.unmodifiableIterable(this.visibleChunks.values()); + return Iterables.unmodifiableIterable(this.getVisibleChunks().values()); // Paper } void a(Writer writer) throws IOException { CSVWriter csvwriter = CSVWriter.a().a("x").a("z").a("level").a("in_memory").a("status").a("full_status").a("accessible_ready").a("ticking_ready").a("entity_ticking_ready").a("ticket").a("spawning").a("entity_count").a("block_entity_count").a(writer); - ObjectBidirectionalIterator objectbidirectionaliterator = this.visibleChunks.long2ObjectEntrySet().iterator(); + ObjectBidirectionalIterator objectbidirectionaliterator = this.getVisibleChunks().long2ObjectEntrySet().iterator(); // Paper while (objectbidirectionaliterator.hasNext()) { Entry 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 84c6dfcd87d9d893e846c08c9d14e01fbfb15fb2..e044534d3cb47cf1228c5e5fa8920df8264fd0b5 100644 --- a/src/main/java/org/bukkit/craftbukkit/CraftWorld.java +++ b/src/main/java/org/bukkit/craftbukkit/CraftWorld.java @@ -73,6 +73,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.MinecraftServer; import net.minecraft.server.MovingObjectPosition; @@ -292,6 +293,7 @@ public class CraftWorld implements World { return ret; } public int getTileEntityCount() { + return MCUtil.ensureMain(() -> { // We don't use the full world tile entity list, so we must iterate chunks Long2ObjectLinkedOpenHashMap chunks = world.getChunkProvider().playerChunkMap.visibleChunks; int size = 0; @@ -303,11 +305,13 @@ public class CraftWorld implements World { size += chunk.tileEntities.size(); } return size; + }); } public int getTickableTileEntityCount() { return world.tileEntityListTick.size(); } public int getChunkCount() { + return MCUtil.ensureMain(() -> { int ret = 0; for (PlayerChunk chunkHolder : world.getChunkProvider().playerChunkMap.visibleChunks.values()) { @@ -316,7 +320,7 @@ public class CraftWorld implements World { } } - return ret; + return ret; }); } public int getPlayerCount() { return world.players.size(); @@ -436,6 +440,14 @@ public class CraftWorld implements World { @Override public Chunk[] getLoadedChunks() { + // Paper start + if (Thread.currentThread() != world.getMinecraftWorld().serverThread) { + synchronized (world.getChunkProvider().playerChunkMap.visibleChunks) { + Long2ObjectLinkedOpenHashMap 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 chunks = world.getChunkProvider().playerChunkMap.visibleChunks; return chunks.values().stream().map(PlayerChunk::getFullChunk).filter(Objects::nonNull).map(net.minecraft.server.Chunk::getBukkitChunk).toArray(Chunk[]::new); }