From 191777773c5da59e4752e86dfeed202517af4a19 Mon Sep 17 00:00:00 2001
From: DaPorkchop_ <daporkchop@daporkchop.net>
Date: Tue, 13 Oct 2020 16:44:47 +0200
Subject: [PATCH] Don't use wrapper objects for positions in ChunkCache (#1398)

* make ChunkPosition use a hashCode implementation with far better hash distribution

this should improve the performance when used as a hash table key

* ChunkCache no longer uses position wrapper objects

this yields a roughly 15-20% increase in performance when converting chunk data

* fix code style issues
---
 .../network/session/cache/ChunkCache.java     | 87 ++++++++++---------
 .../java/world/JavaUnloadChunkTranslator.java |  3 +-
 .../translators/world/GeyserWorldManager.java |  6 +-
 .../world/chunk/ChunkPosition.java            | 30 ++++++-
 .../geysermc/connector/utils/ChunkUtils.java  |  5 +-
 5 files changed, 79 insertions(+), 52 deletions(-)

diff --git a/connector/src/main/java/org/geysermc/connector/network/session/cache/ChunkCache.java b/connector/src/main/java/org/geysermc/connector/network/session/cache/ChunkCache.java
index 2cc9ea13..14825b71 100644
--- a/connector/src/main/java/org/geysermc/connector/network/session/cache/ChunkCache.java
+++ b/connector/src/main/java/org/geysermc/connector/network/session/cache/ChunkCache.java
@@ -27,22 +27,18 @@ package org.geysermc.connector.network.session.cache;
 
 import com.github.steveice10.mc.protocol.data.game.chunk.Chunk;
 import com.github.steveice10.mc.protocol.data.game.chunk.Column;
-import com.github.steveice10.mc.protocol.data.game.entity.metadata.Position;
-import lombok.Getter;
+import it.unimi.dsi.fastutil.longs.Long2ObjectMap;
+import it.unimi.dsi.fastutil.longs.Long2ObjectOpenHashMap;
 import org.geysermc.connector.bootstrap.GeyserBootstrap;
 import org.geysermc.connector.network.session.GeyserSession;
 import org.geysermc.connector.network.translators.world.block.BlockTranslator;
 import org.geysermc.connector.network.translators.world.chunk.ChunkPosition;
 
-import java.util.HashMap;
-import java.util.Map;
-
 public class ChunkCache {
 
     private final boolean cache;
 
-    @Getter
-    private final Map<ChunkPosition, Column> chunks = new HashMap<>();
+    private final Long2ObjectMap<Column> chunks = new Long2ObjectOpenHashMap<>();
 
     public ChunkCache(GeyserSession session) {
         if (session.getConnector().getWorldManager().getClass() == GeyserBootstrap.DEFAULT_CHUNK_MANAGER.getClass()) {
@@ -56,57 +52,66 @@ public class ChunkCache {
         if (!cache) {
             return;
         }
-        ChunkPosition position = new ChunkPosition(chunk.getX(), chunk.getZ());
-        if (chunk.getBiomeData() == null && chunks.containsKey(position)) {
-            Column newColumn = chunk;
-            chunk = chunks.get(position);
-            for (int i = 0; i < newColumn.getChunks().length; i++) {
-                if (newColumn.getChunks()[i] != null) {
-                    chunk.getChunks()[i] = newColumn.getChunks()[i];
+
+        long chunkPosition = ChunkPosition.toLong(chunk.getX(), chunk.getZ());
+        Column existingChunk;
+        if (chunk.getBiomeData() != null // Only consider merging columns if the new chunk isn't a full chunk
+            && (existingChunk = chunks.getOrDefault(chunkPosition, null)) != null) { // Column is already present in cache, we can merge with existing
+            for (int i = 0; i < chunk.getChunks().length; i++) { // The chunks member is final, so chunk.getChunks() will probably be inlined and then completely optimized away
+                if (chunk.getChunks()[i] != null) {
+                    existingChunk.getChunks()[i] = chunk.getChunks()[i];
                 }
             }
-        }
-        chunks.put(position, chunk);
-    }
-
-    public void updateBlock(Position position, int block) {
-        if (!cache) {
-            return;
-        }
-        ChunkPosition chunkPosition = new ChunkPosition(position.getX() >> 4, position.getZ() >> 4);
-        if (!chunks.containsKey(chunkPosition))
-            return;
-
-        Column column = chunks.get(chunkPosition);
-        Chunk chunk = column.getChunks()[position.getY() >> 4];
-        Position blockPosition = chunkPosition.getChunkBlock(position.getX(), position.getY(), position.getZ());
-        if (chunk != null) {
-            chunk.set(blockPosition.getX(), blockPosition.getY(), blockPosition.getZ(), block);
+        } else {
+            chunks.put(chunkPosition, chunk);
         }
     }
 
-    public int getBlockAt(Position position) {
+    public Column getChunk(int chunkX, int chunkZ)  {
+        long chunkPosition = ChunkPosition.toLong(chunkX, chunkZ);
+        return chunks.getOrDefault(chunkPosition, null);
+    }
+
+    public void updateBlock(int x, int y, int z, int block) {
+        if (!cache) {
+            return;
+        }
+
+        Column column = this.getChunk(x >> 4, z >> 4);
+        if (column == null) {
+            return;
+        }
+
+        Chunk chunk = column.getChunks()[y >> 4];
+        if (chunk != null) {
+            chunk.set(x & 0xF, y & 0xF, z & 0xF, block);
+        }
+    }
+
+    public int getBlockAt(int x, int y, int z) {
         if (!cache) {
             return BlockTranslator.AIR;
         }
-        ChunkPosition chunkPosition = new ChunkPosition(position.getX() >> 4, position.getZ() >> 4);
-        if (!chunks.containsKey(chunkPosition))
-            return BlockTranslator.AIR;
 
-        Column column = chunks.get(chunkPosition);
-        Chunk chunk = column.getChunks()[position.getY() >> 4];
-        Position blockPosition = chunkPosition.getChunkBlock(position.getX(), position.getY(), position.getZ());
+        Column column = this.getChunk(x >> 4, z >> 4);
+        if (column == null) {
+            return BlockTranslator.AIR;
+        }
+
+        Chunk chunk = column.getChunks()[y >> 4];
         if (chunk != null) {
-            return chunk.get(blockPosition.getX(), blockPosition.getY(), blockPosition.getZ());
+            return chunk.get(x & 0xF, y & 0xF, z & 0xF);
         }
 
         return BlockTranslator.AIR;
     }
 
-    public void removeChunk(ChunkPosition position) {
+    public void removeChunk(int chunkX, int chunkZ) {
         if (!cache) {
             return;
         }
-        chunks.remove(position);
+
+        long chunkPosition = ChunkPosition.toLong(chunkX, chunkZ);
+        chunks.remove(chunkPosition);
     }
 }
diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/java/world/JavaUnloadChunkTranslator.java b/connector/src/main/java/org/geysermc/connector/network/translators/java/world/JavaUnloadChunkTranslator.java
index d54d8b6a..652e1294 100644
--- a/connector/src/main/java/org/geysermc/connector/network/translators/java/world/JavaUnloadChunkTranslator.java
+++ b/connector/src/main/java/org/geysermc/connector/network/translators/java/world/JavaUnloadChunkTranslator.java
@@ -28,7 +28,6 @@ package org.geysermc.connector.network.translators.java.world;
 import org.geysermc.connector.network.session.GeyserSession;
 import org.geysermc.connector.network.translators.PacketTranslator;
 import org.geysermc.connector.network.translators.Translator;
-import org.geysermc.connector.network.translators.world.chunk.ChunkPosition;
 
 import com.github.steveice10.mc.protocol.packet.ingame.server.world.ServerUnloadChunkPacket;
 
@@ -37,6 +36,6 @@ public class JavaUnloadChunkTranslator extends PacketTranslator<ServerUnloadChun
 
     @Override
     public void translate(ServerUnloadChunkPacket packet, GeyserSession session) {
-        session.getChunkCache().removeChunk(new ChunkPosition(packet.getX(), packet.getZ()));
+        session.getChunkCache().removeChunk(packet.getX(), packet.getZ());
     }
 }
diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/world/GeyserWorldManager.java b/connector/src/main/java/org/geysermc/connector/network/translators/world/GeyserWorldManager.java
index 46aa6f7f..983e42b7 100644
--- a/connector/src/main/java/org/geysermc/connector/network/translators/world/GeyserWorldManager.java
+++ b/connector/src/main/java/org/geysermc/connector/network/translators/world/GeyserWorldManager.java
@@ -25,14 +25,12 @@
 
 package org.geysermc.connector.network.translators.world;
 
-import com.github.steveice10.mc.protocol.data.game.entity.metadata.Position;
 import com.github.steveice10.mc.protocol.data.game.entity.player.GameMode;
 import com.github.steveice10.mc.protocol.data.game.setting.Difficulty;
 import com.github.steveice10.mc.protocol.packet.ingame.client.ClientChatPacket;
 import it.unimi.dsi.fastutil.objects.Object2ObjectMap;
 import it.unimi.dsi.fastutil.objects.Object2ObjectOpenHashMap;
 import org.geysermc.connector.network.session.GeyserSession;
-import org.geysermc.connector.network.translators.world.chunk.ChunkPosition;
 import org.geysermc.connector.utils.GameRule;
 
 public class GeyserWorldManager extends WorldManager {
@@ -41,14 +39,14 @@ public class GeyserWorldManager extends WorldManager {
 
     @Override
     public int getBlockAt(GeyserSession session, int x, int y, int z) {
-        return session.getChunkCache().getBlockAt(new Position(x, y, z));
+        return session.getChunkCache().getBlockAt(x, y, z);
     }
 
     @Override
     public int[] getBiomeDataAt(GeyserSession session, int x, int z) {
         if (!session.getConnector().getConfig().isCacheChunks())
             return new int[1024];
-        return session.getChunkCache().getChunks().get(new ChunkPosition(x, z)).getBiomeData();
+        return session.getChunkCache().getChunk(x, z).getBiomeData();
     }
 
     @Override
diff --git a/connector/src/main/java/org/geysermc/connector/network/translators/world/chunk/ChunkPosition.java b/connector/src/main/java/org/geysermc/connector/network/translators/world/chunk/ChunkPosition.java
index a32d9316..9e721aa9 100644
--- a/connector/src/main/java/org/geysermc/connector/network/translators/world/chunk/ChunkPosition.java
+++ b/connector/src/main/java/org/geysermc/connector/network/translators/world/chunk/ChunkPosition.java
@@ -27,16 +27,25 @@ package org.geysermc.connector.network.translators.world.chunk;
 
 import com.github.steveice10.mc.protocol.data.game.entity.metadata.Position;
 import lombok.AllArgsConstructor;
-import lombok.EqualsAndHashCode;
 import lombok.Getter;
 import lombok.Setter;
 
 @Getter
 @Setter
 @AllArgsConstructor
-@EqualsAndHashCode
 public class ChunkPosition {
 
+    /**
+     * Packs a chunk's X and Z coordinates into a single {@code long}.
+     *
+     * @param x the X coordinate
+     * @param z the Z coordinate
+     * @return the packed coordinates
+     */
+    public static long toLong(int x, int z) {
+        return ((x & 0xFFFFFFFFL) << 32L) | (z & 0xFFFFFFFFL);
+    }
+
     private int x;
     private int z;
 
@@ -50,4 +59,21 @@ public class ChunkPosition {
         int chunkZ = z & 15;
         return new Position(chunkX, chunkY, chunkZ);
     }
+
+    @Override
+    public boolean equals(Object obj) {
+        if (obj == this)    {
+            return true;
+        } else if (obj instanceof ChunkPosition) {
+            ChunkPosition chunkPosition = (ChunkPosition) obj;
+            return this.x == chunkPosition.x && this.z == chunkPosition.z;
+        } else {
+            return false;
+        }
+    }
+
+    @Override
+    public int hashCode() {
+        return this.x * 2061811133 + this.z * 1424368303;
+    }
 }
diff --git a/connector/src/main/java/org/geysermc/connector/utils/ChunkUtils.java b/connector/src/main/java/org/geysermc/connector/utils/ChunkUtils.java
index 9413bbba..d47584e9 100644
--- a/connector/src/main/java/org/geysermc/connector/utils/ChunkUtils.java
+++ b/connector/src/main/java/org/geysermc/connector/utils/ChunkUtils.java
@@ -112,8 +112,7 @@ public class ChunkUtils {
                         int blockState;
                         // If a non-full chunk, then grab the block that should be here to create a 'full' chunk
                         if (chunk == null) {
-                            Position pos = new ChunkPosition(column.getX(), column.getZ()).getBlock(x, (chunkY << 4) + y, z);
-                            blockState = session.getConnector().getWorldManager().getBlockAt(session, pos.getX(), pos.getY(), pos.getZ());
+                            blockState = session.getConnector().getWorldManager().getBlockAt(session, (column.getX() << 4) + x, (chunkY << 4) + y, (column.getZ() << 4) + z);
                         } else {
                             blockState = chunk.get(x, y, z);
                         }
@@ -250,7 +249,7 @@ public class ChunkUtils {
                 break; //No block will be a part of two classes
             }
         }
-        session.getChunkCache().updateBlock(new Position(position.getX(), position.getY(), position.getZ()), blockState);
+        session.getChunkCache().updateBlock(position.getX(), position.getY(), position.getZ(), blockState);
     }
 
     public static void sendEmptyChunks(GeyserSession session, Vector3i position, int radius, boolean forceUpdate) {