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.
This commit is contained in:
parent
5c82955733
commit
c5a10665b8
794 changed files with 402 additions and 282 deletions
|
@ -0,0 +1,103 @@
|
|||
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
|
||||
From: Aikar <aikar@aikar.co>
|
||||
Date: Tue, 9 Jun 2020 03:33:03 -0400
|
||||
Subject: [PATCH] Add Plugin Tickets to API Chunk Methods
|
||||
|
||||
Like previous versions, plugins loading chunks kept them loaded until
|
||||
they garbage collected to avoid constant spamming of chunk loads
|
||||
|
||||
This adds tickets to a few more places so that they can be unloaded.
|
||||
|
||||
Additionally, this drops their ticket level to BORDER so they wont be ticking
|
||||
so they will just sit inactive instead.
|
||||
|
||||
Using .loadChunk to keep a chunk ticking was a horrible idea for upstream
|
||||
when we have TWO methods that are able to do that already in the API.
|
||||
|
||||
Also reduce their collection count down to a maximum of 1 second. Barely
|
||||
anyone knows what chunk-gc is in bukkit.yml as its less relevant now, and
|
||||
since this wasn't spigot behavior, this is safe to mostly ignore (unless someone
|
||||
wants it to collect even faster, they can restore that setting back to 1 instead of 20+)
|
||||
|
||||
Not adding it to .getType() though to keep behavior consistent with vanilla for performance reasons.
|
||||
|
||||
diff --git a/src/main/java/org/bukkit/craftbukkit/CraftServer.java b/src/main/java/org/bukkit/craftbukkit/CraftServer.java
|
||||
index 89c4763b990ca9e801ee16c4a50e806740d061d9..0b288cc080869efed42a28be682f7b68f8691416 100644
|
||||
--- a/src/main/java/org/bukkit/craftbukkit/CraftServer.java
|
||||
+++ b/src/main/java/org/bukkit/craftbukkit/CraftServer.java
|
||||
@@ -380,7 +380,7 @@ public final class CraftServer implements Server {
|
||||
this.overrideSpawnLimits();
|
||||
console.autosavePeriod = this.configuration.getInt("ticks-per.autosave");
|
||||
this.warningState = WarningState.value(this.configuration.getString("settings.deprecated-verbose"));
|
||||
- TicketType.PLUGIN.timeout = this.configuration.getInt("chunk-gc.period-in-ticks");
|
||||
+ TicketType.PLUGIN.timeout = Math.min(20, this.configuration.getInt("chunk-gc.period-in-ticks")); // Paper - cap plugin loads to 1 second
|
||||
this.minimumAPI = ApiVersion.getOrCreateVersion(this.configuration.getString("settings.minimum-api"));
|
||||
this.loadIcon();
|
||||
this.loadCompatibilities();
|
||||
@@ -975,7 +975,7 @@ public final class CraftServer implements Server {
|
||||
this.console.setMotd(config.motd);
|
||||
this.overrideSpawnLimits();
|
||||
this.warningState = WarningState.value(this.configuration.getString("settings.deprecated-verbose"));
|
||||
- TicketType.PLUGIN.timeout = this.configuration.getInt("chunk-gc.period-in-ticks");
|
||||
+ TicketType.PLUGIN.timeout = Math.min(20, configuration.getInt("chunk-gc.period-in-ticks")); // Paper - cap plugin loads to 1 second
|
||||
this.minimumAPI = ApiVersion.getOrCreateVersion(this.configuration.getString("settings.minimum-api"));
|
||||
this.printSaveWarning = false;
|
||||
this.console.autosavePeriod = this.configuration.getInt("ticks-per.autosave");
|
||||
diff --git a/src/main/java/org/bukkit/craftbukkit/CraftWorld.java b/src/main/java/org/bukkit/craftbukkit/CraftWorld.java
|
||||
index a381688de78f5a78980b660f9765f839413a6c65..6e01013228798f9682624e912618500f0d4fa485 100644
|
||||
--- a/src/main/java/org/bukkit/craftbukkit/CraftWorld.java
|
||||
+++ b/src/main/java/org/bukkit/craftbukkit/CraftWorld.java
|
||||
@@ -266,7 +266,13 @@ public class CraftWorld extends CraftRegionAccessor implements World {
|
||||
|
||||
@Override
|
||||
public Chunk getChunkAt(int x, int z) {
|
||||
- net.minecraft.world.level.chunk.LevelChunk chunk = (net.minecraft.world.level.chunk.LevelChunk) this.world.getChunk(x, z, ChunkStatus.FULL, true);
|
||||
+ // Paper start - add ticket to hold chunk for a little while longer if plugin accesses it
|
||||
+ net.minecraft.world.level.chunk.LevelChunk chunk = this.world.getChunkSource().getChunkAtIfLoadedImmediately(x, z);
|
||||
+ if (chunk == null) {
|
||||
+ this.addTicket(x, z);
|
||||
+ chunk = this.world.getChunkSource().getChunk(x, z, true);
|
||||
+ }
|
||||
+ // Paper end
|
||||
return new CraftChunk(chunk);
|
||||
}
|
||||
|
||||
@@ -280,6 +286,12 @@ public class CraftWorld extends CraftRegionAccessor implements World {
|
||||
return new CraftChunk(this.getHandle(), x, z);
|
||||
}
|
||||
|
||||
+ // Paper start
|
||||
+ private void addTicket(int x, int z) {
|
||||
+ io.papermc.paper.util.MCUtil.MAIN_EXECUTOR.execute(() -> this.world.getChunkSource().addRegionTicket(TicketType.PLUGIN, new ChunkPos(x, z), 0, Unit.INSTANCE)); // Paper
|
||||
+ }
|
||||
+ // Paper end
|
||||
+
|
||||
@Override
|
||||
public Chunk getChunkAt(Block block) {
|
||||
Preconditions.checkArgument(block != null, "null block");
|
||||
@@ -331,7 +343,7 @@ public class CraftWorld extends CraftRegionAccessor implements World {
|
||||
public boolean unloadChunkRequest(int x, int z) {
|
||||
org.spigotmc.AsyncCatcher.catchOp("chunk unload"); // Spigot
|
||||
if (this.isChunkLoaded(x, z)) {
|
||||
- this.world.getChunkSource().removeRegionTicket(TicketType.PLUGIN, new ChunkPos(x, z), 1, Unit.INSTANCE);
|
||||
+ this.world.getChunkSource().removeRegionTicket(TicketType.PLUGIN, new ChunkPos(x, z), 0, Unit.INSTANCE); // Paper
|
||||
}
|
||||
|
||||
return true;
|
||||
@@ -441,7 +453,7 @@ public class CraftWorld extends CraftRegionAccessor implements World {
|
||||
}
|
||||
|
||||
if (chunk instanceof net.minecraft.world.level.chunk.LevelChunk) {
|
||||
- this.world.getChunkSource().addRegionTicket(TicketType.PLUGIN, new ChunkPos(x, z), 1, Unit.INSTANCE);
|
||||
+ this.world.getChunkSource().addRegionTicket(TicketType.PLUGIN, new ChunkPos(x, z), 0, Unit.INSTANCE); // Paper
|
||||
return true;
|
||||
}
|
||||
|
||||
@@ -2241,6 +2253,7 @@ public class CraftWorld extends CraftRegionAccessor implements World {
|
||||
ca.spottedleaf.moonrise.common.util.ChunkSystem.scheduleChunkLoad(this.getHandle(), x, z, gen, ChunkStatus.FULL, true, priority, (c) -> {
|
||||
net.minecraft.server.MinecraftServer.getServer().scheduleOnMain(() -> {
|
||||
net.minecraft.world.level.chunk.LevelChunk chunk = (net.minecraft.world.level.chunk.LevelChunk)c;
|
||||
+ if (chunk != null) this.addTicket(x, z); // Paper
|
||||
ret.complete(chunk == null ? null : new CraftChunk(chunk));
|
||||
});
|
||||
});
|
Loading…
Add table
Add a link
Reference in a new issue