From 6ef9abeec2fc9be9d2467c1950cfe9d5bf4260e7 Mon Sep 17 00:00:00 2001
From: egg82 <eggys82@gmail.com>
Date: Sat, 11 Aug 2018 06:46:46 -0600
Subject: [PATCH] Use ConcurrentHashMap in JsonList & Optimize (#471) (#1309)
* [CI-SKIP] add .editorconfig for base code style settings
* * Created patch 0349 (fixes #471)
* * Made requested modifications
* * Made requested modifications (x2)
* * Made recommended changes (x3)
* * Moved ConcurrentMap return values to Map as no functions specific to ConcurrentMap were used (backing map is still ConcurrentMap)
* Removed ConcurrentMap import
---
...49-Use-ConcurrentHashMap-in-JsonList.patch | 114 ++++++++++++++++++
1 file changed, 114 insertions(+)
create mode 100644 Spigot-Server-Patches/0349-Use-ConcurrentHashMap-in-JsonList.patch
diff --git a/Spigot-Server-Patches/0349-Use-ConcurrentHashMap-in-JsonList.patch b/Spigot-Server-Patches/0349-Use-ConcurrentHashMap-in-JsonList.patch
new file mode 100644
index 0000000000..608df19f2d
--- /dev/null
+++ b/Spigot-Server-Patches/0349-Use-ConcurrentHashMap-in-JsonList.patch
@@ -0,0 +1,114 @@
+From d5626a994903ac7997f1fd6e1efa7ede3d180588 Mon Sep 17 00:00:00 2001
+From: egg82 <phantom_zero@ymail.com>
+Date: Tue, 7 Aug 2018 01:24:23 -0600
+Subject: [PATCH] Use ConcurrentHashMap in JsonList
+
+This is specifically aimed at fixing #471
+
+Using a ConcurrentHashMap because thread safety
+The performance benefit of Map over ConcurrentMap is negligabe at best in this scenaio, as most operations will be get and not add or remove
+Even without considering the use-case the benefits are still negligable
+
+Original ideas for the system included an expiration policy and/or handler
+The simpler solution was to use a computeIfPresent in the get method
+This will simultaneously have an O(1) lookup time and automatically expire any values
+Since the get method (nor other similar methods) don't seem to have a critical need to flush the map to disk at any of these points further processing is simply wasteful
+Meaning the original function expired values unrelated to the current value without actually having any explicit need to
+
+The h method was heavily modified to be much more efficient in its processing
+Also instead of being called on every get, it's now called just before a save
+This will eliminate stale values being flushed to disk
+
+Modified isEmpty to use the isEmpty() method instead of the slightly confusing size() < 1
+The point of this is readability, but does have a side-benefit of a small microptimization
+
+Finally, added a couple obfhelpers for the modified code
+
+diff --git a/src/main/java/net/minecraft/server/JsonList.java b/src/main/java/net/minecraft/server/JsonList.java
+index 0859c7eb2..93111cc24 100644
+--- a/src/main/java/net/minecraft/server/JsonList.java
++++ b/src/main/java/net/minecraft/server/JsonList.java
+@@ -26,6 +26,7 @@ import java.util.Collection;
+ import java.util.Iterator;
+ import java.util.List;
+ import java.util.Map;
++
+ import org.apache.commons.io.IOUtils;
+ import org.apache.logging.log4j.LogManager;
+ import org.apache.logging.log4j.Logger;
+@@ -35,7 +36,8 @@ public class JsonList<K, V extends JsonListEntry<K>> {
+ protected static final Logger a = LogManager.getLogger();
+ protected final Gson b;
+ private final File c;
+- private final Map<String, V> d = Maps.newHashMap();
++ // Paper - replace HashMap is ConcurrentHashMap
++ private final Map<String, V> d = Maps.newConcurrentMap(); private final Map<String, V> getBackingMap() { return this.d; } // Paper - OBFHELPER
+ private boolean e = true;
+ private static final ParameterizedType f = new ParameterizedType() {
+ public Type[] getActualTypeArguments() {
+@@ -83,8 +85,13 @@ public class JsonList<K, V extends JsonListEntry<K>> {
+ }
+
+ public V get(K k0) {
+- this.h();
+- return (V) this.d.get(this.a(k0)); // CraftBukkit - fix decompile error
++ // Paper start
++ // this.h();
++ // return (V) this.d.get(this.a(k0)); // CraftBukkit - fix decompile error
++ return (V) this.getBackingMap().computeIfPresent(this.getMappingKey(k0), (k, v) -> {
++ return v.hasExpired() ? null : v;
++ });
++ // Paper end
+ }
+
+ public void remove(K k0) {
+@@ -109,9 +116,11 @@ public class JsonList<K, V extends JsonListEntry<K>> {
+ // CraftBukkit end
+
+ public boolean isEmpty() {
+- return this.d.size() < 1;
++ // return this.d.size() < 1; // Paper
++ return this.getBackingMap().isEmpty(); // Paper - readability is the goal. As an aside, isEmpty() uses only sumCount() and a comparison. size() uses sumCount(), casts, and boolean logic
+ }
+
++ protected final String getMappingKey(K k0) { return a(k0); } // Paper - OBFHELPER
+ protected String a(K k0) {
+ return k0.toString();
+ }
+@@ -120,8 +129,10 @@ public class JsonList<K, V extends JsonListEntry<K>> {
+ return this.d.containsKey(this.a(k0));
+ }
+
++ private void removeStaleEntries() { h(); } // Paper - OBFHELPER
+ private void h() {
+- ArrayList arraylist = Lists.newArrayList();
++ // Paper start
++ /*ArrayList arraylist = Lists.newArrayList();
+ Iterator iterator = this.d.values().iterator();
+
+ while (iterator.hasNext()) {
+@@ -138,8 +149,10 @@ public class JsonList<K, V extends JsonListEntry<K>> {
+ Object object = iterator.next();
+
+ this.d.remove(object);
+- }
+-
++ }*/
++
++ this.getBackingMap().values().removeIf((v) -> v.hasExpired());
++ // Paper end
+ }
+
+ protected JsonListEntry<K> a(JsonObject jsonobject) {
+@@ -151,6 +164,8 @@ public class JsonList<K, V extends JsonListEntry<K>> {
+ }
+
+ public void save() throws IOException {
++ this.removeStaleEntries(); // Paper - remove expired values before saving
++
+ Collection collection = this.d.values();
+ String s = this.b.toJson(collection);
+ BufferedWriter bufferedwriter = null;
+--
+2.18.0.windows.1
+