From 55b59db2a3d24f7f3c8990389b6282f01cbe0a7c Mon Sep 17 00:00:00 2001 From: Aikar Date: Sun, 16 Sep 2018 00:00:16 -0400 Subject: [PATCH] Optimize and Fix ExpiringMap Issues computeIfAbsent would leak as the entry was never registered into the ttl map, as well as some other methods were at risk, so they were added. This also synchronizes all access make the map thread safe. This also redesigns cleaning to not run on every manipulation, and instead to run clean once per tick per active expiring map. diff --git a/src/main/java/net/minecraft/server/ChunkGeneratorAbstract.java b/src/main/java/net/minecraft/server/ChunkGeneratorAbstract.java index 39a548881..37ea37de5 100644 --- a/src/main/java/net/minecraft/server/ChunkGeneratorAbstract.java +++ b/src/main/java/net/minecraft/server/ChunkGeneratorAbstract.java @@ -143,13 +143,13 @@ public abstract class ChunkGeneratorAbstract implem public Long2ObjectMap getStructureStartCache(StructureGenerator structuregenerator) { return (Long2ObjectMap) this.d.computeIfAbsent(structuregenerator, (s) -> { - return Long2ObjectMaps.synchronize(new ExpiringMap(8192, 10000)); + return new ExpiringMap(8192, 10000); // Paper - already synchronized }); } public Long2ObjectMap getStructureCache(StructureGenerator structuregenerator) { return (Long2ObjectMap) this.e.computeIfAbsent(structuregenerator, (s) -> { - return Long2ObjectMaps.synchronize(new ExpiringMap(8192, 10000)); + return new ExpiringMap(8192, 10000); // Paper - already synchronized }); } diff --git a/src/main/java/net/minecraft/server/ExpiringMap.java b/src/main/java/net/minecraft/server/ExpiringMap.java index 9c0518d67..84a6dcb40 100644 --- a/src/main/java/net/minecraft/server/ExpiringMap.java +++ b/src/main/java/net/minecraft/server/ExpiringMap.java @@ -2,94 +2,201 @@ package net.minecraft.server; import it.unimi.dsi.fastutil.longs.Long2LongLinkedOpenHashMap; import it.unimi.dsi.fastutil.longs.Long2LongMap; +import it.unimi.dsi.fastutil.longs.Long2ObjectMaps; import it.unimi.dsi.fastutil.longs.Long2ObjectOpenHashMap; -import it.unimi.dsi.fastutil.longs.Long2LongMap.Entry; import it.unimi.dsi.fastutil.objects.ObjectCollection; import it.unimi.dsi.fastutil.objects.ObjectIterator; import java.util.Map; +import java.util.function.BiFunction; +import java.util.function.Function; import java.util.function.LongFunction; -public class ExpiringMap extends Long2ObjectOpenHashMap { - +public class ExpiringMap extends Long2ObjectMaps.SynchronizedMap { // paper - synchronize accesss private final int a; - private final Long2LongMap b = new Long2LongLinkedOpenHashMap(); + private final Long2LongMap ttl = new Long2LongLinkedOpenHashMap(); // Paper + private static final boolean DEBUG_EXPIRING_MAP = Boolean.getBoolean("debug.expiringmap"); public ExpiringMap(int i, int j) { - super(i); + super(new Long2ObjectOpenHashMap<>(i)); // Paper this.a = j; } + // Paper start + private void setAccess(long i) { a(i); } // Paper - OBFHELPER private void a(long i) { - long j = SystemUtils.getMonotonicMillis(); + synchronized (this.sync) { + long j = System.currentTimeMillis(); // Paper + this.ttl.put(i, j); + if (!registered) { + registered = true; + MinecraftServer.getServer().expiringMaps.add(this); + } + } + } - this.b.put(i, j); - cleanup(); - // CraftBukkit start + @Override + public T compute(long l, BiFunction biFunction) { + setAccess(l); + return super.compute(l, biFunction); } - public void cleanup() { - long j = SystemUtils.getMonotonicMillis(); - ObjectIterator objectiterator = this.b.long2LongEntrySet().iterator(); // CraftBukkit - decompile error + @Override + public T putIfAbsent(long l, T t) { + setAccess(l); + return super.putIfAbsent(l, t); + } - while (objectiterator.hasNext()) { - Long2LongMap.Entry entry = (Long2LongMap.Entry) objectiterator.next(); // CraftBukkit - decompile error - T object = super.get(entry.getLongKey()); // CraftBukkit - decompile error + @Override + public T computeIfPresent(long l, BiFunction biFunction) { + setAccess(l); + return super.computeIfPresent(l, biFunction); + } - if (j - entry.getLongValue() <= (long) this.a) { - break; - } + @Override + public T computeIfAbsent(long l, LongFunction longFunction) { + setAccess(l); + return super.computeIfAbsent(l, longFunction); + } - if (object != null && this.a(object)) { - super.remove(entry.getLongKey()); - objectiterator.remove(); - } + @Override + public boolean replace(long l, T t, T v1) { + setAccess(l); + return super.replace(l, t, v1); + } + + @Override + public T replace(long l, T t) { + setAccess(l); + return super.replace(l, t); + } + + @Override + public T putIfAbsent(Long aLong, T t) { + setAccess(aLong); + return super.putIfAbsent(aLong, t); + } + + @Override + public boolean replace(Long aLong, T t, T v1) { + setAccess(aLong); + return super.replace(aLong, t, v1); + } + + @Override + public T replace(Long aLong, T t) { + setAccess(aLong); + return super.replace(aLong, t); + } + + @Override + public T computeIfAbsent(Long aLong, Function function) { + setAccess(aLong); + return super.computeIfAbsent(aLong, function); + } + + @Override + public T computeIfPresent(Long aLong, BiFunction biFunction) { + setAccess(aLong); + return super.computeIfPresent(aLong, biFunction); + } + + @Override + public T compute(Long aLong, BiFunction biFunction) { + setAccess(aLong); + return super.compute(aLong, biFunction); + } + + @Override + public void clear() { + synchronized (this.sync) { + ttl.clear(); + super.clear(); } - // CraftBukkit end + } + private boolean registered = false; + + // Break clean to its own method to be ticked + boolean clean() { + synchronized (this.sync) { + long now = System.currentTimeMillis(); + ObjectIterator objectiterator = this.ttl.long2LongEntrySet().iterator(); // Paper + + while (objectiterator.hasNext()) { + Long2LongMap.Entry entry = objectiterator.next(); // Paper + T object = super.get(entry.getLongKey()); // Paper + if (now - entry.getLongValue() <= (long) this.a) { + break; + } + + if (object != null && this.a(object)) { + super.remove(entry.getLongKey()); + objectiterator.remove(); + } + } + int ttlSize = this.ttl.size(); + int thisSize = this.size(); + if (ttlSize < thisSize) { + if (DEBUG_EXPIRING_MAP) { + MinecraftServer.LOGGER.warn("WARNING: ExpiringMap desync (ttl:" + ttlSize + " < actual:" + thisSize + ")"); + } + try { + for (Entry entry : this.long2ObjectEntrySet()) { + ttl.putIfAbsent(entry.getLongKey(), now); + } + } catch (Exception ignored) { + } // Ignore any como's + } else if (ttlSize > this.size()) { + if (DEBUG_EXPIRING_MAP) { + MinecraftServer.LOGGER.warn("WARNING: ExpiringMap desync (ttl:" + ttlSize + " > actual:" + thisSize + ")"); + } + try { + this.ttl.long2LongEntrySet().removeIf(entry -> !this.containsKey(entry.getLongKey())); + } catch (Exception ignored) { + } // Ignore any como's + } + if (isEmpty()) { + registered = false; + return true; + } + return false; + } + // Paper end } - protected boolean a(T t0) { + protected boolean a(T var1) { return true; } - public T put(long i, T t0) { + public T put(long i, T object) { this.a(i); - return super.put(i, t0); + return (T)super.put(i, object); } - public T put(Long olong, T t0) { + public T put(Long olong, T object) { this.a(olong); - return super.put(olong, t0); + return (T)super.put(olong, object); } public T get(long i) { - this.a(i); - return super.get(i); + // Paper start - don't setAccess unless a hit + T t = super.get(i); + if (t != null) { + this.setAccess(i); + } + return t; + // Paper end } - public void putAll(Map map) { + public void putAll(Map var1) { throw new RuntimeException("Not implemented"); } - public T remove(long i) { + public T remove(long var1) { throw new RuntimeException("Not implemented"); } - public T remove(Object object) { + public T remove(Object var1) { throw new RuntimeException("Not implemented"); } - - // CraftBukkit start - @Override - public T computeIfAbsent(long l, LongFunction lf) { - this.b.put(l, SystemUtils.getMonotonicMillis()); - return super.computeIfAbsent(l, lf); - } - - @Override - public ObjectCollection values() { - cleanup(); - return super.values(); - } - // CraftBukkit end } diff --git a/src/main/java/net/minecraft/server/MinecraftServer.java b/src/main/java/net/minecraft/server/MinecraftServer.java index aaefb98c0..0bc1c887a 100644 --- a/src/main/java/net/minecraft/server/MinecraftServer.java +++ b/src/main/java/net/minecraft/server/MinecraftServer.java @@ -155,6 +155,7 @@ public abstract class MinecraftServer implements IAsyncTaskHandler, IMojangStati public int autosavePeriod; public File bukkitDataPackFolder; public CommandDispatcher vanillaCommandDispatcher; + public List expiringMaps = java.util.Collections.synchronizedList(new java.util.ArrayList<>()); // Paper // CraftBukkit end // Spigot start public static final int TPS = 20; @@ -999,6 +1000,7 @@ public abstract class MinecraftServer implements IAsyncTaskHandler, IMojangStati this.methodProfiler.exit(); org.spigotmc.WatchdogThread.tick(); // Spigot PaperLightingQueue.processQueue(startTime); // Paper + expiringMaps.removeIf(ExpiringMap::clean); // Paper this.slackActivityAccountant.tickEnded(l); // Spigot co.aikar.timings.TimingsManager.FULL_SERVER_TICK.stopTiming(); // Paper } diff --git a/src/main/java/org/bukkit/craftbukkit/generator/CustomChunkGenerator.java b/src/main/java/org/bukkit/craftbukkit/generator/CustomChunkGenerator.java index 4cca65666..9c917370a 100644 --- a/src/main/java/org/bukkit/craftbukkit/generator/CustomChunkGenerator.java +++ b/src/main/java/org/bukkit/craftbukkit/generator/CustomChunkGenerator.java @@ -144,7 +144,7 @@ public class CustomChunkGenerator extends InternalChunkGenerator getStructureStartCache(StructureGenerator structuregenerator) { return (Long2ObjectMap) this.structureStartCache.computeIfAbsent(structuregenerator, (s) -> { - return Long2ObjectMaps.synchronize(new ExpiringMap(8192, 10000)); + return new ExpiringMap(8192, 10000); // Paper - already synchronized }); } @@ -154,7 +154,7 @@ public class CustomChunkGenerator extends InternalChunkGenerator getStructureCache(StructureGenerator structuregenerator) { return (Long2ObjectMap) this.structureCache.computeIfAbsent(structuregenerator, (s) -> { - return Long2ObjectMaps.synchronize(new ExpiringMap(8192, 10000)); + return new ExpiringMap(8192, 10000); // Paper - already synchronized }); } -- 2.20.0