Merge remote-tracking branch 'origin/master' into pre/1.13
* origin/master: Use ConcurrentHashMap in JsonList & Optimize (#471) (#1309)
This commit is contained in:
commit
5d42c9c99d
1 changed files with 105 additions and 0 deletions
|
@ -0,0 +1,105 @@
|
|||
From b91e3e397c77e26889b815000b9831c5fdd333c3 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 6700e68c06..a2815b91fd 100644
|
||||
--- a/src/main/java/net/minecraft/server/JsonList.java
|
||||
+++ b/src/main/java/net/minecraft/server/JsonList.java
|
||||
@@ -36,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() {
|
||||
@@ -85,8 +86,13 @@ public class JsonList<K, V extends JsonListEntry<K>> {
|
||||
|
||||
@Nullable
|
||||
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) {
|
||||
@@ -115,9 +121,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();
|
||||
}
|
||||
@@ -126,8 +134,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()) {
|
||||
@@ -144,8 +154,10 @@ public class JsonList<K, V extends JsonListEntry<K>> {
|
||||
Object object = iterator.next();
|
||||
|
||||
this.d.remove(this.a((K) object)); // CraftBukkit - decompile error
|
||||
- }
|
||||
+ }*/
|
||||
|
||||
+ this.getBackingMap().values().removeIf(JsonListEntry::hasExpired);
|
||||
+ // Paper end
|
||||
}
|
||||
|
||||
protected JsonListEntry<K> a(JsonObject jsonobject) {
|
||||
@@ -157,6 +169,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
|
||||
|
Loading…
Reference in a new issue