-
Bug
-
Resolution: Invalid
-
None
-
Minecraft 1.13-pre7
-
None
-
Unconfirmed
The bug
The server tick logic (MinecraftServer#run()) was changed in one of the 1.13 snapshots or pre-releases. The new logic appears to be faulty (the previous was faulty as well though, see MC-121196).
In the section "Suggested fix" below is a suggested fix and I would strongly advise to use it or at least to use parts of it.
long l2 = k.b() - this.aa; if (l2 > 2000 && this.aa - this.Q >= 15000) { long l3 = l2 / 50; h.warn("Can't keep up! Is the server overloaded? Running {}ms or {} ticks behind", (Object)l2, (Object)l3); this.aa += l3 * 50; this.Q = this.aa; } this.v(); this.aa += 50; while (k.b() < this.aa) { Thread.sleep(1); } this.P = true;
long backlogMillis = currentMillis() - this.lastTickExpectedEnd; if (backlogMillis > 2000 && this.lastTickExpectedEnd - this.lastWarnTime >= 15000) { long backlogTicks = backlogMillis / 50; h.warn("Can't keep up! Is the server overloaded? Running {}ms or {} ticks behind", (Object)backlogMillis, (Object)backlogTicks); this.lastTickExpectedEnd += backlogTicks * 50; this.lastWarnTime = this.lastTickExpectedEnd; } this.tick(); this.lastTickExpectedEnd += 50; while (currentMillis() < this.lastTickExpectedEnd) { Thread.sleep(1); } this.isServerRunning = true;
It appears the line containing the backlog logic is incorrect, it should subtract the backlog ticks. Otherwise a server already running behind will sleep even longer, is should be:
this.lastTickExpectedEnd -= backlogTicks * 50;
Assuming that is the case, there are still the following problems:
- Possible problems when nanoTime() (used for currentMillis()) overflows; the Java docs suggest to perform comparisons as "t1 - t0 < 0, not t1 < t0"
- Thread.sleep(1) might switch control too often to the server thread even if it still has not reached lastTickExpectedEnd yet
- The server is not able to catch up after running behind without accumulating the backlog to 2000 milliseconds, because only then the lastTickExpectedEnd is adjusted; and always 50 milliseconds are added to the expected end
- It always adds 50 milliseconds to the expected end, even if the backlogTicks had an remainder (this is related to the point above, not being able to catch up)
- The lastWarnTime depends on the tick speed instead of directly using currentMillis()
Suggested fix
The following suggested fix should address all issues mentioned above. Additionally it runs backlog ticks in a loop and does not query the current millis for each iteration then.
Feel free to use it; the author does not have to be mentioned.
At the moment this fix is only theoretical and not tested yet.
Field changes:
- Remove lastTickExpectedEnd
- Add new field this.lastTickEnd, initialized with currentMillis()
Two new local variables outside the while (this.serverRunning) loop are needed:
- backlogMillis = 0
- static maxTickLengthMillis = 50
The following is the content of the while (this.serverRunning) loop:
backlogMillis += maxTickLengthMillis; while (backlogMillis >= maxTickLengthMillis) { backlogMillis -= maxTickLengthMillis; tick(); this.serverIsRunning = true; } long currentMillis = currentMillis(); long remainingMillis = maxTickLengthMillis - (currentMillis - this.lastTickEnd) - backlogMillis; if (remainingMillis > 0) { Thread.sleep(remainingMillis); this.lastTickEnd = currentMillis + remainingMillis; backlogMillis = 0; } else { this.lastTickEnd = currentMillis; backlogMillis = -remainingMillis; // Position to show warnings; use currentMillis as time value }
Notes
- The field isServerRunning can probably be removed since it looks like it is not used anymore
- The watchdog code has to be changed; whatever is used to signal that the server is still running, it has to be in the loop containing the call to tick()