Entity tracker optimizations (#232)

* Async entity tracking

This is very basic, and I plan to commit the full entity tracker optimizations in the near future.

* Fix compatibility with Citizens

This also simplifies the patch quite a bit

* Make the patch much more safer

* Current progress

Maybe most of those \"ensureMain\" calls are unnecessary, but it has to be tested.
the only problems now are that entities are teleporting instead of normally moving

* Fix entities "teleporting"

* Fixup some of the diff

* Add some notes

Co-authored-by: Ivan Pekov <ivan@mrivanplays.com>
This commit is contained in:
Mykyta 2020-10-05 20:05:57 -07:00 committed by GitHub
parent 2019524096
commit c8320920b0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 347 additions and 0 deletions

View File

@ -36,6 +36,7 @@ # Patches
| server | Allow to change the piston push limit | tr7zw | |
| server | Alternative Keepalive Handling | William Blake Galbreath | |
| server | Apply advancements async | Mariell Hoversholm | |
| server | Async entity tracking | Mykyta Komarn | Ivan Pekov |
| server | Avoid double I/O operation on load player file | ㄗㄠˋ ㄑㄧˊ | |
| server | Barrels and enderchests 6 rows | William Blake Galbreath | |
| server | Brandings | tr7zw | |

View File

@ -0,0 +1,346 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Mykyta Komarn <nkomarn@hotmail.com>
Date: Sat, 3 Oct 2020 22:40:00 -0700
Subject: [PATCH] Async entity tracking
Entity tracking is done the same way, but asynchronously.
Lowers the MSPT and NSPT by a lot.
The changes made:
Synchronised the trackedEntities map in PlayerChunkMap, also added additional lock to it.
Created a tracker executor which is processing the track queue asynchronously.
A minor change was to replace 2 array lists with our GlueList implementation.
Certain stuff had to be ensured to be called on the main thread when the tracker entry was ticked due to how
the game's code works and due to how bukkit's event system works.
Entity tracking is some of the heavy stuff the server does. Making this async is trivial for making the performance
go up.
Co-authored-by: Ivan Pekov <ivan@mrivanplays.com>
diff --git a/src/main/java/net/minecraft/server/EntityPlayer.java b/src/main/java/net/minecraft/server/EntityPlayer.java
index a61b449ede25a3215a48002a3223c8430b74d835..450d592e51cbeed97a2e9220050a2761ef0f4ff7 100644
--- a/src/main/java/net/minecraft/server/EntityPlayer.java
+++ b/src/main/java/net/minecraft/server/EntityPlayer.java
@@ -1804,10 +1804,18 @@ public class EntityPlayer extends EntityHuman implements ICrafting {
this.getBukkitEntity().teleport(new Location(newSpectatorTarget.getWorld().getWorld(), newSpectatorTarget.locX(), newSpectatorTarget.locY(), newSpectatorTarget.locZ(), this.yaw, this.pitch), TeleportCause.SPECTATE); // Correctly handle cross-world entities from api calls by using CB teleport
// Make sure we're tracking the entity before sending
- PlayerChunkMap.EntityTracker tracker = ((WorldServer)newSpectatorTarget.world).getChunkProvider().playerChunkMap.trackedEntities.get(newSpectatorTarget.getId());
+ // Yatopia start
+ PlayerChunkMap chunkMap = ((WorldServer)newSpectatorTarget.world).getChunkProvider().playerChunkMap;
+ chunkMap.trackedEntitiesLock.lock();
+ try {
+ PlayerChunkMap.EntityTracker tracker = chunkMap.trackedEntities.get(newSpectatorTarget.getId());
+ // Yatopia end
if (tracker != null) { // dumb plugins...
tracker.updatePlayer(this);
}
+ } finally { // Yatopia start
+ chunkMap.trackedEntitiesLock.unlock();
+ } // Yatopia end
} else {
this.playerConnection.teleport(this.spectatedEntity.locX(), this.spectatedEntity.locY(), this.spectatedEntity.locZ(), this.yaw, this.pitch, TeleportCause.SPECTATE); // CraftBukkit
}
diff --git a/src/main/java/net/minecraft/server/EntityTNTPrimed.java b/src/main/java/net/minecraft/server/EntityTNTPrimed.java
index 2c058a9eb4ce77f6090f3516ea048483173e7511..7678ee6f8fbe5d40739f8426724447268474f3ab 100644
--- a/src/main/java/net/minecraft/server/EntityTNTPrimed.java
+++ b/src/main/java/net/minecraft/server/EntityTNTPrimed.java
@@ -87,7 +87,12 @@ public class EntityTNTPrimed extends Entity {
*/
// Send position and velocity updates to nearby players on every tick while the TNT is in water.
// This does pretty well at keeping their clients in sync with the server.
- PlayerChunkMap.EntityTracker ete = ((WorldServer)this.world).getChunkProvider().playerChunkMap.trackedEntities.get(this.getId());
+ // Yatopia start
+ PlayerChunkMap chunkMap = ((WorldServer)this.world).getChunkProvider().playerChunkMap;
+ chunkMap.trackedEntitiesLock.lock();
+ try {
+ PlayerChunkMap.EntityTracker ete = chunkMap.trackedEntities.get(this.getId());
+ // Yatopia end
if (ete != null) {
PacketPlayOutEntityVelocity velocityPacket = new PacketPlayOutEntityVelocity(this);
PacketPlayOutEntityTeleport positionPacket = new PacketPlayOutEntityTeleport(this);
@@ -110,6 +115,9 @@ public class EntityTNTPrimed extends Entity {
}
// Akarin end
}
+ } finally { // Yatopia start
+ chunkMap.trackedEntitiesLock.unlock();
+ } // Yatopia end
}
// Paper end
}
diff --git a/src/main/java/net/minecraft/server/EntityTrackerEntry.java b/src/main/java/net/minecraft/server/EntityTrackerEntry.java
index aea72b0db10eed151db18490c02f291c3cded92a..f379d591ed7083840e15ba94f514b4bf06e52010 100644
--- a/src/main/java/net/minecraft/server/EntityTrackerEntry.java
+++ b/src/main/java/net/minecraft/server/EntityTrackerEntry.java
@@ -39,6 +39,7 @@ public class EntityTrackerEntry {
private boolean r;
// CraftBukkit start
final Set<EntityPlayer> trackedPlayers; // Paper - private -> package
+ final java.util.concurrent.locks.Lock trackedPlayersLock = new java.util.concurrent.locks.ReentrantLock(); // Yatopia
// Paper start
private java.util.Map<EntityPlayer, Boolean> trackedPlayerMap = null;
@@ -74,7 +75,7 @@ public class EntityTrackerEntry {
public final void tick() { this.a(); } // Paper - OBFHELPER
public void a() {
- com.tuinity.tuinity.util.TickThread.softEnsureTickThread("Tracker update"); // Tuinity
+ // com.tuinity.tuinity.util.TickThread.softEnsureTickThread("Tracker update"); // Tuinity // Yatopia
List<Entity> list = this.tracker.getPassengers();
if (!list.equals(this.p)) {
@@ -89,18 +90,25 @@ public class EntityTrackerEntry {
if (this.tickCounter % 10 == 0 && itemstack.getItem() instanceof ItemWorldMap) { // CraftBukkit - Moved this.tickCounter % 10 logic here so item frames do not enter the other blocks
WorldMap worldmap = ItemWorldMap.getSavedMap(itemstack, this.b);
+ trackedPlayersLock.lock(); // Yatopia
+ try { // Yatopia
Iterator iterator = this.trackedPlayers.iterator(); // CraftBukkit
while (iterator.hasNext()) {
EntityPlayer entityplayer = (EntityPlayer) iterator.next();
+ MCUtil.ensureMain(() -> { // Yatopia - ensure worldmap updates are done on the main thread
worldmap.a((EntityHuman) entityplayer, itemstack);
+ }); // Yatopia
Packet<?> packet = ((ItemWorldMap) itemstack.getItem()).a(itemstack, (World) this.b, (EntityHuman) entityplayer);
if (packet != null) {
entityplayer.playerConnection.sendPacket(packet);
}
}
+ } finally { // Yatopia start
+ trackedPlayersLock.unlock();
+ } // Yatopia end
}
this.c();
@@ -240,6 +248,7 @@ public class EntityTrackerEntry {
++this.tickCounter;
if (this.tracker.velocityChanged) {
// CraftBukkit start - Create PlayerVelocity event
+ MCUtil.ensureMain(() -> { // Yatopia - ensure this is called on the main thread
boolean cancelled = false;
if (this.tracker instanceof EntityPlayer) {
@@ -259,6 +268,7 @@ public class EntityTrackerEntry {
if (!cancelled) {
this.broadcastIncludingSelf(new PacketPlayOutEntityVelocity(this.tracker));
}
+ }); // Yatopia
// CraftBukkit end
this.tracker.velocityChanged = false;
}
diff --git a/src/main/java/net/minecraft/server/PlayerChunkMap.java b/src/main/java/net/minecraft/server/PlayerChunkMap.java
index 9f32a26fdbfaf024cfe5c0996c2253f2dd581d5e..807a54e24018455a997e4410cdb7579dac13bd8d 100644
--- a/src/main/java/net/minecraft/server/PlayerChunkMap.java
+++ b/src/main/java/net/minecraft/server/PlayerChunkMap.java
@@ -11,6 +11,7 @@ import com.google.common.collect.Sets;
import com.mojang.datafixers.DataFixer;
import com.mojang.datafixers.util.Either;
import it.unimi.dsi.fastutil.ints.Int2ObjectMap;
+import it.unimi.dsi.fastutil.ints.Int2ObjectMaps; // Yatopia
import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap;
import it.unimi.dsi.fastutil.longs.Long2ByteMap;
import it.unimi.dsi.fastutil.longs.Long2ByteOpenHashMap;
@@ -104,10 +105,12 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
private final File w;
private final PlayerMap playerMap;
public final Int2ObjectMap<PlayerChunkMap.EntityTracker> trackedEntities;
+ public final java.util.concurrent.locks.Lock trackedEntitiesLock = new java.util.concurrent.locks.ReentrantLock(); // Yatopia
private final Long2ByteMap z;
private final Queue<Runnable> A; private final Queue<Runnable> getUnloadQueueTasks() { return this.A; } // Paper - OBFHELPER
int viewDistance; // Paper - private -> package private
public final com.destroystokyo.paper.util.PlayerMobDistanceMap playerMobDistanceMap; // Paper
+ private static final java.util.concurrent.ExecutorService trackerExecutor = java.util.concurrent.Executors.newCachedThreadPool(new com.google.common.util.concurrent.ThreadFactoryBuilder().setDaemon(true).setNameFormat("Entity Tracker - %d").build()); // Yatopia
// CraftBukkit start - recursion-safe executor for Chunk loadCallback() and unloadCallback()
public final CallbackExecutor callbackExecutor = new CallbackExecutor();
@@ -292,7 +295,7 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
this.unloadQueue = new LongOpenHashSet();
this.u = new AtomicInteger();
this.playerMap = new PlayerMap();
- this.trackedEntities = new Int2ObjectOpenHashMap();
+ this.trackedEntities = Int2ObjectMaps.synchronize(new Int2ObjectOpenHashMap<>()); // Yatopia
this.z = new Long2ByteOpenHashMap();
this.A = new com.destroystokyo.paper.utils.CachedSizeConcurrentLinkedQueue<>(); // Paper - need constant-time size()
this.definedStructureManager = definedstructuremanager;
@@ -2033,6 +2036,8 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
protected void addEntity(Entity entity) {
org.spigotmc.AsyncCatcher.catchOp("entity track"); // Spigot
// Paper start - ignore and warn about illegal addEntity calls instead of crashing server
+ trackedEntitiesLock.lock(); // Yatopia
+ try { // Yatopia
if (!entity.valid || entity.world != this.world || this.trackedEntities.containsKey(entity.getId())) {
new Throwable("[ERROR] Illegal PlayerChunkMap::addEntity for world " + this.world.getWorld().getName()
+ ": " + entity + (this.trackedEntities.containsKey(entity.getId()) ? " ALREADY CONTAINED (This would have crashed your server)" : ""))
@@ -2072,10 +2077,15 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
}
}
+ } finally { // Yatopia start
+ trackedEntitiesLock.unlock();
+ } // Yatopia end
}
protected void removeEntity(Entity entity) {
org.spigotmc.AsyncCatcher.catchOp("entity untrack"); // Spigot
+ trackedEntitiesLock.lock(); // Yatopia
+ try { // Yatopia
if (entity instanceof EntityPlayer) {
EntityPlayer entityplayer = (EntityPlayer) entity;
@@ -2094,6 +2104,9 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
if (playerchunkmap_entitytracker1 != null) {
playerchunkmap_entitytracker1.a();
}
+ } finally { // Yatopia start
+ trackedEntitiesLock.unlock();
+ } // Yatopia end
entity.tracker = null; // Paper - We're no longer tracked
}
@@ -2102,12 +2115,14 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
this.world.timings.tracker1.startTiming();
try {
com.tuinity.tuinity.util.maplist.IteratorSafeOrderedReferenceSet.Iterator<Chunk> iterator = this.world.getChunkProvider().entityTickingChunks.iterator();
+ trackedEntitiesLock.lock(); // Yatopia
try {
while (iterator.hasNext()) {
Chunk chunk = iterator.next();
Entity[] entities = chunk.entities.getRawData();
for (int i = 0, len = chunk.entities.size(); i < len; ++i) {
Entity entity = entities[i];
+ if (entity == null) continue; // Yatopia
EntityTracker tracker = this.trackedEntities.get(entity.getId());
if (tracker != null) {
tracker.updatePlayers(tracker.tracker.getPlayersInTrackRange());
@@ -2117,6 +2132,7 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
}
} finally {
iterator.finishedIterating();
+ trackedEntitiesLock.unlock(); // Yatopia
}
} finally {
this.world.timings.tracker1.stopTiming();
@@ -2124,10 +2140,10 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
}
// Paper end - optimised tracker
- protected void g() {
+ protected void g() { // Yatopia - diff on change - make sure paper didn't drop tracker optimizations, otherwise kaboom
// Paper start - optimized tracker
if (true) {
- this.processTrackQueue();
+ trackerExecutor.execute(this::processTrackQueue); // Yatopia
return;
}
// Paper end - optimized tracker
@@ -2171,20 +2187,30 @@ public class PlayerChunkMap extends IChunkLoader implements PlayerChunk.d {
}
protected void broadcast(Entity entity, Packet<?> packet) {
+ trackedEntitiesLock.lock(); // Yatopia
+ try { // Yatopia
PlayerChunkMap.EntityTracker playerchunkmap_entitytracker = (PlayerChunkMap.EntityTracker) this.trackedEntities.get(entity.getId());
if (playerchunkmap_entitytracker != null) {
playerchunkmap_entitytracker.broadcast(packet);
}
+ } finally { // Yatopia start
+ trackedEntitiesLock.unlock();
+ } // Yatopia end
}
protected void broadcastIncludingSelf(Entity entity, Packet<?> packet) {
+ trackedEntitiesLock.lock(); // Yatopia
+ try { // Yatopia
PlayerChunkMap.EntityTracker playerchunkmap_entitytracker = (PlayerChunkMap.EntityTracker) this.trackedEntities.get(entity.getId());
if (playerchunkmap_entitytracker != null) {
playerchunkmap_entitytracker.broadcastIncludingSelf(packet);
}
+ } finally { // Yatopia start
+ trackedEntitiesLock.unlock();
+ } // Yatopia end
}
@@ -2297,11 +2323,13 @@ Sections go from 0..16. Now whenever a section is not empty, it can potentially
entityplayer.a(chunk.getPos(), apacket[0], apacket[1]);
PacketDebug.a(this.world, chunk.getPos());
- List<Entity> list = Lists.newArrayList();
- List<Entity> list1 = Lists.newArrayList();
+ List<Entity> list = new net.yatopia.server.list.GlueList<>(); // Yatopia
+ List<Entity> list1 = new net.yatopia.server.list.GlueList<>(); // Yatopia
// Paper start - optimise entity tracker
// use the chunk entity list, not the whole trackedEntities map...
Entity[] entities = chunk.entities.getRawData();
+ trackedEntitiesLock.lock(); // Yatopia
+ try { // Yatopia
for (int i = 0, size = chunk.entities.size(); i < size; ++i) {
Entity entity = entities[i];
if (entity == entityplayer) {
@@ -2323,6 +2351,9 @@ Sections go from 0..16. Now whenever a section is not empty, it can potentially
list1.add(entity);
}
}
+ } finally { // Yatopia start
+ trackedEntitiesLock.unlock();
+ } // Yatopia end
// Paper end - optimise entity tracker
Iterator iterator;
@@ -2461,7 +2492,7 @@ Sections go from 0..16. Now whenever a section is not empty, it can potentially
}
public void updatePlayer(EntityPlayer entityplayer) {
- org.spigotmc.AsyncCatcher.catchOp("player tracker update"); // Spigot
+ // org.spigotmc.AsyncCatcher.catchOp("player tracker update"); // Spigot // Yatopia
if (entityplayer != this.tracker) {
// Paper start - remove allocation of Vec3D here
//Vec3D vec3d = entityplayer.getPositionVector().d(this.tracker.getPositionVector()); // MC-155077, SPIGOT-5113
diff --git a/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java b/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java
index 715d2c5e888b4a5c955d2dee2429757a27b50b00..e1e1aafd5dde611a3f2c72273f7af431006851b1 100644
--- a/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java
+++ b/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java
@@ -1253,10 +1253,16 @@ public class CraftPlayer extends CraftHumanEntity implements Player {
private void unregisterPlayer(EntityPlayer other) {
PlayerChunkMap tracker = ((WorldServer) entity.world).getChunkProvider().playerChunkMap;
// Paper end
+ // Yatopia start
+ tracker.trackedEntitiesLock.lock();
+ try { // Yatopia end
PlayerChunkMap.EntityTracker entry = tracker.trackedEntities.get(other.getId());
if (entry != null) {
entry.clear(getHandle());
}
+ } finally { // Yatopia start
+ tracker.trackedEntitiesLock.unlock();
+ } // Yatopia end
// Remove the hidden player from this player user list, if they're on it
if (other.sentListPacket) {
@@ -1303,10 +1309,16 @@ public class CraftPlayer extends CraftHumanEntity implements Player {
getHandle().playerConnection.sendPacket(new PacketPlayOutPlayerInfo(PacketPlayOutPlayerInfo.EnumPlayerInfoAction.ADD_PLAYER, other));
+ // Yatopia start
+ tracker.trackedEntitiesLock.lock();
+ try { // Yatopia end
PlayerChunkMap.EntityTracker entry = tracker.trackedEntities.get(other.getId());
if (entry != null && !entry.trackedPlayers.contains(getHandle())) {
entry.updatePlayer(getHandle());
}
+ } finally { // Yatopia start
+ tracker.trackedEntitiesLock.unlock();
+ } // Yatopia end
}
// Paper start
private void reregisterPlayer(EntityPlayer player) {