Better chunk packet caching

src/main/java/net/minestom/server/instance/Chunk.java

@@ -13,18 +13,18 @@ import net.minestom.server.instance.block.Block;
 import net.minestom.server.instance.block.BlockGetter;
 import net.minestom.server.instance.block.BlockSetter;
 import net.minestom.server.network.packet.server.play.ChunkDataPacket;
-import net.minestom.server.network.packet.server.play.UpdateLightPacket;
-import net.minestom.server.network.player.PlayerConnection;
 import net.minestom.server.tag.Tag;
 import net.minestom.server.tag.TagHandler;
-import net.minestom.server.utils.ArrayUtils;
 import net.minestom.server.utils.chunk.ChunkSupplier;
 import net.minestom.server.world.biomes.Biome;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.jglrxavpok.hephaistos.nbt.NBTCompound;
 
-import java.util.*;
+import java.util.Collections;
+import java.util.Map;
+import java.util.Set;
+import java.util.UUID;
 import java.util.concurrent.ConcurrentHashMap;
 
 // TODO light data & API
@@ -126,11 +126,13 @@ public abstract class Chunk implements BlockGetter, BlockSetter, Viewable, Ticka
     public abstract long getLastChangeTime();
 
     /**
-     * Creates a {@link ChunkDataPacket} with this chunk data ready to be written.
+     * Sends the chunk data to {@code player}.
      *
-     * @return a new chunk data packet
+     * @param player the player
      */
-    public abstract @NotNull ChunkDataPacket createChunkPacket();
+    public abstract void sendChunk(@NotNull Player player);
+
+    public abstract void sendChunk();
 
     /**
      * Creates a copy of this chunk, including blocks state id, custom block id, biomes, update data.
@@ -152,7 +154,7 @@ public abstract class Chunk implements BlockGetter, BlockSetter, Viewable, Ticka
     /**
      * Gets the unique identifier of this chunk.
      * <p>
-     * WARNING: this UUID is not persistent but randomized once the object is instantiate.
+     * WARNING: this UUID is not persistent but randomized once the object is instantiated.
      *
      * @return the chunk identifier
      */
@@ -244,50 +246,6 @@ public abstract class Chunk implements BlockGetter, BlockSetter, Viewable, Ticka
         this.columnarSpace = columnarSpace;
     }
 
-    /**
-     * Gets the light packet of this chunk.
-     *
-     * @return the light packet
-     */
-    @NotNull
-    public UpdateLightPacket getLightPacket() {
-        long skyMask = 0;
-        long blockMask = 0;
-        List<byte[]> skyLights = new ArrayList<>();
-        List<byte[]> blockLights = new ArrayList<>();
-
-        UpdateLightPacket updateLightPacket = new UpdateLightPacket();
-        updateLightPacket.chunkX = getChunkX();
-        updateLightPacket.chunkZ = getChunkZ();
-
-        updateLightPacket.skyLight = skyLights;
-        updateLightPacket.blockLight = blockLights;
-
-        final var sections = getSections();
-        for (var entry : sections.entrySet()) {
-            final int index = entry.getKey() + 1;
-            final Section section = entry.getValue();
-
-            final var skyLight = section.getSkyLight();
-            final var blockLight = section.getBlockLight();
-
-            if (!ArrayUtils.empty(skyLight)) {
-                skyLights.add(skyLight);
-                skyMask |= 1L << index;
-            }
-            if (!ArrayUtils.empty(blockLight)) {
-                blockLights.add(blockLight);
-                blockMask |= 1L << index;
-            }
-        }
-
-        updateLightPacket.skyLightMask = new long[]{skyMask};
-        updateLightPacket.blockLightMask = new long[]{blockMask};
-        updateLightPacket.emptySkyLightMask = new long[0];
-        updateLightPacket.emptyBlockLightMask = new long[0];
-        return updateLightPacket;
-    }
-
     /**
      * Used to verify if the chunk should still be kept in memory.
      *
@@ -365,28 +323,6 @@ public abstract class Chunk implements BlockGetter, BlockSetter, Viewable, Ticka
         tag.write(nbt, value);
     }
 
-    /**
-     * Sends the chunk data to {@code player}.
-     *
-     * @param player the player
-     */
-    public synchronized void sendChunk(@NotNull Player player) {
-        // Only send loaded chunk
-        if (!isLoaded())
-            return;
-        final PlayerConnection playerConnection = player.getPlayerConnection();
-        playerConnection.sendPacket(getLightPacket());
-        playerConnection.sendPacket(createChunkPacket());
-    }
-
-    public synchronized void sendChunk() {
-        if (!isLoaded()) {
-            return;
-        }
-        sendPacketToViewers(getLightPacket());
-        sendPacketToViewers(createChunkPacket());
-    }
-
     /**
      * Sets the chunk as "unloaded".
      */

src/main/java/net/minestom/server/instance/DynamicChunk.java

@@ -4,16 +4,24 @@ import com.extollit.gaming.ai.path.model.ColumnarOcclusionFieldList;
 import it.unimi.dsi.fastutil.ints.Int2ObjectAVLTreeMap;
 import it.unimi.dsi.fastutil.ints.Int2ObjectOpenHashMap;
 import net.minestom.server.coordinate.Vec;
+import net.minestom.server.entity.Player;
 import net.minestom.server.entity.pathfinding.PFBlock;
 import net.minestom.server.instance.block.Block;
 import net.minestom.server.instance.block.BlockHandler;
 import net.minestom.server.network.packet.server.play.ChunkDataPacket;
+import net.minestom.server.network.packet.server.play.UpdateLightPacket;
+import net.minestom.server.network.player.NettyPlayerConnection;
+import net.minestom.server.network.player.PlayerConnection;
+import net.minestom.server.utils.ArrayUtils;
+import net.minestom.server.utils.PacketUtils;
 import net.minestom.server.utils.chunk.ChunkUtils;
 import net.minestom.server.world.biomes.Biome;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 
-import java.lang.ref.SoftReference;
+import java.nio.ByteBuffer;
+import java.util.ArrayList;
+import java.util.List;
 import java.util.Map;
 import java.util.Objects;
 
@@ -30,9 +38,10 @@ public class DynamicChunk extends Chunk {
     protected final Int2ObjectOpenHashMap<Block> entries = new Int2ObjectOpenHashMap<>();
     protected final Int2ObjectOpenHashMap<Block> tickableMap = new Int2ObjectOpenHashMap<>();
 
-    private long lastChangeTime;
+    private volatile long lastChangeTime;
 
-    private SoftReference<ChunkDataPacket> cachedPacket = new SoftReference<>(null);
+    private ByteBuffer cachedChunkBuffer;
+    private ByteBuffer cachedLightBuffer;
     private long cachedPacketTime;
 
     public DynamicChunk(@NotNull Instance instance, @Nullable Biome[] biomes, int chunkX, int chunkZ) {
@@ -117,23 +126,32 @@ public class DynamicChunk extends Chunk {
         return lastChangeTime;
     }
 
-    @NotNull
     @Override
-    public ChunkDataPacket createChunkPacket() {
+    public synchronized void sendChunk(@NotNull Player player) {
-        ChunkDataPacket packet = cachedPacket.get();
+        if (!isLoaded()) return;
-        if (packet != null && cachedPacketTime == getLastChangeTime()) {
+        final PlayerConnection connection = player.getPlayerConnection();
-            return packet;
+        if (connection instanceof NettyPlayerConnection) {
+            final long lastChange = getLastChangeTime();
+            if (lastChange > cachedPacketTime ||
+                    (cachedChunkBuffer == null || cachedLightBuffer == null)) {
+                this.cachedChunkBuffer = PacketUtils.createFramedPacket(ByteBuffer.allocate(65000), createChunkPacket());
+                this.cachedLightBuffer = PacketUtils.createFramedPacket(ByteBuffer.allocate(65000), createLightPacket());
+                this.cachedPacketTime = lastChange;
+            }
+            NettyPlayerConnection nettyPlayerConnection = (NettyPlayerConnection) connection;
+            nettyPlayerConnection.write(cachedChunkBuffer);
+            nettyPlayerConnection.write(cachedLightBuffer);
+        } else {
+            connection.sendPacket(createLightPacket());
+            connection.sendPacket(createChunkPacket());
         }
-        packet = new ChunkDataPacket();
+    }
-        packet.biomes = biomes;
-        packet.chunkX = chunkX;
-        packet.chunkZ = chunkZ;
-        packet.sections = sectionMap.clone(); // TODO deep clone
-        packet.entries = entries.clone();
 
-        this.cachedPacketTime = getLastChangeTime();
+    @Override
-        this.cachedPacket = new SoftReference<>(packet);
+    public synchronized void sendChunk() {
-        return packet;
+        if (!isLoaded()) return;
+        sendPacketToViewers(createLightPacket());
+        sendPacketToViewers(createChunkPacket());
     }
 
     @NotNull
@@ -153,6 +171,54 @@ public class DynamicChunk extends Chunk {
         this.entries.clear();
     }
 
+    private @NotNull ChunkDataPacket createChunkPacket() {
+        ChunkDataPacket packet = new ChunkDataPacket();
+        packet.biomes = biomes;
+        packet.chunkX = chunkX;
+        packet.chunkZ = chunkZ;
+        packet.sections = sectionMap.clone(); // TODO deep clone
+        packet.entries = entries.clone();
+        return packet;
+    }
+
+    private @NotNull UpdateLightPacket createLightPacket() {
+        long skyMask = 0;
+        long blockMask = 0;
+        List<byte[]> skyLights = new ArrayList<>();
+        List<byte[]> blockLights = new ArrayList<>();
+
+        UpdateLightPacket updateLightPacket = new UpdateLightPacket();
+        updateLightPacket.chunkX = getChunkX();
+        updateLightPacket.chunkZ = getChunkZ();
+
+        updateLightPacket.skyLight = skyLights;
+        updateLightPacket.blockLight = blockLights;
+
+        final var sections = getSections();
+        for (var entry : sections.entrySet()) {
+            final int index = entry.getKey() + 1;
+            final Section section = entry.getValue();
+
+            final var skyLight = section.getSkyLight();
+            final var blockLight = section.getBlockLight();
+
+            if (!ArrayUtils.empty(skyLight)) {
+                skyLights.add(skyLight);
+                skyMask |= 1L << index;
+            }
+            if (!ArrayUtils.empty(blockLight)) {
+                blockLights.add(blockLight);
+                blockMask |= 1L << index;
+            }
+        }
+
+        updateLightPacket.skyLightMask = new long[]{skyMask};
+        updateLightPacket.blockLightMask = new long[]{blockMask};
+        updateLightPacket.emptySkyLightMask = new long[0];
+        updateLightPacket.emptyBlockLightMask = new long[0];
+        return updateLightPacket;
+    }
+
     private @Nullable Section getOptionalSection(int y) {
         final int sectionIndex = ChunkUtils.getSectionAt(y);
         return sectionMap.get(sectionIndex);

src/main/java/net/minestom/server/instance/batch/ChunkBatch.java

@@ -8,8 +8,6 @@ import net.minestom.server.instance.Chunk;
 import net.minestom.server.instance.Instance;
 import net.minestom.server.instance.InstanceContainer;
 import net.minestom.server.instance.block.Block;
-import net.minestom.server.network.packet.server.play.ChunkDataPacket;
-import net.minestom.server.utils.PacketUtils;
 import net.minestom.server.utils.callback.OptionalCallback;
 import net.minestom.server.utils.chunk.ChunkCallback;
 import net.minestom.server.utils.chunk.ChunkUtils;
@@ -225,9 +223,8 @@ public class ChunkBatch implements Batch<ChunkCallback> {
     private void updateChunk(@NotNull Instance instance, Chunk chunk, IntSet updatedSections, @Nullable ChunkCallback callback, boolean safeCallback) {
         // Refresh chunk for viewers
         if (options.shouldSendUpdate()) {
-            ChunkDataPacket chunkDataPacket = chunk.createChunkPacket();
             // TODO update all sections from `updatedSections`
-            PacketUtils.sendGroupedPacket(chunk.getViewers(), chunkDataPacket);
+            chunk.sendChunk();
         }
 
         if (instance instanceof InstanceContainer) {

src/main/java/net/minestom/server/utils/PacketUtils.java

@@ -16,7 +16,6 @@ import net.minestom.server.network.socket.Server;
 import net.minestom.server.utils.binary.BinaryWriter;
 import net.minestom.server.utils.callback.validator.PlayerValidator;
 import org.jetbrains.annotations.NotNull;
-import org.jetbrains.annotations.Nullable;
 
 import java.nio.BufferOverflowException;
 import java.nio.ByteBuffer;
@@ -78,43 +77,36 @@ public final class PacketUtils {
      * @param playerValidator optional callback to check if a specify player of {@code players} should receive the packet
      */
     public static void sendGroupedPacket(@NotNull Collection<Player> players, @NotNull ServerPacket packet,
-                                         @Nullable PlayerValidator playerValidator) {
+                                         @NotNull PlayerValidator playerValidator) {
         if (players.isEmpty())
             return;
-
         // work out if the packet needs to be sent individually due to server-side translating
         boolean needsTranslating = false;
         if (MinestomAdventure.AUTOMATIC_COMPONENT_TRANSLATION && packet instanceof ComponentHoldingServerPacket) {
             needsTranslating = ComponentUtils.areAnyTranslatable(((ComponentHoldingServerPacket) packet).components());
         }
-
         if (MinecraftServer.hasGroupedPacket() && !needsTranslating) {
             // Send grouped packet...
-            final boolean success = PACKET_LISTENER_MANAGER.processServerPacket(packet, players);
+            if (!PACKET_LISTENER_MANAGER.processServerPacket(packet, players))
-            if (success) {
+                return;
-                ByteBuffer finalBuffer = createFramedPacket(packet);
+            final ByteBuffer finalBuffer = createFramedPacket(packet);
-                final FramedPacket framedPacket = new FramedPacket(packet.getId(), finalBuffer);
+            final FramedPacket framedPacket = new FramedPacket(packet.getId(), finalBuffer);
-                // Send packet to all players
+            // Send packet to all players
-                for (Player player : players) {
+            for (Player player : players) {
-                    if (!player.isOnline())
+                if (!player.isOnline() || !playerValidator.isValid(player))
-                        continue;
+                    continue;
-                    // Verify if the player should receive the packet
+                final PlayerConnection connection = player.getPlayerConnection();
-                    if (playerValidator != null && !playerValidator.isValid(player))
+                if (connection instanceof NettyPlayerConnection) {
-                        continue;
+                    ((NettyPlayerConnection) connection).write(framedPacket);
-                    final PlayerConnection playerConnection = player.getPlayerConnection();
+                } else {
-                    if (playerConnection instanceof NettyPlayerConnection) {
+                    connection.sendPacket(packet);
-                        ((NettyPlayerConnection) playerConnection).write(framedPacket);
-                    } else {
-                        playerConnection.sendPacket(packet);
-                    }
                 }
-                finalBuffer.clear(); // Clear packet to be reused
             }
+            finalBuffer.clear(); // Clear packet to be reused
         } else {
             // Write the same packet for each individual players
             for (Player player : players) {
-                // Verify if the player should receive the packet
+                if (!player.isOnline() || !playerValidator.isValid(player))
-                if (playerValidator != null && !playerValidator.isValid(player))
                     continue;
                 player.getPlayerConnection().sendPacket(packet, false);
             }
@@ -128,7 +120,7 @@ public final class PacketUtils {
      * @see #sendGroupedPacket(Collection, ServerPacket, PlayerValidator)
      */
     public static void sendGroupedPacket(@NotNull Collection<Player> players, @NotNull ServerPacket packet) {
-        sendGroupedPacket(players, packet, null);
+        sendGroupedPacket(players, packet, player -> true);
     }
 
     public static void writeFramedPacket(@NotNull ByteBuffer buffer,
@@ -172,16 +164,21 @@ public final class PacketUtils {
         }
     }
 
-    public static ByteBuffer createFramedPacket(@NotNull ServerPacket packet) {
+    public static ByteBuffer createFramedPacket(@NotNull ByteBuffer initial, @NotNull ServerPacket packet) {
-        var buffer = BUFFER.get();
+        final boolean compression = MinecraftServer.getCompressionThreshold() > 0;
+        var buffer = initial;
         try {
-            writeFramedPacket(buffer, packet, MinecraftServer.getCompressionThreshold() > 0);
+            writeFramedPacket(buffer, packet, compression);
         } catch (BufferOverflowException e) {
             // In the unlikely case where the packet is bigger than the default buffer size,
             // increase to the highest authorized buffer size using heap (for cheap allocation)
             buffer = ByteBuffer.allocate(Server.MAX_PACKET_SIZE);
-            writeFramedPacket(buffer, packet, MinecraftServer.getCompressionThreshold() > 0);
+            writeFramedPacket(buffer, packet, compression);
         }
         return buffer;
     }
+
+    public static ByteBuffer createFramedPacket(@NotNull ServerPacket packet) {
+        return createFramedPacket(BUFFER.get(), packet);
+    }
 }

src/main/java/net/minestom/server/utils/binary/BitmaskUtil.java

@@ -1,9 +0,0 @@
-package net.minestom.server.utils.binary;
-
-public final class BitmaskUtil {
-
-    public static byte changeBit(byte value, byte mask, byte replacement, byte shift) {
-        return (byte) (value & ~mask | (replacement << shift));
-    }
-
-}