PaperMC/Spigot-Server-Patches/0502-Optimize-entity-list-iteration-requiring-entities-be.patch

From d7c73b33312bee2b2fe27b6691ad4b5ccb20a383 Mon Sep 17 00:00:00 2001
From: Spottedleaf <Spottedleaf@users.noreply.github.com>
Date: Tue, 5 May 2020 19:49:23 -0700
Subject: [PATCH] Optimize entity list iteration requiring entities be in
 loaded chunks

We retain a list of loaded entities specifically for this usage

diff --git a/src/main/java/net/minecraft/server/Chunk.java b/src/main/java/net/minecraft/server/Chunk.java
index 00dd21205d..53d3acccd3 100644
--- a/src/main/java/net/minecraft/server/Chunk.java
+++ b/src/main/java/net/minecraft/server/Chunk.java
@@ -801,6 +801,7 @@ public class Chunk implements IChunkAccess {
         this.setNeighbourLoaded(0, 0, this);
         this.loadedTicketLevel = true;
         // Paper end - neighbour cache
+        ((WorldServer)this.world).onChunkLoad(this); // Paper - optimise entity list iteration
         org.bukkit.Server server = this.world.getServer();
         ((WorldServer)this.world).getChunkProvider().addLoadedChunk(this); // Paper
         if (server != null) {
@@ -859,6 +860,7 @@ public class Chunk implements IChunkAccess {
         this.loadedTicketLevel = false;
         this.resetNeighbours();
         // Paper end
+        ((WorldServer)this.world).onChunkUnload(this); // Paper - optimise entity list iteration
     }
     // CraftBukkit end
 
diff --git a/src/main/java/net/minecraft/server/WorldServer.java b/src/main/java/net/minecraft/server/WorldServer.java
index b3785775ec..d9b3aa285a 100644
--- a/src/main/java/net/minecraft/server/WorldServer.java
+++ b/src/main/java/net/minecraft/server/WorldServer.java
@@ -189,6 +189,25 @@ public class WorldServer extends World {
     }
     // Paper end - rewrite ticklistserver
 
+    // Paper start - Optimize entity list iteration requiring entities be in loaded chunks
+    public final com.destroystokyo.paper.util.maplist.EntityList loadedEntities = new com.destroystokyo.paper.util.maplist.EntityList();
+    void onChunkLoad(final Chunk chunk) {
+        final com.destroystokyo.paper.util.maplist.EntityList list = chunk.entities;
+        final Entity[] entities = list.getRawData();
+        for (int i = 0, size = list.size(); i < size; ++i) {
+            this.loadedEntities.add(entities[i]);
+        }
+    }
+
+    void onChunkUnload(final Chunk chunk) {
+        final com.destroystokyo.paper.util.maplist.EntityList list = chunk.entities;
+        final Entity[] entities = list.getRawData();
+        for (int i = 0, size = list.size(); i < size; ++i) {
+            this.loadedEntities.remove(entities[i]);
+        }
+    }
+    // Paper end - Optimize entity list iteration requiring entities be in loaded chunks
+
     // Add env and gen to constructor
     public WorldServer(MinecraftServer minecraftserver, Executor executor, WorldNBTStorage worldnbtstorage, WorldData worlddata, DimensionManager dimensionmanager, GameProfilerFiller gameprofilerfiller, WorldLoadListener worldloadlistener, org.bukkit.World.Environment env, org.bukkit.generator.ChunkGenerator gen) {
         super(worlddata, dimensionmanager, (world, worldprovider) -> {
@@ -489,14 +508,13 @@ public class WorldServer extends World {
 
             gameprofilerfiller.exitEnter("regular");
             this.tickingEntities = true;
-            ObjectIterator objectiterator = this.entitiesById.int2ObjectEntrySet().iterator();
+            Iterator<Entity> entityiterator = this.loadedEntities.iterator(); // Paper - use loaded entity list - change var name to compile fail on usage change, we need to hook remove() calls here
 
             org.spigotmc.ActivationRange.activateEntities(this); // Spigot
             timings.entityTick.startTiming(); // Spigot
             TimingHistory.entityTicks += this.globalEntityList.size(); // Paper
-            while (objectiterator.hasNext()) {
-                Entry<Entity> entry = (Entry) objectiterator.next();
-                Entity entity1 = (Entity) entry.getValue();
+            while (entityiterator.hasNext()) { // Paper - use loaded entity list
+                Entity entity1 = entityiterator.next(); // Paper - use loaded entity list
                 Entity entity2 = entity1.getVehicle();
 
                 /* CraftBukkit start - We prevent spawning in general, so this butchering is not needed
@@ -532,7 +550,7 @@ public class WorldServer extends World {
                 gameprofilerfiller.enter("remove");
                 if (entity1.dead) {
                     this.removeEntityFromChunk(entity1);
-                    objectiterator.remove();
+                    entityiterator.remove(); this.entitiesById.remove(entity1.getId()); // Paper - use loaded entity list
                     this.unregisterEntity(entity1);
                 }
 
@@ -1445,6 +1463,7 @@ public class WorldServer extends World {
         if (entity instanceof EntityInsentient) {
             this.navigators.remove(((EntityInsentient) entity).getNavigation());
         }
+        this.loadedEntities.remove(entity); // Paper - loaded entity list
         new com.destroystokyo.paper.event.entity.EntityRemoveFromWorldEvent(entity.getBukkitEntity()).callEvent(); // Paper - fire while valid
         entity.valid = false; // CraftBukkit
     }
@@ -1514,6 +1533,11 @@ public class WorldServer extends World {
             }
             // Paper end
             entity.shouldBeRemoved = false; // Paper - shouldn't be removed after being re-added
+            // Paper start - loaded entity list
+            if (this.isChunkLoaded(net.minecraft.server.MCUtil.getChunkCoordinate(entity.locX()), net.minecraft.server.MCUtil.getChunkCoordinate(entity.locZ()))) {
+                this.loadedEntities.add(entity);
+            }
+            // Paper end - loaded entity list
             new com.destroystokyo.paper.event.entity.EntityAddToWorldEvent(entity.getBukkitEntity()).callEvent(); // Paper - fire while valid
         }
 
diff --git a/src/main/java/org/bukkit/craftbukkit/CraftWorld.java b/src/main/java/org/bukkit/craftbukkit/CraftWorld.java
index ac257d50de..995f706678 100644
--- a/src/main/java/org/bukkit/craftbukkit/CraftWorld.java
+++ b/src/main/java/org/bukkit/craftbukkit/CraftWorld.java
@@ -1110,16 +1110,16 @@ public class CraftWorld implements World {
 
     @Override
     public List<Entity> getEntities() {
-        List<Entity> list = new ArrayList<Entity>();
+        List<Entity> list = new ArrayList<Entity>(world.loadedEntities.size()); // Paper - optimize this call
 
-        for (Object o : world.entitiesById.values()) {
+        for (Object o : world.loadedEntities) { // Paper - optimize this call
             if (o instanceof net.minecraft.server.Entity) {
                 net.minecraft.server.Entity mcEnt = (net.minecraft.server.Entity) o;
                 if (mcEnt.shouldBeRemoved) continue; // Paper
                 Entity bukkitEntity = mcEnt.getBukkitEntity();
 
                 // Assuming that bukkitEntity isn't null
-                if (bukkitEntity != null && bukkitEntity.isValid()) {
+                if (bukkitEntity != null && CraftEntity.canBeSeenByPlugins(bukkitEntity)) { // Paper - optimize this call
                     list.add(bukkitEntity);
                 }
             }
@@ -1130,16 +1130,16 @@ public class CraftWorld implements World {
 
     @Override
     public List<LivingEntity> getLivingEntities() {
-        List<LivingEntity> list = new ArrayList<LivingEntity>();
+        List<LivingEntity> list = new ArrayList<LivingEntity>(world.loadedEntities.size()); // Paper - optimize this call
 
-        for (Object o : world.entitiesById.values()) {
+        for (Object o : world.loadedEntities) { // Paper - optimize this call
             if (o instanceof net.minecraft.server.Entity) {
                 net.minecraft.server.Entity mcEnt = (net.minecraft.server.Entity) o;
                 if (mcEnt.shouldBeRemoved) continue; // Paper
                 Entity bukkitEntity = mcEnt.getBukkitEntity();
 
                 // Assuming that bukkitEntity isn't null
-                if (bukkitEntity != null && bukkitEntity instanceof LivingEntity && bukkitEntity.isValid()) {
+                if (bukkitEntity != null && bukkitEntity instanceof LivingEntity && CraftEntity.canBeSeenByPlugins(bukkitEntity)) { // Paper - optimize this call
                     list.add((LivingEntity) bukkitEntity);
                 }
             }
@@ -1160,7 +1160,7 @@ public class CraftWorld implements World {
     public <T extends Entity> Collection<T> getEntitiesByClass(Class<T> clazz) {
         Collection<T> list = new ArrayList<T>();
 
-        for (Object entity: world.entitiesById.values()) {
+        for (Object entity: world.loadedEntities) { // Paper - optimize this call
             if (entity instanceof net.minecraft.server.Entity) {
                 if (((net.minecraft.server.Entity) entity).shouldBeRemoved) continue; // Paper
                 Entity bukkitEntity = ((net.minecraft.server.Entity) entity).getBukkitEntity();
@@ -1171,7 +1171,7 @@ public class CraftWorld implements World {
 
                 Class<?> bukkitClass = bukkitEntity.getClass();
 
-                if (clazz.isAssignableFrom(bukkitClass) && bukkitEntity.isValid()) {
+                if (clazz.isAssignableFrom(bukkitClass) && CraftEntity.canBeSeenByPlugins(bukkitEntity)) { // Paper - optimize this call
                     list.add((T) bukkitEntity);
                 }
             }
@@ -1184,7 +1184,7 @@ public class CraftWorld implements World {
     public Collection<Entity> getEntitiesByClasses(Class<?>... classes) {
         Collection<Entity> list = new ArrayList<Entity>();
 
-        for (Object entity: world.entitiesById.values()) {
+        for (Object entity: world.loadedEntities) { // Paper - optimize this call
             if (entity instanceof net.minecraft.server.Entity) {
                 if (((net.minecraft.server.Entity) entity).shouldBeRemoved) continue; // Paper
                 Entity bukkitEntity = ((net.minecraft.server.Entity) entity).getBukkitEntity();
@@ -1197,7 +1197,7 @@ public class CraftWorld implements World {
 
                 for (Class<?> clazz : classes) {
                     if (clazz.isAssignableFrom(bukkitClass)) {
-                        if (bukkitEntity.isValid()) {
+                        if (CraftEntity.canBeSeenByPlugins(bukkitEntity)) { // Paper - optimize this call
                             list.add(bukkitEntity);
                         }
                         break;
diff --git a/src/main/java/org/bukkit/craftbukkit/entity/CraftEntity.java b/src/main/java/org/bukkit/craftbukkit/entity/CraftEntity.java
index ff60568ce4..f377112161 100644
--- a/src/main/java/org/bukkit/craftbukkit/entity/CraftEntity.java
+++ b/src/main/java/org/bukkit/craftbukkit/entity/CraftEntity.java
@@ -180,6 +180,18 @@ public abstract class CraftEntity implements org.bukkit.entity.Entity {
         this.entity = entity;
     }
 
+    // Paper start
+    // note: this does not check isChunkLoaded, use Entity#isValid to do that
+    public static boolean canBeSeenByPlugins(org.bukkit.entity.Entity entity) {
+        Entity handle = ((CraftEntity)entity).getHandle();
+        // TODO
+        // isAlive is a dumb choice, given living entities aren't alive (but are in the world) if health < 0
+        // this needs to be brought up to spigot to fix though, we are NOT breaking api implementation, especially
+        // if no-one's complained.
+        return !handle.shouldBeRemoved && handle.isAlive() && handle.valid;
+    }
+    // Paper end
+
     @Override
     public Chunk getChunk() {
         net.minecraft.server.Chunk currentChunk = entity.getCurrentChunk();
-- 
2.26.0