Improve EntityScheduler optimization patch

README.md

@@ -39,8 +39,8 @@ SparklyPaper's config file is `sparklypaper.yml`, the file is, by default, place
 * Optimize `EntityScheduler`'s `executeTick`
   * On each tick, Paper runs `EntityScheduler`'s `executeTick` of each entity. This is a bit expensive, due to `ArrayDeque`'s `size()` call because it ain't a simple "get the current queue size" function, and due to the thread checks.
   * To avoid those hefty calls, we check if we *really* need to execute the `executeTick`, by doing two changes:
-    * First, we check if the scheduler had a task scheduled to it at least once, if not, we bail out early and quickly. In this case, we don't even increase the scheduler's `tickCount`, nor do we check if the scheduler has been retired. This is not an issue, since `tickCount` is relative and not bound to the server's current tick, so it doesn't matter if we don't increase it.
-    * Then, we check if the scheduler has any pending tasks by checking if `currentlyExecuting` is not empty or if `oneTimeDelayed` is not empty. This avoids the thread checks and unnecessary `size()` calls.
+    * First, we check if the scheduler actually has scheduled tasks to it by looking up `hasScheduledTasks`, if not, we bail out early and quickly. In this case, we don't even increase the scheduler's `tickCount`, nor do we check if the scheduler has been retired. This is not an issue, since `tickCount` is relative and not bound to the server's current tick, so it doesn't matter if we don't increase it.
+    * Then, we check if the scheduler has any pending tasks by checking if `currentlyExecuting` is not empty or if `oneTimeDelayed` is not empty. If it isn't, then we process the tasks, if it is, then we set `hasScheduledTasks` to false, avoiding future `size()` checks. This avoids the thread checks and unnecessary `size()` calls.
   * Most entities won't have any scheduled tasks, so this is a nice performance bonus. These optimizations, however, wouldn't work in a Folia environment, but because in SparklyPaper `executeTick` is always executed on the main thread, it ain't an issue for us (yay). With this change, the `executeTick` loop in `tickChildren` CPU % usage drops from 2.48% to 0.61% in a server with ~12k entities!
   * Of course, this doesn't mean that `ArrayDeque#size()` is slow! It is mostly that because the `executeTick` function is called each tick for each entity, it would be better for us to avoid as many useless calls as possible.
 * Blazingly Simple Farm Checks

patches/server/0010-Skip-EntityScheduler-s-executeTick-checks-if-there-i.patch

@@ -11,14 +11,14 @@ To avoid the hefty ArrayDeque's size() call, we check if we *really* need to exe
 Most entities won't have any scheduled tasks, so this is a nice performance bonus. These optimizations, however, wouldn't work in a Folia environment, but because in SparklyPaper executeTick is always executed on the main thread, it ain't an issue for us (yay).
 
 diff --git a/src/main/java/io/papermc/paper/threadedregions/EntityScheduler.java b/src/main/java/io/papermc/paper/threadedregions/EntityScheduler.java
-index 62484ebf4550b05182f693a3180bbac5d5fd906d..9e15a2a9e47a5b50c3c9e2afce2eb0f48ed0fdaf 100644
+index 62484ebf4550b05182f693a3180bbac5d5fd906d..4af6652edc0651ab8ad28a738055d407451e49bf 100644
 --- a/src/main/java/io/papermc/paper/threadedregions/EntityScheduler.java
 +++ b/src/main/java/io/papermc/paper/threadedregions/EntityScheduler.java
 @@ -45,6 +45,7 @@ public final class EntityScheduler {
      private final Long2ObjectOpenHashMap<List<ScheduledTask>> oneTimeDelayed = new Long2ObjectOpenHashMap<>();
  
      private final ArrayDeque<ScheduledTask> currentlyExecuting = new ArrayDeque<>();
-+    private boolean hasScheduledAtLeastOneTask = false; // SparklyPaper - skip EntityScheduler's executeTick checks if there isn't any tasks to be run
++    private boolean hasScheduledTasks = false; // SparklyPaper - skip EntityScheduler's executeTick checks if there isn't any tasks to be run
  
      public EntityScheduler(final CraftEntity entity) {
          this.entity = Validate.notNull(entity);
@@ -26,11 +26,11 @@ index 62484ebf4550b05182f693a3180bbac5d5fd906d..9e15a2a9e47a5b50c3c9e2afce2eb0f4
              if (this.tickCount == RETIRED_TICK_COUNT) {
                  return false;
              }
-+            hasScheduledAtLeastOneTask = true; // SparklyPaper - skip EntityScheduler's executeTick checks if there isn't any tasks to be run
++            hasScheduledTasks = true; // SparklyPaper - skip EntityScheduler's executeTick checks if there isn't any tasks to be run
              this.oneTimeDelayed.computeIfAbsent(this.tickCount + Math.max(1L, delay), (final long keyInMap) -> {
                  return new ArrayList<>();
              }).add(task);
-@@ -138,15 +140,34 @@ public final class EntityScheduler {
+@@ -138,6 +140,23 @@ public final class EntityScheduler {
       * @throws IllegalStateException If the scheduler is retired.
       */
      public void executeTick() {
@@ -39,33 +39,18 @@ index 62484ebf4550b05182f693a3180bbac5d5fd906d..9e15a2a9e47a5b50c3c9e2afce2eb0f4
 +        // main thread, we don't need to care about concurrency
 +        // First, we use a simple boolean check here
 +        // Yeah, this sounds dumb but, after benchmarking, it seems like a simple boolean check like this is actually more performant than calling both is empty checks
-+        // So first we check if the EntityScheduler has been used at least ONCE
-+        // If it wasn't, then we skip all checks entirely
-+        // We don't even need to update the tick count because the tick count is actually relative to this scheduler, not bound to the global current tick, so it isn't a big deal if we don't update the tick count
-+        if (!this.hasScheduledAtLeastOneTask)
++        // So first we check if the EntityScheduler has any scheduled tasks
++        // If it doesn't, then we skip all checks entirely
++        // We don't even need to update the tick count because the tick count is actually relative to this scheduler, not bound to the global current tick, so it isn't a big deal if we don't update the tick count, as long
++        // as there isn't any pending tasks
++        if (!this.hasScheduledTasks)
++            return;
++        // We do have scheduled tasks then... but do we *really* have scheduled tasks tho?
++        if (this.currentlyExecuting.isEmpty() && this.oneTimeDelayed.isEmpty()) { // Check if we have any pending tasks and, if not, skip!
++            this.hasScheduledTasks = false; // We don't! Bye bye!!
 +            return;
-+        if (this.tickCount == RETIRED_TICK_COUNT) {
-+            throw new IllegalStateException("Ticking retired scheduler");
 +        }
-+        ++this.tickCount;
-+        if (this.currentlyExecuting.isEmpty() && this.oneTimeDelayed.isEmpty()) // Check if we have any pending tasks and, if not, skip!
-+            return;
 +        // SparklyPaper end
          final Entity thisEntity = this.entity.getHandleRaw();
  
          TickThread.ensureTickThread(thisEntity, "May not tick entity scheduler asynchronously");
-         final List<ScheduledTask> toRun;
-         synchronized (this.stateLock) {
--            if (this.tickCount == RETIRED_TICK_COUNT) {
--                throw new IllegalStateException("Ticking retired scheduler");
--            }
--            ++this.tickCount;
-+            // SparklyPaper start - skip EntityScheduler's executeTick checks if there isn't any tasks to be run
-+            // if (this.tickCount == RETIRED_TICK_COUNT) {
-+            //     throw new IllegalStateException("Ticking retired scheduler");
-+            // }
-+            // ++this.tickCount;
-+            // SparklyPaper end
-             if (this.oneTimeDelayed.isEmpty()) {
-                 toRun = null;
-             } else {