From eac397cbab5ee2d60d09d7176b9cda3bb3fbc611 Mon Sep 17 00:00:00 2001 From: Aikar Date: Fri, 16 Oct 2015 22:23:05 -0500 Subject: [PATCH] Implement EMC's optimized entity and tileentity removal --- ...imize-Entity-and-Tile-Entity-Removal.patch | 236 ++++++++++++++++++ 1 file changed, 236 insertions(+) create mode 100644 Spigot-Server-Patches/Optimize-Entity-and-Tile-Entity-Removal.patch diff --git a/Spigot-Server-Patches/Optimize-Entity-and-Tile-Entity-Removal.patch b/Spigot-Server-Patches/Optimize-Entity-and-Tile-Entity-Removal.patch new file mode 100644 index 0000000000..0e18686cc4 --- /dev/null +++ b/Spigot-Server-Patches/Optimize-Entity-and-Tile-Entity-Removal.patch @@ -0,0 +1,236 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Aikar +Date: Fri, 16 Oct 2015 22:14:48 -0500 +Subject: [PATCH] Optimize Entity and Tile Entity Removal + +Java's implementation of List.removeAll is extremely slow. This is +currently causing lots of TPS loss when lots of chunk unload activity +occurs, as the process iterates the removal list for every entry in the +source list, resulting in O(n^2) performance. + +This change will switch the process to instead iterate over the +removal list, and marking a boolean that its removed. + +Then, we then iterate the source list and use a compaction technique +that skips any object marked for removal. + +After all live objects are compacted down, we do a range +removal to clear out any removed objects at the end of the current list. + +This gives us O(n) performance and a much cheaper overall operation. + +Compaction technique was originally used by CyberTiger in a different +implementation. + +Finally, we remove MOST single .remove() calls, and run a 2nd compaction +after ticking in order to remove the singles. + +This also fixes a bug with Tick Position in the Tick limiter, where +previously .removeAll would shift entity index order but the tick +position was never moved to its new location. + +diff --git a/src/main/java/net/minecraft/server/Entity.java b/src/main/java/net/minecraft/server/Entity.java +index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 +--- a/src/main/java/net/minecraft/server/Entity.java ++++ b/src/main/java/net/minecraft/server/Entity.java +@@ -0,0 +0,0 @@ import org.bukkit.event.entity.EntityPortalEvent; + import org.bukkit.plugin.PluginManager; + // CraftBukkit end + +-public abstract class Entity implements ICommandListener { ++// PaperSpigot start - Optimized entity and tileentity removal ++public abstract class Entity implements ICommandListener, org.github.paperspigot.OptimizedRemoveAll.Marker { ++ private boolean needsRemoved = false; ++ public boolean isToBeRemoved() { return needsRemoved; } ++ public void markToBeRemoved() { needsRemoved = true; } ++ // PaperSpigot end + + // CraftBukkit start + private static final int CURRENT_LEVEL = 2; +diff --git a/src/main/java/net/minecraft/server/TileEntity.java b/src/main/java/net/minecraft/server/TileEntity.java +index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 +--- a/src/main/java/net/minecraft/server/TileEntity.java ++++ b/src/main/java/net/minecraft/server/TileEntity.java +@@ -0,0 +0,0 @@ import org.apache.logging.log4j.Logger; + import org.spigotmc.CustomTimingsHandler; // Spigot + import org.bukkit.inventory.InventoryHolder; // CraftBukkit + +-public abstract class TileEntity { ++// PaperSpigot start - Optimized entity and tileentity removal ++public abstract class TileEntity implements org.github.paperspigot.OptimizedRemoveAll.Marker { ++ private boolean needsRemoved = false; ++ public boolean isToBeRemoved() { return needsRemoved; } ++ public void markToBeRemoved() { needsRemoved = true; } ++ // PaperSpigot end + + public CustomTimingsHandler tickTimer = org.bukkit.craftbukkit.SpigotTimings.getTileEntityTimings(this); // Spigot + private static final Logger a = LogManager.getLogger(); +diff --git a/src/main/java/net/minecraft/server/World.java b/src/main/java/net/minecraft/server/World.java +index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 +--- a/src/main/java/net/minecraft/server/World.java ++++ b/src/main/java/net/minecraft/server/World.java +@@ -0,0 +0,0 @@ public abstract class World implements IBlockAccess { + } + + this.methodProfiler.c("remove"); +- this.entityList.removeAll(this.g); ++ tickPosition = org.github.paperspigot.OptimizedRemoveAll.removeAll(this.entityList, this.g, tickPosition); // PaperSpigot + + int j; + int k; +@@ -0,0 +0,0 @@ public abstract class World implements IBlockAccess { + } + + guardEntityList = false; // Spigot +- this.entityList.remove(this.tickPosition--); // CraftBukkit - Use field for loop variable ++ if (entity instanceof EntityPlayer) this.entityList.remove(this.tickPosition--); // CraftBukkit - Use field for loop variable // PaperSpigot ++ else entity.markToBeRemoved(); // PaperSpigot + guardEntityList = true; // Spigot + this.b(entity); + } + + this.methodProfiler.b(); + } ++ tickPosition = org.github.paperspigot.OptimizedRemoveAll.removeAll(this.entityList, null, tickPosition); // PaperSpigot + guardEntityList = false; // Spigot + + timings.entityTick.stopTiming(); // Spigot +@@ -0,0 +0,0 @@ public abstract class World implements IBlockAccess { + this.M = true; + // CraftBukkit start - From below, clean up tile entities before ticking them + if (!this.c.isEmpty()) { +- this.tileEntityList.removeAll(this.c); ++ tileTickPosition = org.github.paperspigot.OptimizedRemoveAll.removeAll(this.tileEntityList, this.c, tileTickPosition); // PaperSpigot + //this.h.removeAll(this.c); // PaperSpigot - Remove unused list + this.c.clear(); + } +@@ -0,0 +0,0 @@ public abstract class World implements IBlockAccess { + + if (tileentity.x()) { + tilesThisCycle--; +- this.tileEntityList.remove(tileTickPosition--); ++ tileentity.markToBeRemoved(); // PaperSpigot ++ //this.tileEntityList.remove(tileTickPosition--); + //this.h.remove(tileentity); // PaperSpigot - Remove unused list + if (this.isLoaded(tileentity.getPosition())) { + this.getChunkAtWorldCoords(tileentity.getPosition()).e(tileentity.getPosition()); + } + } + } ++ tileTickPosition = org.github.paperspigot.OptimizedRemoveAll.removeAll(this.tileEntityList, null, tileTickPosition); // PaperSpigot + + timings.tileEntityTick.stopTiming(); // Spigot + timings.tileEntityPending.startTiming(); // Spigot +@@ -0,0 +0,0 @@ public abstract class World implements IBlockAccess { + + while (iterator.hasNext()) { + Entity entity = (Entity) iterator.next(); ++ if (entity.isToBeRemoved()) { continue; } // PaperSpigot + + if (oclass.isAssignableFrom(entity.getClass()) && predicate.apply((T) entity)) { // CraftBukkit - fix decompile error + arraylist.add(entity); +@@ -0,0 +0,0 @@ public abstract class World implements IBlockAccess { + + while (iterator.hasNext()) { + Entity entity = (Entity) iterator.next(); ++ if (entity.isToBeRemoved()) { continue; } // PaperSpigot + // CraftBukkit start - Split out persistent check, don't apply it to special persistent mobs + if (entity instanceof EntityInsentient) { + EntityInsentient entityinsentient = (EntityInsentient) entity; +diff --git a/src/main/java/org/bukkit/craftbukkit/CraftWorld.java b/src/main/java/org/bukkit/craftbukkit/CraftWorld.java +index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 100644 +--- a/src/main/java/org/bukkit/craftbukkit/CraftWorld.java ++++ b/src/main/java/org/bukkit/craftbukkit/CraftWorld.java +@@ -0,0 +0,0 @@ public class CraftWorld implements World { + Entity bukkitEntity = mcEnt.getBukkitEntity(); + + // Assuming that bukkitEntity isn't null +- if (bukkitEntity != null) { ++ if (bukkitEntity != null && !mcEnt.isToBeRemoved()) { // PaperSpigot + list.add(bukkitEntity); + } + } +@@ -0,0 +0,0 @@ public class CraftWorld implements World { + Entity bukkitEntity = mcEnt.getBukkitEntity(); + + // Assuming that bukkitEntity isn't null +- if (bukkitEntity != null && bukkitEntity instanceof LivingEntity) { ++ if (bukkitEntity != null && bukkitEntity instanceof LivingEntity && !mcEnt.isToBeRemoved()) { // PaperSpigot + list.add((LivingEntity) bukkitEntity); + } + } +@@ -0,0 +0,0 @@ public class CraftWorld implements World { + if (entity instanceof net.minecraft.server.Entity) { + Entity bukkitEntity = ((net.minecraft.server.Entity) entity).getBukkitEntity(); + +- if (bukkitEntity == null) { ++ if (bukkitEntity == null || ((net.minecraft.server.Entity) entity).isToBeRemoved()) { // PaperSpigot + continue; + } + +@@ -0,0 +0,0 @@ public class CraftWorld implements World { + if (entity instanceof net.minecraft.server.Entity) { + Entity bukkitEntity = ((net.minecraft.server.Entity) entity).getBukkitEntity(); + +- if (bukkitEntity == null) { ++ if (bukkitEntity == null || ((net.minecraft.server.Entity) entity).isToBeRemoved()) { // PaperSpigot + continue; + } + +diff --git a/src/main/java/org/github/paperspigot/OptimizedRemoveAll.java b/src/main/java/org/github/paperspigot/OptimizedRemoveAll.java +new file mode 100644 +index 0000000000000000000000000000000000000000..0000000000000000000000000000000000000000 +--- /dev/null ++++ b/src/main/java/org/github/paperspigot/OptimizedRemoveAll.java +@@ -0,0 +0,0 @@ ++package org.github.paperspigot; ++ ++import java.util.List; ++ ++/** ++ * Improved algorithim for bulk removing entries from a list ++ *

++ * WARNING: This system only works on Identity Based lists, ++ * unlike traditional .removeAll() that operates on object equality. ++ */ ++public final class OptimizedRemoveAll { ++ private OptimizedRemoveAll() { ++ } ++ ++ public interface Marker { ++ boolean isToBeRemoved(); ++ ++ void markToBeRemoved(); ++ } ++ ++ /** ++ * More effecient removeAll method ++ * ++ * @param tickPosition Previous Tick Position ++ * @return New Tick Position ++ */ ++ public static int removeAll(List list, List removal, int tickPosition) { ++ if (removal != null && !removal.isEmpty()) { ++ int removalSize = removal.size(); ++ for (int i = 0; i < removalSize; i++) { ++ removal.get(i).markToBeRemoved(); ++ } ++ } ++ ++ int size = list.size(); ++ int insertAt = 0; ++ for (int i = 0; i < size; i++) { ++ E el = list.get(i); ++ if (i == tickPosition) { ++ tickPosition = insertAt; ++ } ++ if (el != null && !el.isToBeRemoved()) { ++ list.set(insertAt++, el); ++ } ++ } ++ list.subList(insertAt, size).clear(); ++ return tickPosition; ++ } ++ ++} +-- \ No newline at end of file