PaperSpigot-Parent/Spigot-Server-Patches/0036-Player-lookup-improvements.patch

From a63a30097cb93decbd8735d557a4d32ece9c2451 Mon Sep 17 00:00:00 2001
From: Aikar <aikar@aikar.co>
Date: Fri, 28 Nov 2014 13:43:11 -0600
Subject: [PATCH] Player lookup improvements

Minecraft and CraftBukkit both use Arrays to store online players,
and any time a player needs to be looked up by name or UUID,
the system iterates all online players and does a name or UUID comparison.

This is very ineffecient and can reduce performance on servers with high player count.

By using a map based approach for player lookups, player lookup should
be consistent in performance regardless of how many players are online.

diff --git a/src/main/java/net/minecraft/server/PlayerList.java b/src/main/java/net/minecraft/server/PlayerList.java
index 9664482..a92a369 100644
--- a/src/main/java/net/minecraft/server/PlayerList.java
+++ b/src/main/java/net/minecraft/server/PlayerList.java
@@ -48,6 +48,31 @@ public abstract class PlayerList {
     private static final SimpleDateFormat i = new SimpleDateFormat("yyyy-MM-dd \'at\' HH:mm:ss z");
     private final MinecraftServer server;
     public final List players = new java.util.concurrent.CopyOnWriteArrayList(); // CraftBukkit - ArrayList -> CopyOnWriteArrayList: Iterator safety
+    // PaperSpigot start - Player lookup improvements
+    public final Map<String, EntityPlayer> playerMap = new java.util.HashMap<String, EntityPlayer>() {
+        @Override
+        public EntityPlayer put(String key, EntityPlayer value) {
+            return super.put(key.toLowerCase(), value);
+        }
+
+        @Override
+        public EntityPlayer get(Object key) {
+            // put the .playerConnection check done in other places here
+            EntityPlayer player = super.get(key instanceof String ? ((String) key).toLowerCase() : key);
+            return (player != null && player.playerConnection != null) ? player : null;
+        }
+
+        @Override
+        public boolean containsKey(Object key) {
+            return get(key) != null;
+        }
+
+        @Override
+        public EntityPlayer remove(Object key) {
+            return super.remove(key instanceof String ? ((String) key).toLowerCase() : key);
+        }
+    };
+    // PaperSpigot end
     public final Map f = Maps.newHashMap();
     private final GameProfileBanList k;
     private final IpBanList l;
@@ -253,6 +278,7 @@ public abstract class PlayerList {
 
     public void onPlayerJoin(EntityPlayer entityplayer) {
         this.players.add(entityplayer);
+        this.playerMap.put(entityplayer.getName(), entityplayer); // PaperSpigot
         this.f.put(entityplayer.getUniqueID(), entityplayer);
         // this.sendAll(new PacketPlayOutPlayerInfo(EnumPlayerInfoAction.ADD_PLAYER, new EntityPlayer[] { entityplayer})); // CraftBukkit - replaced with loop below
         WorldServer worldserver = this.server.getWorldServer(entityplayer.dimension);
@@ -324,6 +350,7 @@ public abstract class PlayerList {
         worldserver.kill(entityplayer);
         worldserver.getPlayerChunkMap().removePlayer(entityplayer);
         this.players.remove(entityplayer);
+        this.playerMap.remove(entityplayer.getName()); // PaperSpigot
         this.f.remove(entityplayer.getUniqueID());
         this.o.remove(entityplayer.getUniqueID());
         // CraftBukkit start
@@ -355,6 +382,8 @@ public abstract class PlayerList {
 
         EntityPlayer entityplayer;
 
+        // PaperSpigot - Use exact lookup below
+        /*
         for (int i = 0; i < this.players.size(); ++i) {
             entityplayer = (EntityPlayer) this.players.get(i);
             if (entityplayer.getUniqueID().equals(uuid)) {
@@ -366,6 +395,8 @@ public abstract class PlayerList {
 
         while (iterator.hasNext()) {
             entityplayer = (EntityPlayer) iterator.next();
+        */
+        if ((entityplayer = this.a(uuid)) != null) {
             savePlayerFile(entityplayer); // CraftBukkit - Force the player's inventory to be saved
             entityplayer.playerConnection.disconnect("You logged in from another location");
         }
@@ -957,6 +988,7 @@ public abstract class PlayerList {
     }
 
     public EntityPlayer getPlayer(String s) {
+        if (true) { return playerMap.get(s); } // PaperSpigot
         Iterator iterator = this.players.iterator();
 
         EntityPlayer entityplayer;
diff --git a/src/main/java/org/bukkit/craftbukkit/CraftOfflinePlayer.java b/src/main/java/org/bukkit/craftbukkit/CraftOfflinePlayer.java
index 8be0698..4a908c6 100644
--- a/src/main/java/org/bukkit/craftbukkit/CraftOfflinePlayer.java
+++ b/src/main/java/org/bukkit/craftbukkit/CraftOfflinePlayer.java
@@ -144,14 +144,10 @@ public class CraftOfflinePlayer implements OfflinePlayer, ConfigurationSerializa
     }
 
     public Player getPlayer() {
-        for (Object obj : server.getHandle().players) {
-            EntityPlayer player = (EntityPlayer) obj;
-            if (player.getUniqueID().equals(getUniqueId())) {
-                return (player.playerConnection != null) ? player.playerConnection.getPlayer() : null;
-            }
-        }
-
-        return null;
+        // PaperSpigot - Improved player lookup, replace entire method
+        final EntityPlayer playerEntity = server.getHandle().a(getUniqueId());
+        return playerEntity != null ? playerEntity.getBukkitEntity() : null;
+        // PaperSpigot end
     }
 
     @Override
diff --git a/src/main/java/org/bukkit/craftbukkit/CraftServer.java b/src/main/java/org/bukkit/craftbukkit/CraftServer.java
index 590fe28..54e595d 100644
--- a/src/main/java/org/bukkit/craftbukkit/CraftServer.java
+++ b/src/main/java/org/bukkit/craftbukkit/CraftServer.java
@@ -397,7 +397,12 @@ public final class CraftServer implements Server {
     public Player getPlayer(final String name) {
         Validate.notNull(name, "Name cannot be null");
 
-        Player found = null;
+        // PaperSpigot start - Improved player lookup changes
+        Player found = getPlayerExact(name);
+        if (found != null) {
+            return found;
+        }
+        // PaperSpigot end
         String lowerName = name.toLowerCase();
         int delta = Integer.MAX_VALUE;
         for (Player player : getOnlinePlayers()) {
@@ -418,15 +423,10 @@ public final class CraftServer implements Server {
     public Player getPlayerExact(String name) {
         Validate.notNull(name, "Name cannot be null");
 
-        String lname = name.toLowerCase();
-
-        for (Player player : getOnlinePlayers()) {
-            if (player.getName().equalsIgnoreCase(lname)) {
-                return player;
-            }
-        }
-
-        return null;
+        // PaperSpigot start - Improved player lookup, replace whole method
+        EntityPlayer player = playerList.playerMap.get(name);
+        return player != null ? player.getBukkitEntity() : null;
+         // PaperSpigot end
     }
 
     @Override
diff --git a/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java b/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java
index f07a3a8..b1646e5 100644
--- a/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java
+++ b/src/main/java/org/bukkit/craftbukkit/entity/CraftPlayer.java
@@ -103,13 +103,7 @@ public class CraftPlayer extends CraftHumanEntity implements Player {
     }
 
     public boolean isOnline() {
-        for (Object obj : server.getHandle().players) {
-            EntityPlayer player = (EntityPlayer) obj;
-            if (player.getName().equalsIgnoreCase(getName())) {
-                return true;
-            }
-        }
-        return false;
+        return server.getHandle().a(getUniqueId()) != null; // PaperSpigot - replace whole method
     }
 
     public InetSocketAddress getAddress() {
-- 
1.9.1