[#231, #234, #230] Minor cache revamp (#239)

feat(#231): add 'enableDiskCache' option
feat(#234): remove chunk invaldation from game thread
feat(#230): fix some rare NPE in plugin shutdown

orebfuscator-common/src/main/java/net/imprex/orebfuscator/config/CacheConfig.java

@@ -7,44 +7,20 @@
 public interface CacheConfig {
 
 	boolean enabled();
-	void enabled(boolean enabled);
 
 	Path baseDirectory();
 
 	Path regionFile(ChunkPosition chunkPosition);
 
 	int maximumOpenRegionFiles();
-	/**
-	 * @param count
-	 * @throws IllegalArgumentException When the count value is lower than one
-	 */
-	void maximumOpenRegionFiles(int count);
 
 	long deleteRegionFilesAfterAccess();
-	/**
-	 * @param expire
-	 * @throws IllegalArgumentException When the expire value is lower than one
-	 */
-	void deleteRegionFilesAfterAccess(long expire);
+
+	boolean enableDiskCache();
 
 	int maximumSize();
-	/**
-	 * @param size
-	 * @throws IllegalArgumentException When the size value is lower than one
-	 */
-	void maximumSize(int size);
 
 	long expireAfterAccess();
-	/**
-	 * @param expire
-	 * @throws IllegalArgumentException When the expire value is lower than one
-	 */
-	void expireAfterAccess(long expire);
 
 	int maximumTaskQueueSize();
-	/**
-	 * @param size
-	 * @throws IllegalArgumentException When the expire value is lower than one
-	 */
-	void maximumTaskQueueSize(int size);
 }

orebfuscator-common/src/main/java/net/imprex/orebfuscator/config/GeneralConfig.java

@@ -3,17 +3,12 @@
 public interface GeneralConfig {
 
 	boolean checkForUpdates();
-	void checkForUpdates(boolean enabled);
 
 	boolean updateOnBlockDamage();
-	void updateOnBlockDamage(boolean enabled);
 
 	boolean bypassNotification();
-	void bypassNotification(boolean enabled);
 
 	boolean ignoreSpectator();
-	void ignoreSpectator(boolean value);
 
 	int updateRadius();
-	void updateRadius(int radius);
 }

orebfuscator-plugin/src/main/java/net/imprex/orebfuscator/Orebfuscator.java

@@ -47,7 +47,7 @@ public void onEnable() {
 			this.config = new OrebfuscatorConfig(this);
 
 			// Check if HeightAccessor can be loaded
-			HeightAccessor.thisMethodIsUsedToInitializeStaticFields();
+			HeightAccessor.thisMethodIsUsedToInitializeStaticFieldsEarly();
 
 			// Initialize metrics
 			new MetricsSystem(this);
@@ -92,11 +92,15 @@ public void onEnable() {
 
 	@Override
 	public void onDisable() {
-		this.obfuscationCache.close();
+		if (this.obfuscationCache != null) {
+			this.obfuscationCache.close();
+		}
 
-		this.obfuscationSystem.shutdown();
+		if (this.obfuscationSystem != null) {
+			this.obfuscationSystem.shutdown();
+		}
 
-		if (this.config.proximityEnabled()) {
+		if (this.config.proximityEnabled() && this.proximityPacketListener != null && this.proximityHider != null) {
 			this.proximityPacketListener.unregister();
 			this.proximityHider.close();
 		}

orebfuscator-plugin/src/main/java/net/imprex/orebfuscator/cache/AsyncChunkSerializer.java

@@ -14,6 +14,17 @@
 import net.imprex.orebfuscator.obfuscation.ObfuscationResult;
 import net.imprex.orebfuscator.util.ChunkPosition;
 
+/**
+ * This class works similar to a bounded buffer for cache read and write
+ * requests but also functions as the only consumer of said buffer. All requests
+ * can get reorder similar to modern memory access reordering in CPUs. If for
+ * example a write request is already in the buffer and a new read request for
+ * the same position is created then the read request doesn't get put in the
+ * buffer and gets completed with the content of the write request.
+ * 
+ * @see <a href="https://en.wikipedia.org/wiki/Producer–consumer_problem">Bound buffer</a>
+ * @see <a href="https://en.wikipedia.org/wiki/Memory_ordering">Memory ordering</a>
+ */
 public class AsyncChunkSerializer implements Runnable {
 
 	private final Lock lock = new ReentrantLock(true);
@@ -45,7 +56,7 @@ public CompletableFuture<ObfuscationResult> read(ChunkPosition position) {
 				return ((ReadTask) task).future;
 			} else {
 				CompletableFuture<ObfuscationResult> future = new CompletableFuture<>();
-				this.queueTask(position, new ReadTask(position, future), true);
+				this.queueTask(position, new ReadTask(position, future));
 				return future;
 			}
 		} finally {
@@ -56,7 +67,7 @@ public CompletableFuture<ObfuscationResult> read(ChunkPosition position) {
 	public void write(ChunkPosition position, ObfuscationResult chunk) {
 		this.lock.lock();
 		try {
-			Runnable prevTask = this.queueTask(position, new WriteTask(position, chunk), true);
+			Runnable prevTask = this.queueTask(position, new WriteTask(position, chunk));
 			if (prevTask instanceof ReadTask) {
 				((ReadTask) prevTask).future.complete(chunk);
 			}
@@ -65,20 +76,8 @@ public void write(ChunkPosition position, ObfuscationResult chunk) {
 		}
 	}
 
-	public void invalidate(ChunkPosition position) {
-		this.lock.lock();
-		try {
-			Runnable prevTask = this.queueTask(position, new WriteTask(position, null), false);
-			if (prevTask instanceof ReadTask) {
-				((ReadTask) prevTask).future.complete(null);
-			}
-		} finally {
-			this.lock.unlock();
-		}
-	}
-
-	private Runnable queueTask(ChunkPosition position, Runnable nextTask, boolean considerSize) {
-		while (this.positions.size() >= this.maxTaskQueueSize && considerSize) {
+	private Runnable queueTask(ChunkPosition position, Runnable nextTask) {
+		while (this.positions.size() >= this.maxTaskQueueSize) {
 			this.notFull.awaitUninterruptibly();
 		}
 
@@ -100,7 +99,7 @@ public void run() {
 		while (this.running) {
 			this.lock.lock();
 			try {
-				if (this.positions.isEmpty()) {
+				while (this.positions.isEmpty()) {
 					this.notEmpty.await();
 				}
 

orebfuscator-plugin/src/main/java/net/imprex/orebfuscator/cache/CacheCleanTask.java → orebfuscator-plugin/src/main/java/net/imprex/orebfuscator/cache/CacheFileCleanupTask.java

@@ -11,12 +11,15 @@
 import net.imprex.orebfuscator.Orebfuscator;
 import net.imprex.orebfuscator.config.CacheConfig;
 import net.imprex.orebfuscator.nms.AbstractRegionFileCache;
+import net.imprex.orebfuscator.util.OFCLogger;
 
-public class CacheCleanTask implements Runnable {
+public class CacheFileCleanupTask implements Runnable {
 
 	private final CacheConfig cacheConfig;
 
-	public CacheCleanTask(Orebfuscator orebfuscator) {
+	private int deleteCount = 0;
+
+	public CacheFileCleanupTask(Orebfuscator orebfuscator) {
 		this.cacheConfig = orebfuscator.getOrebfuscatorConfig().cache();
 	}
 
@@ -25,6 +28,8 @@ public void run() {
 		long deleteAfterMillis = this.cacheConfig.deleteRegionFilesAfterAccess();
 		AbstractRegionFileCache<?> regionFileCache = NmsInstance.getRegionFileCache();
 
+		this.deleteCount = 0;
+
 		try {
 			Files.walkFileTree(this.cacheConfig.baseDirectory(), new SimpleFileVisitor<Path>() {
 
@@ -33,12 +38,19 @@ public FileVisitResult visitFile(Path path, BasicFileAttributes attributes) thro
 					if (System.currentTimeMillis() - attributes.lastAccessTime().toMillis() > deleteAfterMillis) {
 						regionFileCache.close(path);
 						Files.delete(path);
+
+						CacheFileCleanupTask.this.deleteCount++;
+						OFCLogger.debug("deleted cache file: " + path);
 					}
 					return FileVisitResult.CONTINUE;
 				}
 			});
 		} catch (IOException e) {
 			e.printStackTrace();
 		}
+
+		if (this.deleteCount > 0) {
+			OFCLogger.info(String.format("CacheFileCleanupTask successfully deleted %d cache file(s)", this.deleteCount));
+		}
 	}
 }

orebfuscator-plugin/src/main/java/net/imprex/orebfuscator/cache/ObfuscationCache.java

@@ -29,16 +29,22 @@ public ObfuscationCache(Orebfuscator orebfuscator) {
 				.expireAfterAccess(this.cacheConfig.expireAfterAccess(), TimeUnit.MILLISECONDS)
 				.removalListener(this::onRemoval).build();
 
-		this.serializer = new AsyncChunkSerializer(orebfuscator);
+		if (this.cacheConfig.enableDiskCache()) {
+			this.serializer = new AsyncChunkSerializer(orebfuscator);
+		} else {
+			this.serializer = null;
+		}
 
 		if (this.cacheConfig.enabled() && this.cacheConfig.deleteRegionFilesAfterAccess() > 0) {
-			Bukkit.getScheduler().runTaskTimerAsynchronously(orebfuscator, new CacheCleanTask(orebfuscator), 0,
+			Bukkit.getScheduler().runTaskTimerAsynchronously(orebfuscator, new CacheFileCleanupTask(orebfuscator), 0,
 					3_600_000L);
 		}
 	}
 
 	private void onRemoval(RemovalNotification<ChunkPosition, ObfuscationResult> notification) {
-		if (notification.wasEvicted()) {
+		// don't serialize invalidated chunks since this would require locking the main
+		// thread and wouldn't bring a huge improvement
+		if (this.cacheConfig.enableDiskCache() && notification.wasEvicted()) {
 			this.serializer.write(notification.getKey(), notification.getValue());
 		}
 	}
@@ -51,32 +57,48 @@ public CompletionStage<ObfuscationResult> get(ObfuscationRequest request) {
 			return request.complete(cacheChunk);
 		}
 
-		this.serializer.read(key).thenCompose(diskChunk -> {
-			if (request.isValid(diskChunk)) {
-				return request.complete(diskChunk);
-			} else {
-				return request.submitForObfuscation().exceptionally(throwable -> null);
-			}
-		}).thenAccept(chunk -> {
-			if (chunk != null) {
-				this.cache.put(key, chunk);
-			}
-		});
+		if (this.cacheConfig.enableDiskCache()) {
+
+			// compose async in order for the serializer to continue its work
+			this.serializer.read(key).thenComposeAsync(diskChunk -> {
+				if (request.isValid(diskChunk)) {
+					return request.complete(diskChunk);
+				} else {
+					// ignore exception and return null
+					return request.submitForObfuscation().exceptionally(throwable -> null);
+				}
+			}).thenAccept(chunk -> {
+				// if successful add chunk to in-memory cache
+				if (chunk != null) {
+					this.cache.put(key, chunk);
+				}
+			});
+		} else {
+
+			request.submitForObfuscation().thenAccept(chunk -> {
+				// if successful add chunk to in-memory cache
+				if (chunk != null) {
+					this.cache.put(key, chunk);
+				}
+			});
+		}
 
 		return request.getFuture();
 	}
 
 	public void invalidate(ChunkPosition key) {
 		this.cache.invalidate(key);
-		this.serializer.invalidate(key);
 	}
 
 	public void close() {
-		this.cache.asMap().entrySet().removeIf(entry -> {
-			this.serializer.write(entry.getKey(), entry.getValue());
-			return true;
-		});
-
-		this.serializer.close();
+		if (this.cacheConfig.enableDiskCache()) {
+			// flush memory cache to disk on shutdown
+			this.cache.asMap().entrySet().removeIf(entry -> {
+				this.serializer.write(entry.getKey(), entry.getValue());
+				return true;
+			});
+
+			this.serializer.close();
+		}
 	}
 }