From 9b9d3f34057a710c5330f9225ea5923a7c54374a Mon Sep 17 00:00:00 2001 From: TheMode Date: Wed, 4 Aug 2021 16:49:01 +0200 Subject: [PATCH] Better chunk packet caching --- .../net/minestom/server/instance/Chunk.java | 84 ++------------- .../server/instance/DynamicChunk.java | 100 +++++++++++++++--- .../server/instance/batch/ChunkBatch.java | 5 +- .../minestom/server/utils/PacketUtils.java | 55 +++++----- .../server/utils/binary/BitmaskUtil.java | 9 -- 5 files changed, 120 insertions(+), 133 deletions(-) delete mode 100644 src/main/java/net/minestom/server/utils/binary/BitmaskUtil.java diff --git a/src/main/java/net/minestom/server/instance/Chunk.java b/src/main/java/net/minestom/server/instance/Chunk.java index dec32ac4d..d5121a09b 100644 --- a/src/main/java/net/minestom/server/instance/Chunk.java +++ b/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. *

- * 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 skyLights = new ArrayList<>(); - List 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". */ diff --git a/src/main/java/net/minestom/server/instance/DynamicChunk.java b/src/main/java/net/minestom/server/instance/DynamicChunk.java index d44fe308a..72812c589 100644 --- a/src/main/java/net/minestom/server/instance/DynamicChunk.java +++ b/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 entries = new Int2ObjectOpenHashMap<>(); protected final Int2ObjectOpenHashMap tickableMap = new Int2ObjectOpenHashMap<>(); - private long lastChangeTime; + private volatile long lastChangeTime; - private SoftReference 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() { - ChunkDataPacket packet = cachedPacket.get(); - if (packet != null && cachedPacketTime == getLastChangeTime()) { - return packet; + public synchronized void sendChunk(@NotNull Player player) { + if (!isLoaded()) return; + final PlayerConnection connection = player.getPlayerConnection(); + 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(); - this.cachedPacket = new SoftReference<>(packet); - return packet; + @Override + public synchronized void sendChunk() { + 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 skyLights = new ArrayList<>(); + List 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); diff --git a/src/main/java/net/minestom/server/instance/batch/ChunkBatch.java b/src/main/java/net/minestom/server/instance/batch/ChunkBatch.java index ce50369e7..5c47a7779 100644 --- a/src/main/java/net/minestom/server/instance/batch/ChunkBatch.java +++ b/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 { 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) { diff --git a/src/main/java/net/minestom/server/utils/PacketUtils.java b/src/main/java/net/minestom/server/utils/PacketUtils.java index 758416538..b45c03211 100644 --- a/src/main/java/net/minestom/server/utils/PacketUtils.java +++ b/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 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 (success) { - ByteBuffer finalBuffer = createFramedPacket(packet); - final FramedPacket framedPacket = new FramedPacket(packet.getId(), finalBuffer); - // Send packet to all players - for (Player player : players) { - if (!player.isOnline()) - continue; - // Verify if the player should receive the packet - if (playerValidator != null && !playerValidator.isValid(player)) - continue; - final PlayerConnection playerConnection = player.getPlayerConnection(); - if (playerConnection instanceof NettyPlayerConnection) { - ((NettyPlayerConnection) playerConnection).write(framedPacket); - } else { - playerConnection.sendPacket(packet); - } + if (!PACKET_LISTENER_MANAGER.processServerPacket(packet, players)) + return; + final ByteBuffer finalBuffer = createFramedPacket(packet); + final FramedPacket framedPacket = new FramedPacket(packet.getId(), finalBuffer); + // Send packet to all players + for (Player player : players) { + if (!player.isOnline() || !playerValidator.isValid(player)) + continue; + final PlayerConnection connection = player.getPlayerConnection(); + if (connection instanceof NettyPlayerConnection) { + ((NettyPlayerConnection) connection).write(framedPacket); + } else { + connection.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 (playerValidator != null && !playerValidator.isValid(player)) + if (!player.isOnline() || !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 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) { - var buffer = BUFFER.get(); + public static ByteBuffer createFramedPacket(@NotNull ByteBuffer initial, @NotNull ServerPacket packet) { + 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); + } } diff --git a/src/main/java/net/minestom/server/utils/binary/BitmaskUtil.java b/src/main/java/net/minestom/server/utils/binary/BitmaskUtil.java deleted file mode 100644 index a7813ebd6..000000000 --- a/src/main/java/net/minestom/server/utils/binary/BitmaskUtil.java +++ /dev/null @@ -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)); - } - -}