Paper/Spigot-Server-Patches/0372-Optimize-and-Fix-ExpiringMap-Issues.patch

From e9f487b844501e35de9892cd88dfb2c85835b8f9 Mon Sep 17 00:00:00 2001
From: Aikar <aikar@aikar.co>
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/ExpiringMap.java b/src/main/java/net/minecraft/server/ExpiringMap.java
index 4006f5a69c..9a3b35ea1a 100644
--- a/src/main/java/net/minecraft/server/ExpiringMap.java
+++ b/src/main/java/net/minecraft/server/ExpiringMap.java
@@ -2,29 +2,124 @@ 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.ObjectIterator;
 import java.util.Map;
+import java.util.function.BiFunction;
+import java.util.function.Function;
+import java.util.function.LongFunction;
 
-public class ExpiringMap<T> extends Long2ObjectOpenHashMap<T> {
+public class ExpiringMap<T> extends Long2ObjectMaps.SynchronizedMap<T> { // paper - synchronize accesss
     private final int a;
-    private final Long2LongMap b = new Long2LongLinkedOpenHashMap();
+    private final Long2LongMap ttl = new Long2LongLinkedOpenHashMap(); // Paper
 
     public ExpiringMap(int i, int j) {
-        super(i);
+        super(new Long2ObjectOpenHashMap<>(i)); // Paper
         this.a = j;
     }
 
-    private void a(long i) {
-        long j = SystemUtils.b();
-        this.b.put(i, j);
-        ObjectIterator objectiterator = this.b.long2LongEntrySet().iterator();
+    // Paper start
+    private synchronized void setAccess(long i) { a(i); } // Paper - OBFHELPER
+    private synchronized void a(long i) {
+        long j = System.currentTimeMillis(); // Paper
+        this.ttl.put(i, j);
+        if (!registered) {
+            registered = true;
+            MinecraftServer.getServer().expiringMaps.add(this);
+        }
+    }
+
+    @Override
+    public T compute(long l, BiFunction<? super Long, ? super T, ? extends T> biFunction) {
+        setAccess(l);
+        return super.compute(l, biFunction);
+    }
+
+    @Override
+    public T putIfAbsent(long l, T t) {
+        setAccess(l);
+        return super.putIfAbsent(l, t);
+    }
+
+    @Override
+    public T computeIfPresent(long l, BiFunction<? super Long, ? super T, ? extends T> biFunction) {
+        setAccess(l);
+        return super.computeIfPresent(l, biFunction);
+    }
+
+    @Override
+    public T computeIfAbsent(long l, LongFunction<? extends T> longFunction) {
+        setAccess(l);
+        return super.computeIfAbsent(l, longFunction);
+    }
+
+    @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<? super Long, ? extends T> function) {
+        setAccess(aLong);
+        return super.computeIfAbsent(aLong, function);
+    }
+
+    @Override
+    public T computeIfPresent(Long aLong, BiFunction<? super Long, ? super T, ? extends T> biFunction) {
+        setAccess(aLong);
+        return super.computeIfPresent(aLong, biFunction);
+    }
+
+    @Override
+    public T compute(Long aLong, BiFunction<? super Long, ? super T, ? extends T> biFunction) {
+        setAccess(aLong);
+        return super.compute(aLong, biFunction);
+    }
+
+    @Override
+    public synchronized void clear() {
+        ttl.clear();
+        super.clear();
+    }
+
+    private boolean registered = false;
+    private boolean hasLeaked = false;
+
+    // Break clean to its own method to be ticked
+    synchronized boolean clean() {
+        long now = System.currentTimeMillis();
+        ObjectIterator<Long2LongMap.Entry> objectiterator = this.ttl.long2LongEntrySet().iterator(); // Paper
 
         while(objectiterator.hasNext()) {
-            Entry entry = (Entry)objectiterator.next();
-            Object object = super.get(entry.getLongKey());
-            if (j - entry.getLongValue() <= (long)this.a) {
+            Long2LongMap.Entry entry = objectiterator.next(); // Paper
+            T object = super.get(entry.getLongKey()); // Paper
+            if (now - entry.getLongValue() <= (long)this.a) {
                 break;
             }
 
@@ -33,7 +128,29 @@ public class ExpiringMap<T> extends Long2ObjectOpenHashMap<T> {
                 objectiterator.remove();
             }
         }
-
+        int ttlSize = this.ttl.size();
+        int thisSize = this.size();
+        if (ttlSize < thisSize) {
+            if (!hasLeaked) { // log once
+                hasLeaked = true;
+                MinecraftServer.LOGGER.warn("WARNING: ExpiringMap desync (" + ttlSize + ":" + thisSize + ")- Memory leak risk! We will recover from this, but this means there is still a bug. Please do not open an issue about this. Mention it in Discord (we don't need everyone reporting the same thing)");
+            }
+            try {
+                for (Entry<T> entry : this.long2ObjectEntrySet()) {
+                    ttl.putIfAbsent(entry.getLongKey(), now);
+                }
+            } catch (Exception e) { } // Ignore any como's
+        } else if (ttlSize > this.size()) {
+            try {
+                this.ttl.long2LongEntrySet().removeIf(entry -> !this.containsKey(entry.getLongKey()));
+            } catch (Exception e) { } // Ignore any como's
+        }
+        if (isEmpty()) {
+            registered = false;
+            return true;
+        }
+        return false;
+        // Paper end
     }
 
     protected boolean a(T var1) {
@@ -51,8 +168,13 @@ public class ExpiringMap<T> extends Long2ObjectOpenHashMap<T> {
     }
 
     public T get(long i) {
-        this.a(i);
-        return (T)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<? extends Long, ? extends T> var1) {
diff --git a/src/main/java/net/minecraft/server/MinecraftServer.java b/src/main/java/net/minecraft/server/MinecraftServer.java
index eb3fc836fb..81cda5df56 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<ExpiringMap> 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.e();
         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
     }
-- 
2.19.0