Improve synchronization on ExpiringMap

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

@@ -1,4 +1,4 @@
-From e9f487b844501e35de9892cd88dfb2c85835b8f9 Mon Sep 17 00:00:00 2001
+From 8fc3810e70f40446333e8ff71983e7c238afcac3 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
@@ -14,10 +14,10 @@ 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
+index 4006f5a69c..127de5c8df 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;
+@@ -2,38 +2,163 @@ package net.minecraft.server;
  
  import it.unimi.dsi.fastutil.longs.Long2LongLinkedOpenHashMap;
  import it.unimi.dsi.fastutil.longs.Long2LongMap;
@@ -42,21 +42,32 @@ index 4006f5a69c..9a3b35ea1a 100644
          this.a = j;
      }
  
--    private void a(long i) {
++    // Paper start
++    private void setAccess(long i) { a(i); } // Paper - OBFHELPER
+     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);
+-
+-        while(objectiterator.hasNext()) {
+-            Entry entry = (Entry)objectiterator.next();
+-            Object object = super.get(entry.getLongKey());
+-            if (j - entry.getLongValue() <= (long)this.a) {
+-                break;
++        synchronized (this.sync) {
++            long j = System.currentTimeMillis(); // Paper
++            this.ttl.put(i, j);
++            if (!registered) {
++                registered = true;
++                MinecraftServer.getServer().expiringMaps.add(this);
+             }
 +        }
 +    }
-+
+ 
+-            if (object != null && this.a(object)) {
+-                super.remove(entry.getLongKey());
+-                objectiterator.remove();
+-            }
 +    @Override
 +    public T compute(long l, BiFunction<? super Long, ? super T, ? extends T> biFunction) {
 +        setAccess(l);
@@ -130,61 +141,64 @@ index 4006f5a69c..9a3b35ea1a 100644
 +    }
 +
 +    @Override
-+    public synchronized void clear() {
-+        ttl.clear();
-+        super.clear();
++    public void clear() {
++        synchronized (this.sync) {
++            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);
++    boolean clean() {
++        synchronized (this.sync) {
++            long now = System.currentTimeMillis();
++            ObjectIterator<Long2LongMap.Entry> 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;
 +                }
-+            } 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 (object != null && this.a(object)) {
++                    super.remove(entry.getLongKey());
++                    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;
 +        }
-+        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> {
+@@ -51,8 +176,13 @@ public class ExpiringMap<T> extends Long2ObjectOpenHashMap<T> {
      }
  
      public T get(long i) {