From b39097e01594ccdba18ff13b2557553f7a666141 Mon Sep 17 00:00:00 2001
From: Andrew Wong <42793301+md5sha256@users.noreply.github.com>
Date: Thu, 18 Apr 2024 23:57:10 +0800
Subject: [PATCH] Refactor ticker task to use a thread-safe Set implementation
 for tickingLocations

---
 .../implementation/tasks/TickerTask.java      | 28 +++++++++++++++----
 1 file changed, 22 insertions(+), 6 deletions(-)

diff --git a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/tasks/TickerTask.java b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/tasks/TickerTask.java
index 76589bba84..51541f01cf 100644
--- a/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/tasks/TickerTask.java
+++ b/src/main/java/io/github/thebusybiscuit/slimefun4/implementation/tasks/TickerTask.java
@@ -35,6 +35,7 @@ public class TickerTask implements Runnable {
 
     /**
      * This Map holds all currently actively ticking locations.
+     * The value of this map (Set entries) MUST be thread-safe and mutable.
      */
     private final Map<ChunkPosition, Set<Location>> tickingLocations = new ConcurrentHashMap<>();
 
@@ -255,7 +256,7 @@ public Map<ChunkPosition, Set<Location>> getLocations() {
     public Set<Location> getLocations(@Nonnull Chunk chunk) {
         Validate.notNull(chunk, "The Chunk cannot be null!");
 
-        Set<Location> locations = tickingLocations.getOrDefault(new ChunkPosition(chunk), new HashSet<>());
+        Set<Location> locations = tickingLocations.getOrDefault(new ChunkPosition(chunk), Collections.emptySet());
         return Collections.unmodifiableSet(locations);
     }
 
@@ -269,11 +270,26 @@ public void enableTicker(@Nonnull Location l) {
         Validate.notNull(l, "Location cannot be null!");
 
         synchronized (tickingLocations) {
-            tickingLocations
-                    .computeIfAbsent(
-                            new ChunkPosition(l.getWorld(), l.getBlockX() >> 4, l.getBlockZ() >> 4),
-                            k -> new HashSet<>())
-                    .add(l);
+            ChunkPosition chunk = new ChunkPosition(l.getWorld(), l.getBlockX() >> 4, l.getBlockZ() >> 4);
+
+            /*
+              Note that all the values in #tickingLocations must be thread-safe.
+              Thus, the choice is between the CHM KeySet or a synchronized set.
+              The CHM KeySet was chosen since it at least permits multiple concurrent
+              reads without blocking.
+            */
+            Set<Location> newValue = ConcurrentHashMap.newKeySet();
+            Set<Location> oldValue = tickingLocations.putIfAbsent(chunk, newValue);
+
+            /**
+             * This is faster than doing computeIfAbsent(...)
+             * on a ConcurrentHashMap because it won't block the Thread for too long
+             */
+            if (oldValue != null) {
+                oldValue.add(l);
+            } else {
+                newValue.add(l);
+            }
         }
     }