Improve EntityScheduler optimization patch even... further... beyond!

README.md

@@ -37,11 +37,9 @@ SparklyPaper's config file is `sparklypaper.yml`, the file is, by default, place
   * ~~We can avoid this call by checking if the `cachedMapId` is not null, if it is, then we get the item in the item frame, if not, then we ignore the `getItem()` call.~~
   * Replaced by [Warriorrrr's "Rewrite framed map tracker ticking" patch (Paper #9605)](https://github.com/PaperMC/Paper/pull/9605)
 * 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.
+  * 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, due to the thread checks, and because it needs to iterate all server entities to tick them.
-  * To avoid those hefty calls, we check if we *really* need to execute the `executeTick`, by doing two changes:
+  * To avoid those hefty calls, instead of iterating all entities in all worlds, we use a set to track which entities have scheduled tasks that we need to tick. When a task is scheduled, we add the entity to the set, when all entity tasks are executed, the entity is removed from the set.
-    * 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.
+  * Most entities won't have any scheduled tasks, so this is a nice performance bonus, even if you have plugins that do use the entity scheduler because, for 99,99% of use cases, you aren't going to create tasks for all entities in your server. With this change, the `executeTick` loop in `tickChildren` CPU % usage drops from 8.40% to 0.00% (!!!) in a server with ~15k entities! Sweet!
-    * 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
   * Changes Minecraft's farm checks for crops, stem blocks, and farm lands to be simpler and less resource intensive

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

@@ -4,53 +4,119 @@ Date: Sun, 19 Nov 2023 12:35:16 -0300
 Subject: [PATCH] Skip EntityScheduler's executeTick checks if there isn't any
  tasks to be run
 
-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.
+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, due to the thread checks, and because it needs to iterate all entities in all worlds.
 
-To avoid the hefty ArrayDeque's size() call, we check if we *really* need to execute the executeTick, by checking if currentlyExecuting is not empty or if oneTimeDelayed is not empty.
+To avoid the hefty ArrayDeque's size() call, we check if we *really* need to execute the executeTick, by adding all entities with scheduled tasks to a global set.
 
 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..4af6652edc0651ab8ad28a738055d407451e49bf 100644
+index 62484ebf4550b05182f693a3180bbac5d5fd906d..4c7aa86bd115c0a6dd6fc8fe20be5c7c48215ca2 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 {
+@@ -36,6 +36,7 @@ public final class EntityScheduler {
-     private final Long2ObjectOpenHashMap<List<ScheduledTask>> oneTimeDelayed = new Long2ObjectOpenHashMap<>();
+      * The Entity. Note that it is the CraftEntity, since only that class properly tracks world transfers.
+      */
+     public final CraftEntity entity;
++    public final net.minecraft.server.MinecraftServer server; // SparklyPaper - skip EntityScheduler's executeTick checks if there isn't any tasks to be run
+ 
+     private static final record ScheduledTask(Consumer<? extends Entity> run, Consumer<? extends Entity> retired) {}
+ 
+@@ -46,7 +47,8 @@ public final class EntityScheduler {
  
      private final ArrayDeque<ScheduledTask> currentlyExecuting = new ArrayDeque<>();
-+    private boolean hasScheduledTasks = false; // SparklyPaper - skip EntityScheduler's executeTick checks if there isn't any tasks to be run
  
-     public EntityScheduler(final CraftEntity entity) {
+-    public EntityScheduler(final CraftEntity entity) {
++    public EntityScheduler(final net.minecraft.server.MinecraftServer server, final CraftEntity entity) { // SparklyPaper - skip EntityScheduler's executeTick checks if there isn't any tasks to be run
++        this.server = Validate.notNull(server);
          this.entity = Validate.notNull(entity);
-@@ -124,6 +125,7 @@ public final class EntityScheduler {
+     }
+ 
+@@ -66,6 +68,7 @@ public final class EntityScheduler {
+                 throw new IllegalStateException("Already retired");
+             }
+             this.tickCount = RETIRED_TICK_COUNT;
++            this.server.entitiesWithScheduledTasks.remove(entity); // SparklyPaper - skip EntityScheduler's executeTick checks if there isn't any tasks to be run
+         }
+ 
+         final Entity thisEntity = this.entity.getHandleRaw();
+@@ -124,6 +127,7 @@ public final class EntityScheduler {
              if (this.tickCount == RETIRED_TICK_COUNT) {
                  return false;
              }
-+            hasScheduledTasks = true; // SparklyPaper - skip EntityScheduler's executeTick checks if there isn't any tasks to be run
++            this.server.entitiesWithScheduledTasks.add(entity); // 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,6 +140,23 @@ public final class EntityScheduler {
+@@ -143,6 +147,13 @@ public final class EntityScheduler {
-      * @throws IllegalStateException If the scheduler is retired.
-      */
-     public void executeTick() {
-+        // SparklyPaper start - skip EntityScheduler's executeTick checks if there isn't any tasks to be run
-+        // This wouldn't work on a multithreaded environment like Folia!!! But because the executeTick is always executed on the
-+        // 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 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;
-+        }
-+        // SparklyPaper end
-         final Entity thisEntity = this.entity.getHandleRaw();
- 
          TickThread.ensureTickThread(thisEntity, "May not tick entity scheduler asynchronously");
+         final List<ScheduledTask> toRun;
+         synchronized (this.stateLock) {
++            // SparklyPaper start - skip EntityScheduler's executeTick checks if there isn't any tasks to be run
++            // 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.server.entitiesWithScheduledTasks.remove(entity); // We don't! Bye bye!!
++                return;
++            }
++            // SparklyPaper end
+             if (this.tickCount == RETIRED_TICK_COUNT) {
+                 throw new IllegalStateException("Ticking retired scheduler");
+             }
+diff --git a/src/main/java/net/minecraft/server/MinecraftServer.java b/src/main/java/net/minecraft/server/MinecraftServer.java
+index 25367df06a8a6e8b0b3a56652a5fb1c70a15632d..e01297d1269e55f4a4f6c43273d194972529645c 100644
+--- a/src/main/java/net/minecraft/server/MinecraftServer.java
++++ b/src/main/java/net/minecraft/server/MinecraftServer.java
+@@ -308,6 +308,7 @@ public abstract class MinecraftServer extends ReentrantBlockableEventLoop<TickTa
+     // Paper start - lag compensation
+     public static final long SERVER_INIT = System.nanoTime();
+     // Paper end - lag compensation
++    public final Set<org.bukkit.craftbukkit.entity.CraftEntity> entitiesWithScheduledTasks = java.util.concurrent.ConcurrentHashMap.newKeySet(); // SparklyPaper - skip EntityScheduler's executeTick checks if there isn't any tasks to be run (concurrent because plugins may schedule tasks async)
+ 
+     public static <S extends MinecraftServer> S spin(Function<Thread, S> serverFactory) {
+         AtomicReference<S> atomicreference = new AtomicReference();
+@@ -1471,6 +1472,15 @@ public abstract class MinecraftServer extends ReentrantBlockableEventLoop<TickTa
+         MinecraftTimings.bukkitSchedulerTimer.stopTiming(); // Spigot // Paper
+         // Paper start - Folia scheduler API
+         ((io.papermc.paper.threadedregions.scheduler.FoliaGlobalRegionScheduler) Bukkit.getGlobalRegionScheduler()).tick();
++        // SparklyPaper - skip EntityScheduler's executeTick checks if there isn't any tasks to be run
++        for (final org.bukkit.craftbukkit.entity.CraftEntity craftEntity : entitiesWithScheduledTasks) {
++            Entity entity = craftEntity.getHandleRaw();
++            if (entity.isRemoved()) {
++                continue;
++            }
++            craftEntity.taskScheduler.executeTick();
++        }
++        /*
+         getAllLevels().forEach(level -> {
+             for (final Entity entity : level.getEntityLookup().getAllCopy()) {
+                 if (entity.isRemoved()) {
+@@ -1482,6 +1492,8 @@ public abstract class MinecraftServer extends ReentrantBlockableEventLoop<TickTa
+                 }
+             }
+         });
++        */
++        // SparklyPaper end
+         // Paper end - Folia scheduler API
+         io.papermc.paper.adventure.providers.ClickCallbackProviderImpl.CALLBACK_MANAGER.handleQueue(this.tickCount); // Paper
+         this.profiler.push("commandFunctions");
+diff --git a/src/main/java/org/bukkit/craftbukkit/entity/CraftEntity.java b/src/main/java/org/bukkit/craftbukkit/entity/CraftEntity.java
+index 9f843b89dc20b91bf7243facee8486d525e4a1b3..d118989be2171c51af6f09e758538b2af476872f 100644
+--- a/src/main/java/org/bukkit/craftbukkit/entity/CraftEntity.java
++++ b/src/main/java/org/bukkit/craftbukkit/entity/CraftEntity.java
+@@ -208,7 +208,7 @@ public abstract class CraftEntity implements org.bukkit.entity.Entity {
+     private final CraftPersistentDataContainer persistentDataContainer = new CraftPersistentDataContainer(CraftEntity.DATA_TYPE_REGISTRY);
+     protected net.kyori.adventure.pointer.Pointers adventure$pointers; // Paper - implement pointers
+     // Paper start - Folia shedulers
+-    public final io.papermc.paper.threadedregions.EntityScheduler taskScheduler = new io.papermc.paper.threadedregions.EntityScheduler(this);
++    public final io.papermc.paper.threadedregions.EntityScheduler taskScheduler; // = new io.papermc.paper.threadedregions.EntityScheduler(this); // SparklyPaper - skip EntityScheduler's executeTick checks if there isn't any tasks to be run
+     private final io.papermc.paper.threadedregions.scheduler.FoliaEntityScheduler apiScheduler = new io.papermc.paper.threadedregions.scheduler.FoliaEntityScheduler(this);
+ 
+     @Override
+@@ -221,6 +221,7 @@ public abstract class CraftEntity implements org.bukkit.entity.Entity {
+         this.server = server;
+         this.entity = entity;
+         this.entityType = CraftEntityType.minecraftToBukkit(entity.getType());
++        this.taskScheduler = new io.papermc.paper.threadedregions.EntityScheduler(this.entity.getServer(), this); // SparklyPaper - skip EntityScheduler's executeTick checks if there isn't any tasks to be run
+     }
+ 
+     public static CraftEntity getEntity(CraftServer server, Entity entity) {

patches/server/0013-Spooky-month-optimizations.patch

@@ -10,13 +10,13 @@ Caches when Bat's spooky season starts and ends, and when Skeleton and Zombies h
 Avoids unnecessary date checks, even tho that this shouldn't really improve performance that much... unless you have a lot of bats/zombies/skeletons spawning.
 
 diff --git a/src/main/java/net/minecraft/server/MinecraftServer.java b/src/main/java/net/minecraft/server/MinecraftServer.java
-index 25367df06a8a6e8b0b3a56652a5fb1c70a15632d..a154182d4bab167086a644dc3907e0893af5adb6 100644
+index e01297d1269e55f4a4f6c43273d194972529645c..250f13e8cef10f6b416f35c668f31512f5679d71 100644
 --- a/src/main/java/net/minecraft/server/MinecraftServer.java
 +++ b/src/main/java/net/minecraft/server/MinecraftServer.java
-@@ -308,6 +308,7 @@ public abstract class MinecraftServer extends ReentrantBlockableEventLoop<TickTa
+@@ -309,6 +309,7 @@ public abstract class MinecraftServer extends ReentrantBlockableEventLoop<TickTa
-     // Paper start - lag compensation
      public static final long SERVER_INIT = System.nanoTime();
      // Paper end - lag compensation
+     public final Set<org.bukkit.craftbukkit.entity.CraftEntity> entitiesWithScheduledTasks = java.util.concurrent.ConcurrentHashMap.newKeySet(); // SparklyPaper - skip EntityScheduler's executeTick checks if there isn't any tasks to be run (concurrent because plugins may schedule tasks async)
 +    public net.sparklypower.sparklypaper.HalloweenManager halloweenManager = new net.sparklypower.sparklypaper.HalloweenManager(); // SparklyPaper - Spooky month optimizations
  
      public static <S extends MinecraftServer> S spin(Function<Thread, S> serverFactory) {

patches/server/0014-Track-how-much-MSPT-each-world-used.patch

@@ -56,10 +56,10 @@ index 8b5293b0c696ef21d0101493ffa41b60bf0bc86b..601198a33adb29316b0617d5390d1620
      }
  
 diff --git a/src/main/java/net/minecraft/server/MinecraftServer.java b/src/main/java/net/minecraft/server/MinecraftServer.java
-index a154182d4bab167086a644dc3907e0893af5adb6..4bdd8e58c7ad86b8ed839a5293e0b7442c05ad05 100644
+index 250f13e8cef10f6b416f35c668f31512f5679d71..52b86e899f0c897fb4deca36936073ade3db8265 100644
 --- a/src/main/java/net/minecraft/server/MinecraftServer.java
 +++ b/src/main/java/net/minecraft/server/MinecraftServer.java
-@@ -1546,7 +1546,16 @@ public abstract class MinecraftServer extends ReentrantBlockableEventLoop<TickTa
+@@ -1558,7 +1558,16 @@ public abstract class MinecraftServer extends ReentrantBlockableEventLoop<TickTa
  
              try {
                  worldserver.timings.doTick.startTiming(); // Spigot

patches/server/0015-Parallel-world-ticking.patch

@@ -557,7 +557,7 @@ index 6f2adf2334e35e8a617a4ced0c1af2abf32bbd8d..a5ea9df0a021ed820c0c1ccb612caebd
              }
  
 diff --git a/src/main/java/net/minecraft/server/MinecraftServer.java b/src/main/java/net/minecraft/server/MinecraftServer.java
-index 4bdd8e58c7ad86b8ed839a5293e0b7442c05ad05..1755fa872726ddb35ba9eb3d9da51a2b0260619b 100644
+index 52b86e899f0c897fb4deca36936073ade3db8265..dfc975c45abbc016e2212eb37883a798cbb4a515 100644
 --- a/src/main/java/net/minecraft/server/MinecraftServer.java
 +++ b/src/main/java/net/minecraft/server/MinecraftServer.java
 @@ -160,6 +160,7 @@ import net.minecraft.world.level.storage.PrimaryLevelData;
@@ -568,9 +568,9 @@ index 4bdd8e58c7ad86b8ed839a5293e0b7442c05ad05..1755fa872726ddb35ba9eb3d9da51a2b
  import org.slf4j.Logger;
  
  // CraftBukkit start
-@@ -309,6 +310,9 @@ public abstract class MinecraftServer extends ReentrantBlockableEventLoop<TickTa
+@@ -310,6 +311,9 @@ public abstract class MinecraftServer extends ReentrantBlockableEventLoop<TickTa
-     public static final long SERVER_INIT = System.nanoTime();
      // Paper end - lag compensation
+     public final Set<org.bukkit.craftbukkit.entity.CraftEntity> entitiesWithScheduledTasks = java.util.concurrent.ConcurrentHashMap.newKeySet(); // SparklyPaper - skip EntityScheduler's executeTick checks if there isn't any tasks to be run (concurrent because plugins may schedule tasks async)
      public net.sparklypower.sparklypaper.HalloweenManager halloweenManager = new net.sparklypower.sparklypaper.HalloweenManager(); // SparklyPaper - Spooky month optimizations
 +    // SparklyPaper - parallel world ticking
 +    public java.util.concurrent.Semaphore serverLevelTickingSemaphore = null;
@@ -578,7 +578,7 @@ index 4bdd8e58c7ad86b8ed839a5293e0b7442c05ad05..1755fa872726ddb35ba9eb3d9da51a2b
  
      public static <S extends MinecraftServer> S spin(Function<Thread, S> serverFactory) {
          AtomicReference<S> atomicreference = new AtomicReference();
-@@ -1524,63 +1528,124 @@ public abstract class MinecraftServer extends ReentrantBlockableEventLoop<TickTa
+@@ -1536,63 +1540,124 @@ public abstract class MinecraftServer extends ReentrantBlockableEventLoop<TickTa
  
          this.isIteratingOverLevels = true; // Paper
          Iterator iterator = this.getAllLevels().iterator(); // Paper - move down