diff --git a/README.md b/README.md index 996a4e5..bc374b8 100644 --- a/README.md +++ b/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. - * 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 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! + * 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, 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. + * 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! * 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 diff --git a/patches/server/0010-Skip-EntityScheduler-s-executeTick-checks-if-there-i.patch b/patches/server/0010-Skip-EntityScheduler-s-executeTick-checks-if-there-i.patch index a4fc707..66514c9 100644 --- a/patches/server/0010-Skip-EntityScheduler-s-executeTick-checks-if-there-i.patch +++ b/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 { - private final Long2ObjectOpenHashMap> oneTimeDelayed = new Long2ObjectOpenHashMap<>(); +@@ -36,6 +36,7 @@ public final class EntityScheduler { + * 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 run, Consumer retired) {} + +@@ -46,7 +47,8 @@ public final class EntityScheduler { private final ArrayDeque 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 { - * @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(); - +@@ -143,6 +147,13 @@ public final class EntityScheduler { TickThread.ensureTickThread(thisEntity, "May not tick entity scheduler asynchronously"); + final List 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 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 spin(Function serverFactory) { + AtomicReference atomicreference = new AtomicReference(); +@@ -1471,6 +1472,15 @@ public abstract class MinecraftServer extends ReentrantBlockableEventLoop { + for (final Entity entity : level.getEntityLookup().getAllCopy()) { + if (entity.isRemoved()) { +@@ -1482,6 +1492,8 @@ public abstract class MinecraftServer extends ReentrantBlockableEventLoop 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 spin(Function serverFactory) { diff --git a/patches/server/0014-Track-how-much-MSPT-each-world-used.patch b/patches/server/0014-Track-how-much-MSPT-each-world-used.patch index f1fda73..310d3a3 100644 --- a/patches/server/0014-Track-how-much-MSPT-each-world-used.patch +++ b/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 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 spin(Function serverFactory) { AtomicReference atomicreference = new AtomicReference(); -@@ -1524,63 +1528,124 @@ public abstract class MinecraftServer extends ReentrantBlockableEventLoop