From abf6def0f0b8769f878c99254a438ff8ac9a89ee Mon Sep 17 00:00:00 2001 From: Maddy Miller Date: Mon, 2 Nov 2026 20:45:53 +1000 Subject: [PATCH 1/4] Cleanup allocations by session caching code --- .../session/AbstractSessionManager.java | 77 +++++++++++++------ .../com/sk89q/worldguard/session/Session.java | 3 + .../worldguard/session/WorldPlayerTuple.java | 1 + 3 files changed, 58 insertions(+), 23 deletions(-) diff --git a/worldguard-core/src/main/java/com/sk89q/worldguard/session/AbstractSessionManager.java b/worldguard-core/src/main/java/com/sk89q/worldguard/session/AbstractSessionManager.java index 0f9a05882..114279ecd 100644 --- a/worldguard-core/src/main/java/com/sk89q/worldguard/session/AbstractSessionManager.java +++ b/worldguard-core/src/main/java/com/sk89q/worldguard/session/AbstractSessionManager.java @@ -44,11 +44,11 @@ import javax.annotation.Nullable; import java.lang.ref.WeakReference; +import java.time.Duration; import java.util.Arrays; import java.util.LinkedList; import java.util.List; import java.util.UUID; -import java.util.concurrent.TimeUnit; import java.util.function.BiPredicate; import java.util.logging.Level; @@ -59,21 +59,26 @@ public abstract class AbstractSessionManager implements SessionManager { public static final int RUN_DELAY = 20; public static final long SESSION_LIFETIME = 10; - private static final BiPredicate BYPASS_PERMISSION_TEST = (world, player) -> { - return player.hasPermission("worldguard.region.bypass." + world.getName()); - }; + private static final BiPredicate BYPASS_PERMISSION_TEST = (worldName, player) -> player.hasPermission("worldguard.region.bypass." + worldName); - private final LoadingCache bypassCache = CacheBuilder.newBuilder() + private final LoadingCache bypassCache = CacheBuilder.newBuilder() .maximumSize(1000) - .expireAfterWrite(2, TimeUnit.SECONDS) - .build(CacheLoader.from(tuple -> BYPASS_PERMISSION_TEST.test(tuple.getWorld(), tuple.getPlayer()))); - - private final Cache sessions = CacheBuilder.newBuilder() - .expireAfterAccess(SESSION_LIFETIME, TimeUnit.MINUTES) + .expireAfterWrite(Duration.ofSeconds(2)) + .build(CacheLoader.from(tuple -> { + LocalPlayer player = tuple.playerRef.get(); + if (player == null) { + // Player has been garbage collected, so we can't check permissions. + return false; + } + return BYPASS_PERMISSION_TEST.test(tuple.worldName, player); + })); + + private final Cache sessions = CacheBuilder.newBuilder() + .expireAfterAccess(Duration.ofMinutes(SESSION_LIFETIME)) .build(); private boolean hasCustom = false; - private List> handlers = new LinkedList<>(); + private final List> handlers = new LinkedList<>(); private static final List> defaultHandlers = new LinkedList<>(); @@ -143,16 +148,16 @@ public boolean hasBypass(LocalPlayer player, World world) { } if (WorldGuard.getInstance().getPlatform().getGlobalStateManager().disablePermissionCache) { - return BYPASS_PERMISSION_TEST.test(world, player); + return BYPASS_PERMISSION_TEST.test(world.getName(), player); } - return bypassCache.getUnchecked(new WorldPlayerTuple(world, player)); + return bypassCache.getUnchecked(new BypassCacheKey(world.getName(), player)); } @Override public void resetState(LocalPlayer player) { checkNotNull(player, "player"); - @Nullable Session session = getIfPresentInternal(new CacheKey(player)); + @Nullable Session session = getIfPresentInternal(new SessionCacheKey(player)); if (session != null) { session.resetState(player); } @@ -161,10 +166,10 @@ public void resetState(LocalPlayer player) { @Override @Nullable public Session getIfPresent(LocalPlayer player) { - return getIfPresentInternal(new CacheKey(player)); + return getIfPresentInternal(new SessionCacheKey(player)); } - private Session getIfPresentInternal(CacheKey cacheKey) { + private Session getIfPresentInternal(SessionCacheKey cacheKey) { @Nullable Session session = sessions.getIfPresent(cacheKey); if (session != null) { session.ensureInitialized(cacheKey.playerRef.get(), this::initializeSession); @@ -175,7 +180,7 @@ private Session getIfPresentInternal(CacheKey cacheKey) { @Override public Session get(LocalPlayer player) { - Session session = sessions.asMap().computeIfAbsent(new CacheKey(player), (k) -> new Session(this)); + Session session = sessions.asMap().computeIfAbsent(new SessionCacheKey(player), (_) -> new Session(this)); session.ensureInitialized(player, this::initializeSession); return session; } @@ -192,22 +197,21 @@ private void initializeSession(Session session, LocalPlayer player) { session.initialize(player); } - protected static final class CacheKey { + protected static final class SessionCacheKey { final WeakReference playerRef; final UUID uuid; - CacheKey(LocalPlayer player) { - playerRef = new WeakReference<>(player); - uuid = player.getUniqueId(); + SessionCacheKey(LocalPlayer player) { + this.playerRef = new WeakReference<>(player); + this.uuid = player.getUniqueId(); } @Override public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; - CacheKey cacheKey = (CacheKey) o; + SessionCacheKey cacheKey = (SessionCacheKey) o; return uuid.equals(cacheKey.uuid); - } @Override @@ -215,4 +219,31 @@ public int hashCode() { return uuid.hashCode(); } } + + protected static final class BypassCacheKey { + final WeakReference playerRef; + final String worldName; + final UUID playerUUID; + + BypassCacheKey(String worldName, LocalPlayer player) { + this.playerRef = new WeakReference<>(player); + this.worldName = worldName; + this.playerUUID = player.getUniqueId(); + } + + @Override + public boolean equals(Object o) { + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + BypassCacheKey that = (BypassCacheKey) o; + return worldName.equals(that.worldName) && playerUUID.equals(that.playerUUID); + } + + @Override + public int hashCode() { + int result = worldName.hashCode(); + result = 31 * result + playerUUID.hashCode(); + return result; + } + } } diff --git a/worldguard-core/src/main/java/com/sk89q/worldguard/session/Session.java b/worldguard-core/src/main/java/com/sk89q/worldguard/session/Session.java index fe42bbaae..3fc21bccb 100644 --- a/worldguard-core/src/main/java/com/sk89q/worldguard/session/Session.java +++ b/worldguard-core/src/main/java/com/sk89q/worldguard/session/Session.java @@ -143,6 +143,9 @@ public void uninitialize(LocalPlayer player) { for (Handler handler : handlers.values()) { handler.uninitialize(player, location, set); } + + lastValid = null; + lastRegionSet = null; } /** diff --git a/worldguard-core/src/main/java/com/sk89q/worldguard/session/WorldPlayerTuple.java b/worldguard-core/src/main/java/com/sk89q/worldguard/session/WorldPlayerTuple.java index 8996e147e..7b2facf7c 100644 --- a/worldguard-core/src/main/java/com/sk89q/worldguard/session/WorldPlayerTuple.java +++ b/worldguard-core/src/main/java/com/sk89q/worldguard/session/WorldPlayerTuple.java @@ -22,6 +22,7 @@ import com.sk89q.worldedit.world.World; import com.sk89q.worldguard.LocalPlayer; +@Deprecated(forRemoval = true) public class WorldPlayerTuple { private final World world; From 229751765c2fcc809b28d7c138ca4a754bd743b5 Mon Sep 17 00:00:00 2001 From: Maddy Miller Date: Mon, 2 Nov 2026 21:06:13 +1000 Subject: [PATCH 2/4] Even further cleanup, no weak references are needed --- .../session/AbstractSessionManager.java | 91 +++++-------------- 1 file changed, 22 insertions(+), 69 deletions(-) diff --git a/worldguard-core/src/main/java/com/sk89q/worldguard/session/AbstractSessionManager.java b/worldguard-core/src/main/java/com/sk89q/worldguard/session/AbstractSessionManager.java index 114279ecd..32f92c3d4 100644 --- a/worldguard-core/src/main/java/com/sk89q/worldguard/session/AbstractSessionManager.java +++ b/worldguard-core/src/main/java/com/sk89q/worldguard/session/AbstractSessionManager.java @@ -21,8 +21,6 @@ import com.google.common.cache.Cache; import com.google.common.cache.CacheBuilder; -import com.google.common.cache.CacheLoader; -import com.google.common.cache.LoadingCache; import com.sk89q.worldedit.world.World; import com.sk89q.worldguard.LocalPlayer; import com.sk89q.worldguard.WorldGuard; @@ -43,7 +41,6 @@ import com.sk89q.worldguard.session.handler.WeatherLockFlag; import javax.annotation.Nullable; -import java.lang.ref.WeakReference; import java.time.Duration; import java.util.Arrays; import java.util.LinkedList; @@ -61,19 +58,12 @@ public abstract class AbstractSessionManager implements SessionManager { private static final BiPredicate BYPASS_PERMISSION_TEST = (worldName, player) -> player.hasPermission("worldguard.region.bypass." + worldName); - private final LoadingCache bypassCache = CacheBuilder.newBuilder() + private final Cache bypassCache = CacheBuilder.newBuilder() .maximumSize(1000) .expireAfterWrite(Duration.ofSeconds(2)) - .build(CacheLoader.from(tuple -> { - LocalPlayer player = tuple.playerRef.get(); - if (player == null) { - // Player has been garbage collected, so we can't check permissions. - return false; - } - return BYPASS_PERMISSION_TEST.test(tuple.worldName, player); - })); - - private final Cache sessions = CacheBuilder.newBuilder() + .build(); + + private final Cache sessions = CacheBuilder.newBuilder() .expireAfterAccess(Duration.ofMinutes(SESSION_LIFETIME)) .build(); @@ -151,13 +141,19 @@ public boolean hasBypass(LocalPlayer player, World world) { return BYPASS_PERMISSION_TEST.test(world.getName(), player); } - return bypassCache.getUnchecked(new BypassCacheKey(world.getName(), player)); + var bypassCacheKey = new BypassCacheKey(world.getName(), player.getUniqueId()); + Boolean bypass = bypassCache.getIfPresent(bypassCacheKey); + if (bypass == null) { + bypass = BYPASS_PERMISSION_TEST.test(world.getName(), player); + bypassCache.put(bypassCacheKey, bypass); + } + return bypass; } @Override public void resetState(LocalPlayer player) { checkNotNull(player, "player"); - @Nullable Session session = getIfPresentInternal(new SessionCacheKey(player)); + @Nullable Session session = getIfPresentInternal(player); if (session != null) { session.resetState(player); } @@ -166,13 +162,13 @@ public void resetState(LocalPlayer player) { @Override @Nullable public Session getIfPresent(LocalPlayer player) { - return getIfPresentInternal(new SessionCacheKey(player)); + return getIfPresentInternal(player); } - private Session getIfPresentInternal(SessionCacheKey cacheKey) { - @Nullable Session session = sessions.getIfPresent(cacheKey); + private Session getIfPresentInternal(LocalPlayer player) { + @Nullable Session session = sessions.getIfPresent(player.getUniqueId()); if (session != null) { - session.ensureInitialized(cacheKey.playerRef.get(), this::initializeSession); + session.ensureInitialized(player, this::initializeSession); return session; } return null; @@ -180,7 +176,11 @@ private Session getIfPresentInternal(SessionCacheKey cacheKey) { @Override public Session get(LocalPlayer player) { - Session session = sessions.asMap().computeIfAbsent(new SessionCacheKey(player), (_) -> new Session(this)); + Session session = sessions.getIfPresent(player.getUniqueId()); + if (session == null) { + session = new Session(this); + sessions.put(player.getUniqueId(), session); + } session.ensureInitialized(player, this::initializeSession); return session; } @@ -197,53 +197,6 @@ private void initializeSession(Session session, LocalPlayer player) { session.initialize(player); } - protected static final class SessionCacheKey { - final WeakReference playerRef; - final UUID uuid; - - SessionCacheKey(LocalPlayer player) { - this.playerRef = new WeakReference<>(player); - this.uuid = player.getUniqueId(); - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - SessionCacheKey cacheKey = (SessionCacheKey) o; - return uuid.equals(cacheKey.uuid); - } - - @Override - public int hashCode() { - return uuid.hashCode(); - } - } - - protected static final class BypassCacheKey { - final WeakReference playerRef; - final String worldName; - final UUID playerUUID; - - BypassCacheKey(String worldName, LocalPlayer player) { - this.playerRef = new WeakReference<>(player); - this.worldName = worldName; - this.playerUUID = player.getUniqueId(); - } - - @Override - public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - BypassCacheKey that = (BypassCacheKey) o; - return worldName.equals(that.worldName) && playerUUID.equals(that.playerUUID); - } - - @Override - public int hashCode() { - int result = worldName.hashCode(); - result = 31 * result + playerUUID.hashCode(); - return result; - } + record BypassCacheKey(String worldName, UUID playerUUID) { } } From 0ba08d796eab637d3fa1750d53c4ca9a415d108c Mon Sep 17 00:00:00 2001 From: Maddy Miller Date: Mon, 2 Nov 2026 21:18:35 +1000 Subject: [PATCH 3/4] back to asMap --- .../worldguard/session/AbstractSessionManager.java | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/worldguard-core/src/main/java/com/sk89q/worldguard/session/AbstractSessionManager.java b/worldguard-core/src/main/java/com/sk89q/worldguard/session/AbstractSessionManager.java index 32f92c3d4..9bde2dc92 100644 --- a/worldguard-core/src/main/java/com/sk89q/worldguard/session/AbstractSessionManager.java +++ b/worldguard-core/src/main/java/com/sk89q/worldguard/session/AbstractSessionManager.java @@ -142,12 +142,7 @@ public boolean hasBypass(LocalPlayer player, World world) { } var bypassCacheKey = new BypassCacheKey(world.getName(), player.getUniqueId()); - Boolean bypass = bypassCache.getIfPresent(bypassCacheKey); - if (bypass == null) { - bypass = BYPASS_PERMISSION_TEST.test(world.getName(), player); - bypassCache.put(bypassCacheKey, bypass); - } - return bypass; + return bypassCache.asMap().computeIfAbsent(bypassCacheKey, (_) -> BYPASS_PERMISSION_TEST.test(world.getName(), player)); } @Override @@ -176,11 +171,7 @@ private Session getIfPresentInternal(LocalPlayer player) { @Override public Session get(LocalPlayer player) { - Session session = sessions.getIfPresent(player.getUniqueId()); - if (session == null) { - session = new Session(this); - sessions.put(player.getUniqueId(), session); - } + Session session = sessions.asMap().computeIfAbsent(player.getUniqueId(), (_) -> new Session(this)); session.ensureInitialized(player, this::initializeSession); return session; } From 0a26674aef54196411e974295f14cd14b0a12743 Mon Sep 17 00:00:00 2001 From: Maddy Miller Date: Mon, 2 Nov 2026 21:23:33 +1000 Subject: [PATCH 4/4] Keep World for the test itself, cache string for key --- .../sk89q/worldguard/session/AbstractSessionManager.java | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/worldguard-core/src/main/java/com/sk89q/worldguard/session/AbstractSessionManager.java b/worldguard-core/src/main/java/com/sk89q/worldguard/session/AbstractSessionManager.java index 9bde2dc92..628f923fa 100644 --- a/worldguard-core/src/main/java/com/sk89q/worldguard/session/AbstractSessionManager.java +++ b/worldguard-core/src/main/java/com/sk89q/worldguard/session/AbstractSessionManager.java @@ -56,7 +56,7 @@ public abstract class AbstractSessionManager implements SessionManager { public static final int RUN_DELAY = 20; public static final long SESSION_LIFETIME = 10; - private static final BiPredicate BYPASS_PERMISSION_TEST = (worldName, player) -> player.hasPermission("worldguard.region.bypass." + worldName); + private static final BiPredicate BYPASS_PERMISSION_TEST = (world, player) -> player.hasPermission("worldguard.region.bypass." + world.getName()); private final Cache bypassCache = CacheBuilder.newBuilder() .maximumSize(1000) @@ -138,11 +138,11 @@ public boolean hasBypass(LocalPlayer player, World world) { } if (WorldGuard.getInstance().getPlatform().getGlobalStateManager().disablePermissionCache) { - return BYPASS_PERMISSION_TEST.test(world.getName(), player); + return BYPASS_PERMISSION_TEST.test(world, player); } var bypassCacheKey = new BypassCacheKey(world.getName(), player.getUniqueId()); - return bypassCache.asMap().computeIfAbsent(bypassCacheKey, (_) -> BYPASS_PERMISSION_TEST.test(world.getName(), player)); + return bypassCache.asMap().computeIfAbsent(bypassCacheKey, (_) -> BYPASS_PERMISSION_TEST.test(world, player)); } @Override