Improve MT-Safety of UserCache (#2080)

We were missing a synchronize on a get if cached method, however
it appears that using a ConcurrentHashMap is a better solution
so readers can avoid locking if they just want a cached value.

Existing synchronization for writers remains untouched, the
ConcurrentHashMap is just so readers can safely read without
synchronization
This commit is contained in:
Spottedleaf 2019-05-26 23:17:50 -07:00 committed by Zach
parent 5dc46cd3b2
commit 7fb12d787e
2 changed files with 30 additions and 22 deletions

View file

@ -1,4 +1,4 @@
From a712ddf77ea7d2397deccb4fe233072b5926649f Mon Sep 17 00:00:00 2001 From a29caf1df2a8c7f8a6201b61dd3487c7525829e5 Mon Sep 17 00:00:00 2001
From: Aikar <aikar@aikar.co> From: Aikar <aikar@aikar.co>
Date: Mon, 16 May 2016 20:47:41 -0400 Date: Mon, 16 May 2016 20:47:41 -0400
Subject: [PATCH] Optimize UserCache / Thread Safe Subject: [PATCH] Optimize UserCache / Thread Safe
@ -10,7 +10,7 @@ Additionally, move Saving of the User cache to be done async, incase
the user never changed the default setting for Spigot's save on stop only. the user never changed the default setting for Spigot's save on stop only.
diff --git a/src/main/java/net/minecraft/server/MinecraftServer.java b/src/main/java/net/minecraft/server/MinecraftServer.java diff --git a/src/main/java/net/minecraft/server/MinecraftServer.java b/src/main/java/net/minecraft/server/MinecraftServer.java
index a3c53d9dbf..3b745b2b75 100644 index d3d8fffeb0..f142ed9a3b 100644
--- a/src/main/java/net/minecraft/server/MinecraftServer.java --- a/src/main/java/net/minecraft/server/MinecraftServer.java
+++ b/src/main/java/net/minecraft/server/MinecraftServer.java +++ b/src/main/java/net/minecraft/server/MinecraftServer.java
@@ -739,7 +739,7 @@ public abstract class MinecraftServer extends IAsyncTaskHandlerReentrant<TickTas @@ -739,7 +739,7 @@ public abstract class MinecraftServer extends IAsyncTaskHandlerReentrant<TickTas
@ -23,9 +23,20 @@ index a3c53d9dbf..3b745b2b75 100644
// Spigot end // Spigot end
} }
diff --git a/src/main/java/net/minecraft/server/UserCache.java b/src/main/java/net/minecraft/server/UserCache.java diff --git a/src/main/java/net/minecraft/server/UserCache.java b/src/main/java/net/minecraft/server/UserCache.java
index 41be5bf64e..cd8a652eb6 100644 index 41be5bf64e..dc5c0afab4 100644
--- a/src/main/java/net/minecraft/server/UserCache.java --- a/src/main/java/net/minecraft/server/UserCache.java
+++ b/src/main/java/net/minecraft/server/UserCache.java +++ b/src/main/java/net/minecraft/server/UserCache.java
@@ -43,8 +43,8 @@ public class UserCache {
public static final SimpleDateFormat a = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss Z");
private static boolean c;
- private final Map<String, UserCache.UserCacheEntry> d = Maps.newHashMap();
- private final Map<UUID, UserCache.UserCacheEntry> e = Maps.newHashMap();
+ private final Map<String, UserCache.UserCacheEntry> d = new java.util.concurrent.ConcurrentHashMap<>(); // Paper
+ private final Map<UUID, UserCache.UserCacheEntry> e = new java.util.concurrent.ConcurrentHashMap<>(); // Paper
private final Deque<GameProfile> f = new java.util.concurrent.LinkedBlockingDeque<GameProfile>(); // CraftBukkit
private final GameProfileRepository g;
protected final Gson b;
@@ -108,7 +108,7 @@ public class UserCache { @@ -108,7 +108,7 @@ public class UserCache {
this.a(gameprofile, (Date) null); this.a(gameprofile, (Date) null);
} }
@ -55,17 +66,14 @@ index 41be5bf64e..cd8a652eb6 100644
String s1 = s.toLowerCase(Locale.ROOT); String s1 = s.toLowerCase(Locale.ROOT);
UserCache.UserCacheEntry usercache_usercacheentry = (UserCache.UserCacheEntry) this.d.get(s1); UserCache.UserCacheEntry usercache_usercacheentry = (UserCache.UserCacheEntry) this.d.get(s1);
@@ -164,8 +165,9 @@ public class UserCache { @@ -164,6 +165,7 @@ public class UserCache {
return usercache_usercacheentry == null ? null : usercache_usercacheentry.a(); return usercache_usercacheentry == null ? null : usercache_usercacheentry.a();
} }
+ @Nullable public GameProfile getProfile(UUID uuid) { return a(uuid); } // Paper - OBFHELPER + @Nullable public GameProfile getProfile(UUID uuid) { return a(uuid); } // Paper - OBFHELPER
@Nullable @Nullable
- public GameProfile a(UUID uuid) { public GameProfile a(UUID uuid) {
+ public synchronized GameProfile a(UUID uuid) { // Paper - synchronize
UserCache.UserCacheEntry usercache_usercacheentry = (UserCache.UserCacheEntry) this.e.get(uuid); UserCache.UserCacheEntry usercache_usercacheentry = (UserCache.UserCacheEntry) this.e.get(uuid);
return usercache_usercacheentry == null ? null : usercache_usercacheentry.a();
@@ -220,8 +222,15 @@ public class UserCache { @@ -220,8 +222,15 @@ public class UserCache {
} }

View file

@ -1,4 +1,4 @@
From 57495dc921d915f3ccc76d8c3b55ad9826165fc8 Mon Sep 17 00:00:00 2001 From 769191c6a329f12bc0b71fb682ab228bd5181e34 Mon Sep 17 00:00:00 2001
From: Aikar <aikar@aikar.co> From: Aikar <aikar@aikar.co>
Date: Mon, 15 Jan 2018 22:11:48 -0500 Date: Mon, 15 Jan 2018 22:11:48 -0500
Subject: [PATCH] Basic PlayerProfile API Subject: [PATCH] Basic PlayerProfile API
@ -7,7 +7,7 @@ Establishes base extension of profile systems for future edits too
diff --git a/src/main/java/com/destroystokyo/paper/profile/CraftPlayerProfile.java b/src/main/java/com/destroystokyo/paper/profile/CraftPlayerProfile.java diff --git a/src/main/java/com/destroystokyo/paper/profile/CraftPlayerProfile.java b/src/main/java/com/destroystokyo/paper/profile/CraftPlayerProfile.java
new file mode 100644 new file mode 100644
index 000000000..b151a13c1 index 0000000000..b151a13c1b
--- /dev/null --- /dev/null
+++ b/src/main/java/com/destroystokyo/paper/profile/CraftPlayerProfile.java +++ b/src/main/java/com/destroystokyo/paper/profile/CraftPlayerProfile.java
@@ -0,0 +1,280 @@ @@ -0,0 +1,280 @@
@ -293,7 +293,7 @@ index 000000000..b151a13c1
+} +}
diff --git a/src/main/java/com/destroystokyo/paper/profile/PaperAuthenticationService.java b/src/main/java/com/destroystokyo/paper/profile/PaperAuthenticationService.java diff --git a/src/main/java/com/destroystokyo/paper/profile/PaperAuthenticationService.java b/src/main/java/com/destroystokyo/paper/profile/PaperAuthenticationService.java
new file mode 100644 new file mode 100644
index 000000000..25836b975 index 0000000000..25836b975b
--- /dev/null --- /dev/null
+++ b/src/main/java/com/destroystokyo/paper/profile/PaperAuthenticationService.java +++ b/src/main/java/com/destroystokyo/paper/profile/PaperAuthenticationService.java
@@ -0,0 +1,30 @@ @@ -0,0 +1,30 @@
@ -329,7 +329,7 @@ index 000000000..25836b975
+} +}
diff --git a/src/main/java/com/destroystokyo/paper/profile/PaperGameProfileRepository.java b/src/main/java/com/destroystokyo/paper/profile/PaperGameProfileRepository.java diff --git a/src/main/java/com/destroystokyo/paper/profile/PaperGameProfileRepository.java b/src/main/java/com/destroystokyo/paper/profile/PaperGameProfileRepository.java
new file mode 100644 new file mode 100644
index 000000000..3bcdb8f93 index 0000000000..3bcdb8f93f
--- /dev/null --- /dev/null
+++ b/src/main/java/com/destroystokyo/paper/profile/PaperGameProfileRepository.java +++ b/src/main/java/com/destroystokyo/paper/profile/PaperGameProfileRepository.java
@@ -0,0 +1,17 @@ @@ -0,0 +1,17 @@
@ -352,7 +352,7 @@ index 000000000..3bcdb8f93
+} +}
diff --git a/src/main/java/com/destroystokyo/paper/profile/PaperMinecraftSessionService.java b/src/main/java/com/destroystokyo/paper/profile/PaperMinecraftSessionService.java diff --git a/src/main/java/com/destroystokyo/paper/profile/PaperMinecraftSessionService.java b/src/main/java/com/destroystokyo/paper/profile/PaperMinecraftSessionService.java
new file mode 100644 new file mode 100644
index 000000000..4b2a67423 index 0000000000..4b2a67423f
--- /dev/null --- /dev/null
+++ b/src/main/java/com/destroystokyo/paper/profile/PaperMinecraftSessionService.java +++ b/src/main/java/com/destroystokyo/paper/profile/PaperMinecraftSessionService.java
@@ -0,0 +1,29 @@ @@ -0,0 +1,29 @@
@ -387,7 +387,7 @@ index 000000000..4b2a67423
+} +}
diff --git a/src/main/java/com/destroystokyo/paper/profile/PaperUserAuthentication.java b/src/main/java/com/destroystokyo/paper/profile/PaperUserAuthentication.java diff --git a/src/main/java/com/destroystokyo/paper/profile/PaperUserAuthentication.java b/src/main/java/com/destroystokyo/paper/profile/PaperUserAuthentication.java
new file mode 100644 new file mode 100644
index 000000000..3aceb0ea8 index 0000000000..3aceb0ea8a
--- /dev/null --- /dev/null
+++ b/src/main/java/com/destroystokyo/paper/profile/PaperUserAuthentication.java +++ b/src/main/java/com/destroystokyo/paper/profile/PaperUserAuthentication.java
@@ -0,0 +1,11 @@ @@ -0,0 +1,11 @@
@ -403,7 +403,7 @@ index 000000000..3aceb0ea8
+ } + }
+} +}
diff --git a/src/main/java/net/minecraft/server/MCUtil.java b/src/main/java/net/minecraft/server/MCUtil.java diff --git a/src/main/java/net/minecraft/server/MCUtil.java b/src/main/java/net/minecraft/server/MCUtil.java
index 1f6a12632..6d278a0da 100644 index 1f6a126329..6d278a0da5 100644
--- a/src/main/java/net/minecraft/server/MCUtil.java --- a/src/main/java/net/minecraft/server/MCUtil.java
+++ b/src/main/java/net/minecraft/server/MCUtil.java +++ b/src/main/java/net/minecraft/server/MCUtil.java
@@ -1,7 +1,10 @@ @@ -1,7 +1,10 @@
@ -429,7 +429,7 @@ index 1f6a12632..6d278a0da 100644
* Calculates distance between 2 entities * Calculates distance between 2 entities
* @param e1 * @param e1
diff --git a/src/main/java/net/minecraft/server/MinecraftServer.java b/src/main/java/net/minecraft/server/MinecraftServer.java diff --git a/src/main/java/net/minecraft/server/MinecraftServer.java b/src/main/java/net/minecraft/server/MinecraftServer.java
index cdf162a95..d909ad682 100644 index 6b83f9769a..5817e94525 100644
--- a/src/main/java/net/minecraft/server/MinecraftServer.java --- a/src/main/java/net/minecraft/server/MinecraftServer.java
+++ b/src/main/java/net/minecraft/server/MinecraftServer.java +++ b/src/main/java/net/minecraft/server/MinecraftServer.java
@@ -1246,7 +1246,7 @@ public abstract class MinecraftServer extends IAsyncTaskHandlerReentrant<TickTas @@ -1246,7 +1246,7 @@ public abstract class MinecraftServer extends IAsyncTaskHandlerReentrant<TickTas
@ -450,16 +450,16 @@ index cdf162a95..d909ad682 100644
return this.minecraftSessionService; return this.minecraftSessionService;
} }
diff --git a/src/main/java/net/minecraft/server/UserCache.java b/src/main/java/net/minecraft/server/UserCache.java diff --git a/src/main/java/net/minecraft/server/UserCache.java b/src/main/java/net/minecraft/server/UserCache.java
index 409bc6da9..9eaf1539f 100644 index fd17a3e93a..d88a510bd0 100644
--- a/src/main/java/net/minecraft/server/UserCache.java --- a/src/main/java/net/minecraft/server/UserCache.java
+++ b/src/main/java/net/minecraft/server/UserCache.java +++ b/src/main/java/net/minecraft/server/UserCache.java
@@ -43,7 +43,7 @@ public class UserCache { @@ -43,7 +43,7 @@ public class UserCache {
public static final SimpleDateFormat a = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss Z"); public static final SimpleDateFormat a = new SimpleDateFormat("yyyy-MM-dd HH:mm:ss Z");
private static boolean c; private static boolean c;
- private final Map<String, UserCache.UserCacheEntry> d = Maps.newHashMap(); - private final Map<String, UserCache.UserCacheEntry> d = new java.util.concurrent.ConcurrentHashMap<>(); // Paper
+ private final Map<String, UserCache.UserCacheEntry> d = Maps.newHashMap();private final Map<String, UserCache.UserCacheEntry> nameCache = d; // Paper - OBFHELPER + private final Map<String, UserCache.UserCacheEntry> d = new java.util.concurrent.ConcurrentHashMap<>();private final Map<String, UserCache.UserCacheEntry> nameCache = d; // Paper - OBFHELPER // Paper
private final Map<UUID, UserCache.UserCacheEntry> e = Maps.newHashMap(); private final Map<UUID, UserCache.UserCacheEntry> e = new java.util.concurrent.ConcurrentHashMap<>(); // Paper
private final Deque<GameProfile> f = new java.util.concurrent.LinkedBlockingDeque<GameProfile>(); // CraftBukkit private final Deque<GameProfile> f = new java.util.concurrent.LinkedBlockingDeque<GameProfile>(); // CraftBukkit
private final GameProfileRepository g; private final GameProfileRepository g;
@@ -165,6 +165,13 @@ public class UserCache { @@ -165,6 +165,13 @@ public class UserCache {
@ -475,7 +475,7 @@ index 409bc6da9..9eaf1539f 100644
+ +
@Nullable public GameProfile getProfile(UUID uuid) { return a(uuid); } // Paper - OBFHELPER @Nullable public GameProfile getProfile(UUID uuid) { return a(uuid); } // Paper - OBFHELPER
@Nullable @Nullable
public synchronized GameProfile a(UUID uuid) { // Paper - synchronize public GameProfile a(UUID uuid) {
@@ -274,7 +281,7 @@ public class UserCache { @@ -274,7 +281,7 @@ public class UserCache {
class UserCacheEntry { class UserCacheEntry {
@ -486,7 +486,7 @@ index 409bc6da9..9eaf1539f 100644
private UserCacheEntry(GameProfile gameprofile, Date date) { private UserCacheEntry(GameProfile gameprofile, Date date) {
diff --git a/src/main/java/org/bukkit/craftbukkit/CraftServer.java b/src/main/java/org/bukkit/craftbukkit/CraftServer.java diff --git a/src/main/java/org/bukkit/craftbukkit/CraftServer.java b/src/main/java/org/bukkit/craftbukkit/CraftServer.java
index 44acc6a5f..cdf3b6b6c 100644 index a79403cbce..8f7fd3490f 100644
--- a/src/main/java/org/bukkit/craftbukkit/CraftServer.java --- a/src/main/java/org/bukkit/craftbukkit/CraftServer.java
+++ b/src/main/java/org/bukkit/craftbukkit/CraftServer.java +++ b/src/main/java/org/bukkit/craftbukkit/CraftServer.java
@@ -196,6 +196,9 @@ import org.yaml.snakeyaml.error.MarkedYAMLException; @@ -196,6 +196,9 @@ import org.yaml.snakeyaml.error.MarkedYAMLException;