ratelimit chunk checks and fix scheduling

src/main/java/me/xginko/villageroptimizer/modules/VillagerChunkLimit.java

@@ -1,6 +1,7 @@
 package me.xginko.villageroptimizer.modules;
 
 import com.cryptomorin.xseries.XEntityType;
+import me.xginko.villageroptimizer.utils.ExpiringSet;
 import me.xginko.villageroptimizer.utils.LocationUtil;
 import me.xginko.villageroptimizer.utils.Util;
 import org.bukkit.Chunk;
@@ -16,6 +17,7 @@ import org.bukkit.event.player.PlayerInteractEntityEvent;
 import org.jetbrains.annotations.NotNull;
 import space.arim.morepaperlib.scheduling.ScheduledTask;
 
+import java.time.Duration;
 import java.util.ArrayList;
 import java.util.Comparator;
 import java.util.List;
@@ -27,6 +29,7 @@ public class VillagerChunkLimit extends VillagerOptimizerModule implements Runna
 
     private ScheduledTask periodic_chunk_check;
     private final List<Villager.Profession> non_optimized_removal_priority, optimized_removal_priority;
+    private final ExpiringSet<Chunk> checked_chunks;
     private final long check_period;
     private final int non_optimized_max_per_chunk, optimized_max_per_chunk;
     private final boolean log_enabled, skip_unloaded_chunks;
@@ -44,6 +47,10 @@ public class VillagerChunkLimit extends VillagerOptimizerModule implements Runna
         this.log_enabled = config.getBoolean(configPath + ".log-removals", true);
         this.non_optimized_max_per_chunk = config.getInt(configPath + ".unoptimized.max-per-chunk", 20,
                 "The maximum amount of unoptimized villagers per chunk.");
+        this.checked_chunks = new ExpiringSet<>(Duration.ofSeconds(
+                Math.max(1, config.getInt(configPath + ".chunk-check-cooldown-seconds", 5,
+                        "The delay in seconds a chunk will not be checked again after the first time.\n" +
+                                "Reduces chances to lag the server due to overchecking."))));
         final List<String> defaults = Stream.of(
                 "NONE", "NITWIT", "SHEPHERD", "FISHERMAN", "BUTCHER", "CARTOGRAPHER", "LEATHERWORKER",
                 "FLETCHER", "MASON", "FARMER", "ARMORER", "TOOLSMITH", "WEAPONSMITH", "CLERIC", "LIBRARIAN")
@@ -111,9 +118,11 @@ public class VillagerChunkLimit extends VillagerOptimizerModule implements Runna
     public void run() {
         for (World world : plugin.getServer().getWorlds()) {
             for (Chunk chunk : world.getLoadedChunks()) {
-                if (!skip_unloaded_chunks || Util.isChunkLoaded(chunk)) {
+                scheduling.regionSpecificScheduler(chunk.getWorld(), chunk.getX(), chunk.getZ()).run(() -> {
-                    manageVillagerCount(chunk);
+                    if (!skip_unloaded_chunks || Util.isChunkLoaded(chunk)) {
-                }
+                        manageVillagerCount(chunk);
+                    }
+                });
             }
         }
     }
@@ -121,72 +130,78 @@ public class VillagerChunkLimit extends VillagerOptimizerModule implements Runna
     @EventHandler(priority = EventPriority.HIGHEST, ignoreCancelled = true)
     private void onCreatureSpawn(CreatureSpawnEvent event) {
         if (event.getEntityType() == XEntityType.VILLAGER.get()) {
-            manageVillagerCount(event.getEntity().getChunk());
+            scheduling.regionSpecificScheduler(event.getLocation()).run(() -> {
+                manageVillagerCount(event.getEntity().getChunk());
+            });
         }
     }
 
-    @EventHandler(priority = EventPriority.LOW, ignoreCancelled = true)
+    @EventHandler(priority = EventPriority.LOWEST, ignoreCancelled = true)
     private void onInteract(PlayerInteractEntityEvent event) {
         if (event.getRightClicked().getType() == XEntityType.VILLAGER.get()) {
-            manageVillagerCount(event.getRightClicked().getChunk());
+            scheduling.regionSpecificScheduler(event.getRightClicked().getLocation()).run(() -> {
+                manageVillagerCount(event.getRightClicked().getChunk());
+            });
         }
     }
 
     private void manageVillagerCount(@NotNull Chunk chunk) {
-        scheduling.regionSpecificScheduler(chunk.getWorld(), chunk.getX(), chunk.getZ()).run(() -> {
+        // Remember which chunk we have already checked
-            // Collect all optimized and unoptimized villagers in that chunk
+        if (checked_chunks.contains(chunk)) return;
-            List<Villager> optimized_villagers = new ArrayList<>();
+        else checked_chunks.add(chunk);
-            List<Villager> not_optimized_villagers = new ArrayList<>();
 
-            for (Entity entity : chunk.getEntities()) {
+        // Collect all optimized and unoptimized villagers in that chunk
-                if (entity.getType() != XEntityType.VILLAGER.get()) continue;
+        List<Villager> optimized_villagers = new ArrayList<>();
+        List<Villager> not_optimized_villagers = new ArrayList<>();
 
-                Villager villager = (Villager) entity;
+        for (Entity entity : chunk.getEntities()) {
+            if (entity.getType() != XEntityType.VILLAGER.get()) continue;
 
-                if (villagerCache.createIfAbsent(villager).isOptimized()) {
+            Villager villager = (Villager) entity;
-                    optimized_villagers.add(villager);
+
-                } else {
+            if (villagerCache.createIfAbsent(villager).isOptimized()) {
-                    not_optimized_villagers.add(villager);
+                optimized_villagers.add(villager);
-                }
+            } else {
+                not_optimized_villagers.add(villager);
             }
+        }
 
-            // Check if there are more unoptimized villagers in that chunk than allowed
+        // Check if there are more unoptimized villagers in that chunk than allowed
-            final int not_optimized_villagers_too_many = not_optimized_villagers.size() - non_optimized_max_per_chunk;
+        final int not_optimized_villagers_too_many = not_optimized_villagers.size() - non_optimized_max_per_chunk;
-            if (not_optimized_villagers_too_many > 0) {
+        if (not_optimized_villagers_too_many > 0) {
-                // Sort villagers by profession priority
+            // Sort villagers by profession priority
-                not_optimized_villagers.sort(Comparator.comparingInt(villager -> {
+            not_optimized_villagers.sort(Comparator.comparingInt(villager -> {
-                    final Villager.Profession profession = villager.getProfession();
+                final Villager.Profession profession = villager.getProfession();
-                    return non_optimized_removal_priority.contains(profession) ? non_optimized_removal_priority.indexOf(profession) : Integer.MAX_VALUE;
+                return non_optimized_removal_priority.contains(profession) ? non_optimized_removal_priority.indexOf(profession) : Integer.MAX_VALUE;
-                }));
+            }));
-                // Remove prioritized villagers that are too many
+            // Remove prioritized villagers that are too many
-                for (int i = 0; i < not_optimized_villagers_too_many; i++) {
+            for (int i = 0; i < not_optimized_villagers_too_many; i++) {
-                    Villager villager = not_optimized_villagers.get(i);
+                Villager villager = not_optimized_villagers.get(i);
-                    scheduling.entitySpecificScheduler(villager).run(kill -> {
+                scheduling.entitySpecificScheduler(villager).run(kill -> {
-                        villager.remove();
+                    villager.remove();
-                        if (log_enabled) info("Removed unoptimized villager with profession '" +
+                    if (log_enabled) info("Removed unoptimized villager with profession '" +
-                                Util.formatEnum(villager.getProfession()) + "' at " + LocationUtil.toString(villager.getLocation()));
+                            Util.formatEnum(villager.getProfession()) + "' at " + LocationUtil.toString(villager.getLocation()));
-                    }, null);
+                }, null);
-                }
             }
+        }
 
-            // Check if there are more optimized villagers in that chunk than allowed
+        // Check if there are more optimized villagers in that chunk than allowed
-            final int optimized_villagers_too_many = optimized_villagers.size() - optimized_max_per_chunk;
+        final int optimized_villagers_too_many = optimized_villagers.size() - optimized_max_per_chunk;
-            if (optimized_villagers_too_many > 0) {
+        if (optimized_villagers_too_many > 0) {
-                // Sort villagers by profession priority
+            // Sort villagers by profession priority
-                optimized_villagers.sort(Comparator.comparingInt(villager -> {
+            optimized_villagers.sort(Comparator.comparingInt(villager -> {
-                    final Villager.Profession profession = villager.getProfession();
+                final Villager.Profession profession = villager.getProfession();
-                    return optimized_removal_priority.contains(profession) ? optimized_removal_priority.indexOf(profession) : Integer.MAX_VALUE;
+                return optimized_removal_priority.contains(profession) ? optimized_removal_priority.indexOf(profession) : Integer.MAX_VALUE;
-                }));
+            }));
-                // Remove prioritized villagers that are too many
+            // Remove prioritized villagers that are too many
-                for (int i = 0; i < optimized_villagers_too_many; i++) {
+            for (int i = 0; i < optimized_villagers_too_many; i++) {
-                    Villager villager = optimized_villagers.get(i);
+                Villager villager = optimized_villagers.get(i);
-                    scheduling.entitySpecificScheduler(villager).run(kill -> {
+                scheduling.entitySpecificScheduler(villager).run(kill -> {
-                        villager.remove();
+                    villager.remove();
-                        if (log_enabled) info("Removed unoptimized villager with profession '" +
+                    if (log_enabled) info("Removed unoptimized villager with profession '" +
-                                Util.formatEnum(villager.getProfession()) + "' at " + LocationUtil.toString(villager.getLocation()));
+                            Util.formatEnum(villager.getProfession()) + "' at " + LocationUtil.toString(villager.getLocation()));
-                    }, null);
+                }, null);
-                }
             }
-        });
+        }
     }
 }

src/main/java/me/xginko/villageroptimizer/utils/ExpiringSet.java

@@ -0,0 +1,349 @@
+package me.xginko.villageroptimizer.utils;
+
+import com.github.benmanes.caffeine.cache.Cache;
+import com.github.benmanes.caffeine.cache.Caffeine;
+import org.jetbrains.annotations.NotNull;
+
+import java.time.Duration;
+import java.util.AbstractSet;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.Set;
+import java.util.concurrent.TimeUnit;
+
+public final class ExpiringSet<E> extends AbstractSet<E> implements Set<E> {
+
+    private final Cache<E, Object> cache;
+    private static final Object PRESENT = new Object(); // Dummy value to associate with an Object in the backing Cache
+
+    public ExpiringSet(long duration, TimeUnit unit) {
+        this.cache = Caffeine.newBuilder().expireAfterWrite(duration, unit).build();
+    }
+
+    public ExpiringSet(Duration duration) {
+        this.cache = Caffeine.newBuilder().expireAfterWrite(duration).build();
+    }
+
+    /**
+     * Returns the number of elements in this set (its cardinality).  If this
+     * set contains more than {@code Integer.MAX_VALUE} elements, returns
+     * {@code Integer.MAX_VALUE}.
+     *
+     * @return the number of elements in this set (its cardinality)
+     */
+    @Override
+    public int size() {
+        return this.cache.asMap().size();
+    }
+
+    /**
+     * Returns {@code true} if this set contains no elements.
+     *
+     * @return {@code true} if this set contains no elements
+     */
+    @Override
+    public boolean isEmpty() {
+        return this.cache.asMap().isEmpty();
+    }
+
+    /**
+     * Returns {@code true} if this set contains the specified element.
+     * More formally, returns {@code true} if and only if this set
+     * contains an element {@code e} such that
+     * {@code Objects.equals(o, e)}.
+     *
+     * @param item element whose presence in this set is to be tested
+     * @return {@code true} if this set contains the specified element
+     * @throws ClassCastException   if the type of the specified element
+     *                              is incompatible with this set
+     *                              (<a href="Collection.html#optional-restrictions">optional</a>)
+     * @throws NullPointerException if the specified element is null and this
+     *                              set does not permit null elements
+     *                              (<a href="Collection.html#optional-restrictions">optional</a>)
+     */
+    @Override
+    public boolean contains(Object item) {
+        return this.cache.getIfPresent((E) item) != null;
+    }
+
+    /**
+     * Returns an iterator over the elements in this set.  The elements are
+     * returned in no particular order (unless this set is an instance of some
+     * class that provides a guarantee).
+     *
+     * @return an iterator over the elements in this set
+     */
+    @Override
+    public @NotNull Iterator<E> iterator() {
+        return this.cache.asMap().keySet().iterator();
+    }
+
+    /**
+     * Returns an array containing all of the elements in this set.
+     * If this set makes any guarantees as to what order its elements
+     * are returned by its iterator, this method must return the
+     * elements in the same order.
+     *
+     * <p>The returned array will be "safe" in that no references to it
+     * are maintained by this set.  (In other words, this method must
+     * allocate a new array even if this set is backed by an array).
+     * The caller is thus free to modify the returned array.
+     *
+     * <p>This method acts as bridge between array-based and collection-based
+     * APIs.
+     *
+     * @return an array containing all the elements in this set
+     */
+    @Override
+    public @NotNull Object @NotNull [] toArray() {
+        return this.cache.asMap().keySet().toArray();
+    }
+
+    /**
+     * Returns an array containing all of the elements in this set; the
+     * runtime type of the returned array is that of the specified array.
+     * If the set fits in the specified array, it is returned therein.
+     * Otherwise, a new array is allocated with the runtime type of the
+     * specified array and the size of this set.
+     *
+     * <p>If this set fits in the specified array with room to spare
+     * (i.e., the array has more elements than this set), the element in
+     * the array immediately following the end of the set is set to
+     * {@code null}.  (This is useful in determining the length of this
+     * set <i>only</i> if the caller knows that this set does not contain
+     * any null elements.)
+     *
+     * <p>If this set makes any guarantees as to what order its elements
+     * are returned by its iterator, this method must return the elements
+     * in the same order.
+     *
+     * <p>Like the {@link #toArray()} method, this method acts as bridge between
+     * array-based and collection-based APIs.  Further, this method allows
+     * precise control over the runtime type of the output array, and may,
+     * under certain circumstances, be used to save allocation costs.
+     *
+     * <p>Suppose {@code x} is a set known to contain only strings.
+     * The following code can be used to dump the set into a newly allocated
+     * array of {@code String}:
+     *
+     * <pre>
+     *     String[] y = x.toArray(new String[0]);</pre>
+     * <p>
+     * Note that {@code toArray(new Object[0])} is identical in function to
+     * {@code toArray()}.
+     *
+     * @param a the array into which the elements of this set are to be
+     *          stored, if it is big enough; otherwise, a new array of the same
+     *          runtime type is allocated for this purpose.
+     * @return an array containing all the elements in this set
+     * @throws ArrayStoreException  if the runtime type of the specified array
+     *                              is not a supertype of the runtime type of every element in this
+     *                              set
+     * @throws NullPointerException if the specified array is null
+     */
+    @Override
+    public @NotNull <T> T @NotNull [] toArray(@NotNull T @NotNull [] a) {
+        return this.cache.asMap().keySet().toArray(a);
+    }
+
+    /**
+     * Adds the specified element to this set if it is not already present
+     * (optional operation).  More formally, adds the specified element
+     * {@code e} to this set if the set contains no element {@code e2}
+     * such that
+     * {@code Objects.equals(e, e2)}.
+     * If this set already contains the element, the call leaves the set
+     * unchanged and returns {@code false}.  In combination with the
+     * restriction on constructors, this ensures that sets never contain
+     * duplicate elements.
+     *
+     * <p>The stipulation above does not imply that sets must accept all
+     * elements; sets may refuse to add any particular element, including
+     * {@code null}, and throw an exception, as described in the
+     * specification for {@link Collection#add Collection.add}.
+     * Individual set implementations should clearly document any
+     * restrictions on the elements that they may contain.
+     *
+     * @param item element to be added to this set
+     * @return {@code true} if this set did not already contain the specified
+     * element
+     * @throws UnsupportedOperationException if the {@code add} operation
+     *                                       is not supported by this set
+     * @throws ClassCastException            if the class of the specified element
+     *                                       prevents it from being added to this set
+     * @throws NullPointerException          if the specified element is null and this
+     *                                       set does not permit null elements
+     * @throws IllegalArgumentException      if some property of the specified element
+     *                                       prevents it from being added to this set
+     */
+    public boolean add(E item) {
+        boolean containedBefore = contains(item);
+        this.cache.put(item, PRESENT);
+        return !containedBefore;
+    }
+
+    /**
+     * Removes the specified element from this set if it is present
+     * (optional operation).  More formally, removes an element {@code e}
+     * such that
+     * {@code Objects.equals(o, e)}, if
+     * this set contains such an element.  Returns {@code true} if this set
+     * contained the element (or equivalently, if this set changed as a
+     * result of the call).  (This set will not contain the element once the
+     * call returns.)
+     *
+     * @param o object to be removed from this set, if present
+     * @return {@code true} if this set contained the specified element
+     * @throws ClassCastException            if the type of the specified element
+     *                                       is incompatible with this set
+     *                                       (<a href="Collection.html#optional-restrictions">optional</a>)
+     * @throws NullPointerException          if the specified element is null and this
+     *                                       set does not permit null elements
+     *                                       (<a href="Collection.html#optional-restrictions">optional</a>)
+     * @throws UnsupportedOperationException if the {@code remove} operation
+     *                                       is not supported by this set
+     */
+    @Override
+    public boolean remove(Object o) {
+        boolean present = contains(o);
+        this.cache.asMap().remove(o);
+        return present;
+    }
+
+    /**
+     * Returns {@code true} if this set contains all of the elements of the
+     * specified collection.  If the specified collection is also a set, this
+     * method returns {@code true} if it is a <i>subset</i> of this set.
+     *
+     * @param c collection to be checked for containment in this set
+     * @return {@code true} if this set contains all of the elements of the
+     * specified collection
+     * @throws ClassCastException   if the types of one or more elements
+     *                              in the specified collection are incompatible with this
+     *                              set
+     *                              (<a href="Collection.html#optional-restrictions">optional</a>)
+     * @throws NullPointerException if the specified collection contains one
+     *                              or more null elements and this set does not permit null
+     *                              elements
+     *                              (<a href="Collection.html#optional-restrictions">optional</a>),
+     *                              or if the specified collection is null
+     * @see #contains(Object)
+     */
+    @Override
+    public boolean containsAll(@NotNull Collection<?> c) {
+        for (Object o : c) {
+            if (!contains(o)) {
+                return false;
+            }
+        }
+        return true;
+    }
+
+    /**
+     * Adds all of the elements in the specified collection to this set if
+     * they're not already present (optional operation).  If the specified
+     * collection is also a set, the {@code addAll} operation effectively
+     * modifies this set so that its value is the <i>union</i> of the two
+     * sets.  The behavior of this operation is undefined if the specified
+     * collection is modified while the operation is in progress.
+     *
+     * @param c collection containing elements to be added to this set
+     * @return {@code true} if this set changed as a result of the call
+     * @throws UnsupportedOperationException if the {@code addAll} operation
+     *                                       is not supported by this set
+     * @throws ClassCastException            if the class of an element of the
+     *                                       specified collection prevents it from being added to this set
+     * @throws NullPointerException          if the specified collection contains one
+     *                                       or more null elements and this set does not permit null
+     *                                       elements, or if the specified collection is null
+     * @throws IllegalArgumentException      if some property of an element of the
+     *                                       specified collection prevents it from being added to this set
+     * @see #add(Object)
+     */
+    @Override
+    public boolean addAll(@NotNull Collection<? extends E> c) {
+        boolean changed = false;
+        for (E o : c) {
+            if (add(o)) {
+                changed = true;
+            }
+        }
+        return changed;
+    }
+
+    /**
+     * Retains only the elements in this set that are contained in the
+     * specified collection (optional operation).  In other words, removes
+     * from this set all of its elements that are not contained in the
+     * specified collection.  If the specified collection is also a set, this
+     * operation effectively modifies this set so that its value is the
+     * <i>intersection</i> of the two sets.
+     *
+     * @param c collection containing elements to be retained in this set
+     * @return {@code true} if this set changed as a result of the call
+     * @throws UnsupportedOperationException if the {@code retainAll} operation
+     *                                       is not supported by this set
+     * @throws ClassCastException            if the class of an element of this set
+     *                                       is incompatible with the specified collection
+     *                                       (<a href="Collection.html#optional-restrictions">optional</a>)
+     * @throws NullPointerException          if this set contains a null element and the
+     *                                       specified collection does not permit null elements
+     *                                       (<a href="Collection.html#optional-restrictions">optional</a>),
+     *                                       or if the specified collection is null
+     * @see #remove(Object)
+     */
+    @Override
+    public boolean retainAll(@NotNull Collection<?> c) {
+        boolean changed = false;
+        for (E e : this.cache.asMap().keySet()) {
+            if (!c.contains(e) && remove(e)) {
+                changed = true;
+            }
+        }
+        return changed;
+    }
+
+    /**
+     * Removes from this set all of its elements that are contained in the
+     * specified collection (optional operation).  If the specified
+     * collection is also a set, this operation effectively modifies this
+     * set so that its value is the <i>asymmetric set difference</i> of
+     * the two sets.
+     *
+     * @param c collection containing elements to be removed from this set
+     * @return {@code true} if this set changed as a result of the call
+     * @throws UnsupportedOperationException if the {@code removeAll} operation
+     *                                       is not supported by this set
+     * @throws ClassCastException            if the class of an element of this set
+     *                                       is incompatible with the specified collection
+     *                                       (<a href="Collection.html#optional-restrictions">optional</a>)
+     * @throws NullPointerException          if this set contains a null element and the
+     *                                       specified collection does not permit null elements
+     *                                       (<a href="Collection.html#optional-restrictions">optional</a>),
+     *                                       or if the specified collection is null
+     * @see #remove(Object)
+     * @see #contains(Object)
+     */
+    @Override
+    public boolean removeAll(@NotNull Collection<?> c) {
+        boolean changed = false;
+        for (E e : this.cache.asMap().keySet()) {
+            if (remove(e)) {
+                changed = true;
+            }
+        }
+        return changed;
+    }
+
+    /**
+     * Removes all of the elements from this set (optional operation).
+     * The set will be empty after this call returns.
+     *
+     * @throws UnsupportedOperationException if the {@code clear} method
+     *                                       is not supported by this set
+     */
+    @Override
+    public void clear() {
+        this.cache.asMap().clear();
+    }
+}