From f65939c6bdb2e84ec048a68ee3ed419a30656623 Mon Sep 17 00:00:00 2001 From: Bjarne Koll Date: Thu, 19 Sep 2024 16:36:07 +0200 Subject: [PATCH] Remove wall-time / unused skip tick protection (#11412) Spigot still maintains some partial implementation of "tick skipping", a practice in which the MinecraftServer.currentTick field is updated not by an increment of one per actual tick, but instead set to System.currentTimeMillis() / 50. This behaviour means that the tracked tick may "skip" a tick value in case a previous tick took more than the expected 50ms. To compensate for this in important paths, spigot/craftbukkit implements "wall-time". Instead of incrementing/decrementing ticks on block entities/entities by one for each call to their tick() method, they instead increment/decrement important values, like an ItemEntity's age or pickupDelay, by the difference of `currentTick - lastTick`, where `lastTick` is the value of `currentTick` during the last tick() call. These "fixes" however do not play nicely with minecraft's simulation distance as entities/block entities implementing the above behaviour would "catch up" their values when moving from a non-ticking chunk to a ticking one as their `lastTick` value remains stuck on the last tick in a ticking chunk and hence lead to a large "catch up" once ticked again. Paper completely removes the "tick skipping" behaviour (See patch "Further-improve-server-tick-loop"), making the above precautions completely unnecessary, which also rids paper of the previous described incompatibility with non-ticking chunks. --- .../Further-improve-server-tick-loop.patch | 2 +- .../server/Improved-Watchdog-Support.patch | 2 +- patches/server/MC-Utils.patch | 16 -- ...all-time-unused-skip-tick-protection.patch | 163 ++++++++++++++++++ ...n-on-world-create-while-being-ticked.patch | 2 +- ...-for-pickupDelay-breaks-picking-up-i.patch | 27 --- 6 files changed, 166 insertions(+), 46 deletions(-) create mode 100644 patches/server/Remove-wall-time-unused-skip-tick-protection.patch delete mode 100644 patches/server/don-t-go-below-0-for-pickupDelay-breaks-picking-up-i.patch diff --git a/patches/server/Further-improve-server-tick-loop.patch b/patches/server/Further-improve-server-tick-loop.patch index 2f9095f42c..39b8dea237 100644 --- a/patches/server/Further-improve-server-tick-loop.patch +++ b/patches/server/Further-improve-server-tick-loop.patch @@ -105,9 +105,9 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 long i; @@ -0,0 +0,0 @@ public abstract class MinecraftServer extends ReentrantBlockableEventLoop S spin(Function serverFactory) { - AtomicReference atomicreference = new AtomicReference(); @@ -0,0 +0,0 @@ public abstract class MinecraftServer extends ReentrantBlockableEventLoop +Date: Mon, 16 Sep 2024 23:07:29 +0200 +Subject: [PATCH] Remove wall-time / unused skip tick protection + +Spigot still maintains some partial implementation of "tick skipping", a +practice in which the MinecraftServer.currentTick field is updated not +by an increment of one per actual tick, but instead set to +System.currentTimeMillis() / 50. This behaviour means that the tracked +tick may "skip" a tick value in case a previous tick took more than the +expected 50ms. + +To compensate for this in important paths, spigot/craftbukkit +implements "wall-time". Instead of incrementing/decrementing ticks on +block entities/entities by one for each call to their tick() method, +they instead increment/decrement important values, like +an ItemEntity's age or pickupDelay, by the difference of +`currentTick - lastTick`, where `lastTick` is the value of +`currentTick` during the last tick() call. + +These "fixes" however do not play nicely with minecraft's simulation +distance as entities/block entities implementing the above behaviour +would "catch up" their values when moving from a non-ticking chunk to a +ticking one as their `lastTick` value remains stuck on the last tick in +a ticking chunk and hence lead to a large "catch up" once ticked again. + +Paper completely removes the "tick skipping" behaviour (See patch +"Further-improve-server-tick-loop"), making the above precautions +completely unnecessary, which also rids paper of the previous described +incompatibility with non-ticking chunks. + +diff --git a/src/main/java/net/minecraft/world/entity/item/ItemEntity.java b/src/main/java/net/minecraft/world/entity/item/ItemEntity.java +index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 +--- a/src/main/java/net/minecraft/world/entity/item/ItemEntity.java ++++ b/src/main/java/net/minecraft/world/entity/item/ItemEntity.java +@@ -0,0 +0,0 @@ public class ItemEntity extends Entity implements TraceableEntity { + @Nullable + public UUID target; + public final float bobOffs; +- private int lastTick = MinecraftServer.currentTick - 1; // CraftBukkit ++ // private int lastTick = MinecraftServer.currentTick - 1; // CraftBukkit // Paper - remove anti tick skipping measures / wall time + public boolean canMobPickup = true; // Paper - Item#canEntityPickup + private int despawnRate = -1; // Paper - Alternative item-despawn-rate + public net.kyori.adventure.util.TriState frictionState = net.kyori.adventure.util.TriState.NOT_SET; // Paper - Friction API +@@ -0,0 +0,0 @@ public class ItemEntity extends Entity implements TraceableEntity { + this.discard(EntityRemoveEvent.Cause.DESPAWN); // CraftBukkit - add Bukkit remove cause + } else { + super.tick(); +- // CraftBukkit start - Use wall time for pickup and despawn timers +- int elapsedTicks = MinecraftServer.currentTick - this.lastTick; +- if (this.pickupDelay != 32767) this.pickupDelay -= elapsedTicks; +- if (this.age != -32768) this.age += elapsedTicks; +- this.lastTick = MinecraftServer.currentTick; +- // CraftBukkit end ++ // Paper start - remove anti tick skipping measures / wall time - revert to vanilla ++ if (this.pickupDelay > 0 && this.pickupDelay != 32767) { ++ --this.pickupDelay; ++ } ++ // Paper end - remove anti tick skipping measures / wall time - revert to vanilla + + this.xo = this.getX(); + this.yo = this.getY(); +@@ -0,0 +0,0 @@ public class ItemEntity extends Entity implements TraceableEntity { + this.mergeWithNeighbours(); + } + +- /* CraftBukkit start - moved up ++ // Paper - remove anti tick skipping measures / wall time - revert to vanilla /* CraftBukkit start - moved up + if (this.age != -32768) { + ++this.age; + } +@@ -0,0 +0,0 @@ public class ItemEntity extends Entity implements TraceableEntity { + // Spigot start - copied from above + @Override + public void inactiveTick() { +- // CraftBukkit start - Use wall time for pickup and despawn timers +- int elapsedTicks = MinecraftServer.currentTick - this.lastTick; +- if (this.pickupDelay != 32767) this.pickupDelay -= elapsedTicks; +- if (this.age != -32768) this.age += elapsedTicks; +- this.lastTick = MinecraftServer.currentTick; +- // CraftBukkit end ++ // Paper start - remove anti tick skipping measures / wall time - copied from above ++ if (this.pickupDelay > 0 && this.pickupDelay != 32767) { ++ --this.pickupDelay; ++ } ++ if (this.age != -32768) { ++ ++this.age; ++ } ++ // Paper end - remove anti tick skipping measures / wall time - copied from above + + if (!this.level().isClientSide && this.age >= this.despawnRate) { // Spigot // Paper - Alternative item-despawn-rate + // CraftBukkit start - fire ItemDespawnEvent +diff --git a/src/main/java/net/minecraft/world/entity/monster/Zombie.java b/src/main/java/net/minecraft/world/entity/monster/Zombie.java +index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 +--- a/src/main/java/net/minecraft/world/entity/monster/Zombie.java ++++ b/src/main/java/net/minecraft/world/entity/monster/Zombie.java +@@ -0,0 +0,0 @@ public class Zombie extends Monster { + private boolean canBreakDoors; + private int inWaterTime; + public int conversionTime; +- private int lastTick = MinecraftServer.currentTick; // CraftBukkit - add field ++ // private int lastTick = MinecraftServer.currentTick; // CraftBukkit - add field // Paper - remove anti tick skipping measures / wall time + private boolean shouldBurnInDay = true; // Paper - Add more Zombie API + + public Zombie(EntityType type, Level world) { +@@ -0,0 +0,0 @@ public class Zombie extends Monster { + public void tick() { + if (!this.level().isClientSide && this.isAlive() && !this.isNoAi()) { + if (this.isUnderWaterConverting()) { +- // CraftBukkit start - Use wall time instead of ticks for conversion +- int elapsedTicks = MinecraftServer.currentTick - this.lastTick; +- this.conversionTime -= elapsedTicks; +- // CraftBukkit end ++ --this.conversionTime; // Paper - remove anti tick skipping measures / wall time + if (this.conversionTime < 0) { + this.doUnderWaterConversion(); + } +@@ -0,0 +0,0 @@ public class Zombie extends Monster { + } + + super.tick(); +- this.lastTick = MinecraftServer.currentTick; // CraftBukkit ++ // this.lastTick = MinecraftServer.currentTick; // CraftBukkit // Paper - remove anti tick skipping measures / wall time + } + + @Override +@@ -0,0 +0,0 @@ public class Zombie extends Monster { + } + // Paper end - Add more Zombie API + public void startUnderWaterConversion(int ticksUntilWaterConversion) { +- this.lastTick = MinecraftServer.currentTick; // CraftBukkit ++ // this.lastTick = MinecraftServer.currentTick; // CraftBukkit // Paper - remove anti tick skipping measures / wall time + this.conversionTime = ticksUntilWaterConversion; + this.getEntityData().set(Zombie.DATA_DROWNED_CONVERSION_ID, true); + } +diff --git a/src/main/java/net/minecraft/world/level/block/entity/BrewingStandBlockEntity.java b/src/main/java/net/minecraft/world/level/block/entity/BrewingStandBlockEntity.java +index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 +--- a/src/main/java/net/minecraft/world/level/block/entity/BrewingStandBlockEntity.java ++++ b/src/main/java/net/minecraft/world/level/block/entity/BrewingStandBlockEntity.java +@@ -0,0 +0,0 @@ public class BrewingStandBlockEntity extends BaseContainerBlockEntity implements + public int fuel; + protected final ContainerData dataAccess; + // CraftBukkit start - add fields and methods +- private int lastTick = MinecraftServer.currentTick; ++ // private int lastTick = MinecraftServer.currentTick; // Paper - remove anti tick skipping measures / wall time + public List transaction = new java.util.ArrayList(); + private int maxStack = MAX_STACK; + +@@ -0,0 +0,0 @@ public class BrewingStandBlockEntity extends BaseContainerBlockEntity implements + boolean flag1 = blockEntity.brewTime > 0; + ItemStack itemstack1 = (ItemStack) blockEntity.items.get(3); + +- // CraftBukkit start - Use wall time instead of ticks for brewing +- int elapsedTicks = MinecraftServer.currentTick - blockEntity.lastTick; +- blockEntity.lastTick = MinecraftServer.currentTick; ++ // Paper - remove anti tick skipping measures / wall time + + if (flag1) { +- blockEntity.brewTime -= elapsedTicks; ++ --blockEntity.brewTime; // Paper - remove anti tick skipping measures / wall time - revert to vanilla + boolean flag2 = blockEntity.brewTime <= 0; // == -> <= + // CraftBukkit end + diff --git a/patches/server/Throw-exception-on-world-create-while-being-ticked.patch b/patches/server/Throw-exception-on-world-create-while-being-ticked.patch index 56024bdebc..e2320aca56 100644 --- a/patches/server/Throw-exception-on-world-create-while-being-ticked.patch +++ b/patches/server/Throw-exception-on-world-create-while-being-ticked.patch @@ -11,9 +11,9 @@ index 0000000000000000000000000000000000000000..00000000000000000000000000000000 --- a/src/main/java/net/minecraft/server/MinecraftServer.java +++ b/src/main/java/net/minecraft/server/MinecraftServer.java @@ -0,0 +0,0 @@ public abstract class MinecraftServer extends ReentrantBlockableEventLoop S spin(Function serverFactory) { diff --git a/patches/server/don-t-go-below-0-for-pickupDelay-breaks-picking-up-i.patch b/patches/server/don-t-go-below-0-for-pickupDelay-breaks-picking-up-i.patch deleted file mode 100644 index a3dec08b3a..0000000000 --- a/patches/server/don-t-go-below-0-for-pickupDelay-breaks-picking-up-i.patch +++ /dev/null @@ -1,27 +0,0 @@ -From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 -From: Aikar -Date: Sun, 24 Mar 2019 18:09:20 -0400 -Subject: [PATCH] don't go below 0 for pickupDelay, breaks picking up items - -vanilla checks for == 0 - -diff --git a/src/main/java/net/minecraft/world/entity/item/ItemEntity.java b/src/main/java/net/minecraft/world/entity/item/ItemEntity.java -index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 ---- a/src/main/java/net/minecraft/world/entity/item/ItemEntity.java -+++ b/src/main/java/net/minecraft/world/entity/item/ItemEntity.java -@@ -0,0 +0,0 @@ public class ItemEntity extends Entity implements TraceableEntity { - // CraftBukkit start - Use wall time for pickup and despawn timers - int elapsedTicks = MinecraftServer.currentTick - this.lastTick; - if (this.pickupDelay != 32767) this.pickupDelay -= elapsedTicks; -+ this.pickupDelay = Math.max(0, this.pickupDelay); // Paper - don't go below 0 - if (this.age != -32768) this.age += elapsedTicks; - this.lastTick = MinecraftServer.currentTick; - // CraftBukkit end -@@ -0,0 +0,0 @@ public class ItemEntity extends Entity implements TraceableEntity { - // CraftBukkit start - Use wall time for pickup and despawn timers - int elapsedTicks = MinecraftServer.currentTick - this.lastTick; - if (this.pickupDelay != 32767) this.pickupDelay -= elapsedTicks; -+ this.pickupDelay = Math.max(0, this.pickupDelay); // Paper - don't go below 0 - if (this.age != -32768) this.age += elapsedTicks; - this.lastTick = MinecraftServer.currentTick; - // CraftBukkit end