From 04b5a4f166cff7aff5126811d96f43905e2fc81b Mon Sep 17 00:00:00 2001 From: themode Date: Mon, 22 Feb 2021 08:41:38 +0100 Subject: [PATCH] Use google common cache instead of our own dirty solution --- .../net/minestom/server/MinecraftServer.java | 2 - .../network/player/NettyPlayerConnection.java | 9 ++--- .../server/utils/cache/CacheablePacket.java | 10 +---- .../server/utils/cache/TemporaryCache.java | 38 +++++-------------- 4 files changed, 14 insertions(+), 45 deletions(-) diff --git a/src/main/java/net/minestom/server/MinecraftServer.java b/src/main/java/net/minestom/server/MinecraftServer.java index be57cb1ff..2dff72bf5 100644 --- a/src/main/java/net/minestom/server/MinecraftServer.java +++ b/src/main/java/net/minestom/server/MinecraftServer.java @@ -44,7 +44,6 @@ import net.minestom.server.storage.StorageManager; import net.minestom.server.timer.SchedulerManager; import net.minestom.server.utils.MathUtils; import net.minestom.server.utils.PacketUtils; -import net.minestom.server.utils.cache.TemporaryCache; import net.minestom.server.utils.thread.MinestomThread; import net.minestom.server.utils.validate.Check; import net.minestom.server.world.Difficulty; @@ -794,7 +793,6 @@ public final class MinecraftServer { LOGGER.info("Shutting down all thread pools."); benchmarkManager.disable(); commandManager.stopConsoleThread(); - TemporaryCache.REMOVER_SERVICE.shutdown(); MinestomThread.shutdownAll(); LOGGER.info("Minestom server stopped successfully."); } diff --git a/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java b/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java index 43b2cfc37..b60ae5794 100644 --- a/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java +++ b/src/main/java/net/minestom/server/network/player/NettyPlayerConnection.java @@ -121,21 +121,20 @@ public class NettyPlayerConnection extends PlayerConnection { if (getPlayer() != null) { // Flush happen during #update() if (serverPacket instanceof CacheablePacket && MinecraftServer.hasPacketCaching()) { - CacheablePacket cacheablePacket = (CacheablePacket) serverPacket; + final CacheablePacket cacheablePacket = (CacheablePacket) serverPacket; final UUID identifier = cacheablePacket.getIdentifier(); if (identifier == null) { - // This packet explicitly said to do not retrieve the cache + // This packet explicitly asks to do not retrieve the cache write(serverPacket); } else { // Try to retrieve the cached buffer TemporaryCache temporaryCache = cacheablePacket.getCache(); - ByteBuf buffer = temporaryCache.retrieve(identifier, cacheablePacket.getLastUpdateTime()); + ByteBuf buffer = temporaryCache.retrieve(identifier); if (buffer == null) { // Buffer not found, create and cache it - final long time = System.currentTimeMillis(); buffer = PacketUtils.createFramedPacket(serverPacket, false); - temporaryCache.cacheObject(identifier, buffer, time); + temporaryCache.cache(identifier, buffer); } FramedPacket framedPacket = new FramedPacket(buffer); write(framedPacket); diff --git a/src/main/java/net/minestom/server/utils/cache/CacheablePacket.java b/src/main/java/net/minestom/server/utils/cache/CacheablePacket.java index 9c32f6103..38729c77b 100644 --- a/src/main/java/net/minestom/server/utils/cache/CacheablePacket.java +++ b/src/main/java/net/minestom/server/utils/cache/CacheablePacket.java @@ -10,8 +10,7 @@ import java.util.UUID; * Implemented by {@link ServerPacket server packets} which can be temporary cached in memory to be re-sent later * without having to go through all the writing and compression. *

- * {@link #getIdentifier()} is to differenciate this packet from the others of the same type, - * and {@link #getLastUpdateTime()} to know if one packet is newer than the previous one. + * {@link #getIdentifier()} is to differentiate this packet from the others of the same type. */ public interface CacheablePacket { @@ -35,11 +34,4 @@ public interface CacheablePacket { @Nullable UUID getIdentifier(); - /** - * Gets the last time this packet changed. - * - * @return the last packet update time in milliseconds - */ - long getLastUpdateTime(); - } diff --git a/src/main/java/net/minestom/server/utils/cache/TemporaryCache.java b/src/main/java/net/minestom/server/utils/cache/TemporaryCache.java index 4627d379a..d4aa43251 100644 --- a/src/main/java/net/minestom/server/utils/cache/TemporaryCache.java +++ b/src/main/java/net/minestom/server/utils/cache/TemporaryCache.java @@ -1,12 +1,11 @@ package net.minestom.server.utils.cache; +import com.google.common.cache.Cache; +import com.google.common.cache.CacheBuilder; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import java.util.UUID; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.Executors; -import java.util.concurrent.ScheduledExecutorService; import java.util.concurrent.TimeUnit; /** @@ -16,13 +15,7 @@ import java.util.concurrent.TimeUnit; */ public class TemporaryCache { - public static final ScheduledExecutorService REMOVER_SERVICE = Executors.newScheduledThreadPool(1); - - // Identifier = Cached object - protected ConcurrentHashMap cache = new ConcurrentHashMap<>(); - // Identifier = time - protected ConcurrentHashMap cacheTime = new ConcurrentHashMap<>(); - + private final Cache cache; private final long keepTime; /** @@ -33,12 +26,9 @@ public class TemporaryCache { */ public TemporaryCache(long keepTime) { this.keepTime = keepTime; - REMOVER_SERVICE.scheduleAtFixedRate(() -> { - final boolean removed = cacheTime.values().removeIf(time -> System.currentTimeMillis() > time + keepTime); - if (removed) { - this.cache.entrySet().removeIf(entry -> !cacheTime.containsKey(entry.getKey())); - } - }, keepTime, keepTime, TimeUnit.MILLISECONDS); + this.cache = CacheBuilder.newBuilder() + .expireAfterWrite(keepTime, TimeUnit.MILLISECONDS) + .build(); } /** @@ -46,30 +36,20 @@ public class TemporaryCache { * * @param identifier the object identifier * @param value the object to cache - * @param time the current time in milliseconds */ - public void cacheObject(@NotNull UUID identifier, T value, long time) { + public void cache(@NotNull UUID identifier, T value) { this.cache.put(identifier, value); - this.cacheTime.put(identifier, time); } /** * Retrieves an object from cache. * * @param identifier the object identifier - * @param lastUpdate the last update time of your identifier's object, - * used to see if the cached value is up-to-date * @return the retrieved object or null if not found */ @Nullable - public T retrieve(@NotNull UUID identifier, long lastUpdate) { - Long tempL = cacheTime.get(identifier); - if (tempL == null) { - return null; - } - - //cache.get(identifier) will return null if the race condition occurred which is what we want - return lastUpdate <= tempL ? cache.get(identifier) : null; + public T retrieve(@NotNull UUID identifier) { + return cache.getIfPresent(identifier); } /**