From 37a4199d4338e859b23bec7aed0f54930d815dfe Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Thu, 6 May 2021 19:56:45 -0400 Subject: [PATCH 1/6] Combine VarInt prefix encoding with compression This saves us a memory copy in the common "there is no need to compress this packet" case. --- .../proxy/connection/MinecraftConnection.java | 10 ++++---- ... MinecraftCompressorAndLengthEncoder.java} | 24 +++++++++++++++---- 2 files changed, 26 insertions(+), 8 deletions(-) rename proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/{MinecraftCompressEncoder.java => MinecraftCompressorAndLengthEncoder.java} (76%) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java b/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java index 4d631896..ba172db4 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java @@ -36,12 +36,13 @@ import com.velocitypowered.proxy.VelocityServer; import com.velocitypowered.proxy.connection.client.HandshakeSessionHandler; import com.velocitypowered.proxy.connection.client.LoginSessionHandler; import com.velocitypowered.proxy.connection.client.StatusSessionHandler; +import com.velocitypowered.proxy.network.Connections; import com.velocitypowered.proxy.protocol.MinecraftPacket; import com.velocitypowered.proxy.protocol.StateRegistry; import com.velocitypowered.proxy.protocol.netty.MinecraftCipherDecoder; import com.velocitypowered.proxy.protocol.netty.MinecraftCipherEncoder; import com.velocitypowered.proxy.protocol.netty.MinecraftCompressDecoder; -import com.velocitypowered.proxy.protocol.netty.MinecraftCompressEncoder; +import com.velocitypowered.proxy.protocol.netty.MinecraftCompressorAndLengthEncoder; import com.velocitypowered.proxy.protocol.netty.MinecraftDecoder; import com.velocitypowered.proxy.protocol.netty.MinecraftEncoder; import com.velocitypowered.proxy.util.except.QuietDecoderException; @@ -402,8 +403,8 @@ public class MinecraftConnection extends ChannelInboundHandlerAdapter { } else { MinecraftCompressDecoder decoder = (MinecraftCompressDecoder) channel.pipeline() .get(COMPRESSION_DECODER); - MinecraftCompressEncoder encoder = (MinecraftCompressEncoder) channel.pipeline() - .get(COMPRESSION_ENCODER); + MinecraftCompressorAndLengthEncoder encoder = + (MinecraftCompressorAndLengthEncoder) channel.pipeline().get(COMPRESSION_ENCODER); if (decoder != null && encoder != null) { decoder.setThreshold(threshold); encoder.setThreshold(threshold); @@ -411,9 +412,10 @@ public class MinecraftConnection extends ChannelInboundHandlerAdapter { int level = server.getConfiguration().getCompressionLevel(); VelocityCompressor compressor = Natives.compress.get().create(level); - encoder = new MinecraftCompressEncoder(threshold, compressor); + encoder = new MinecraftCompressorAndLengthEncoder(threshold, compressor); decoder = new MinecraftCompressDecoder(threshold, compressor); + channel.pipeline().remove(FRAME_ENCODER); channel.pipeline().addBefore(MINECRAFT_DECODER, COMPRESSION_DECODER, decoder); channel.pipeline().addBefore(MINECRAFT_ENCODER, COMPRESSION_ENCODER, encoder); } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressEncoder.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressorAndLengthEncoder.java similarity index 76% rename from proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressEncoder.java rename to proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressorAndLengthEncoder.java index f6cb495c..6684ee35 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressEncoder.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressorAndLengthEncoder.java @@ -23,13 +23,14 @@ import com.velocitypowered.proxy.protocol.ProtocolUtils; import io.netty.buffer.ByteBuf; import io.netty.channel.ChannelHandlerContext; import io.netty.handler.codec.MessageToByteEncoder; +import java.util.zip.DataFormatException; -public class MinecraftCompressEncoder extends MessageToByteEncoder { +public class MinecraftCompressorAndLengthEncoder extends MessageToByteEncoder { private int threshold; private final VelocityCompressor compressor; - public MinecraftCompressEncoder(int threshold, VelocityCompressor compressor) { + public MinecraftCompressorAndLengthEncoder(int threshold, VelocityCompressor compressor) { this.threshold = threshold; this.compressor = compressor; } @@ -39,16 +40,31 @@ public class MinecraftCompressEncoder extends MessageToByteEncoder { int uncompressed = msg.readableBytes(); if (uncompressed < threshold) { // Under the threshold, there is nothing to do. + ProtocolUtils.writeVarInt(out, uncompressed + 1); ProtocolUtils.writeVarInt(out, 0); out.writeBytes(msg); } else { - ProtocolUtils.writeVarInt(out, uncompressed); + handleCompressed(ctx, msg, out); + } + } + + private void handleCompressed(ChannelHandlerContext ctx, ByteBuf msg, ByteBuf out) + throws DataFormatException { + int uncompressed = msg.readableBytes(); + ByteBuf tmpBuf = MoreByteBufUtils.preferredBuffer(ctx.alloc(), compressor, uncompressed - 1); + try { + ProtocolUtils.writeVarInt(tmpBuf, uncompressed); ByteBuf compatibleIn = MoreByteBufUtils.ensureCompatible(ctx.alloc(), compressor, msg); try { - compressor.deflate(compatibleIn, out); + compressor.deflate(compatibleIn, tmpBuf); } finally { compatibleIn.release(); } + + ProtocolUtils.writeVarInt(out, tmpBuf.readableBytes()); + out.writeBytes(tmpBuf); + } finally { + tmpBuf.release(); } } From a8e0516d18b996b3a6aea80de97e017daa6e38a1 Mon Sep 17 00:00:00 2001 From: Leymooo Date: Fri, 7 May 2021 19:36:30 +0300 Subject: [PATCH 2/6] Also do not copy memory in case when packet needs to compress --- .../proxy/protocol/ProtocolUtils.java | 10 +++++ .../MinecraftCompressorAndLengthEncoder.java | 45 ++++++++++--------- .../netty/MinecraftVarintLengthEncoder.java | 2 +- .../proxy/protocol/ProtocolUtilsTest.java | 14 ++++++ 4 files changed, 49 insertions(+), 22 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java index c7e25f52..a40598e4 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java @@ -147,6 +147,16 @@ public enum ProtocolUtils { } } + /** + * Writes all integers as 3 bytes Minecraft-style VarInt to the specified {@code buf}. + * @param buf the buffer to read from + * @param value the integer to write + */ + public static void writeVarIntAs3Bytes(ByteBuf buf, int value) { + int w = (value & 0x7F | 0x80) << 16 | ((value >>> 7) & 0x7F | 0x80) << 8 | (value >>> 14); + buf.writeMedium(w); + } + public static String readString(ByteBuf buf) { return readString(buf, DEFAULT_MAX_STRING_SIZE); } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressorAndLengthEncoder.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressorAndLengthEncoder.java index 6684ee35..73cea902 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressorAndLengthEncoder.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressorAndLengthEncoder.java @@ -17,6 +17,8 @@ package com.velocitypowered.proxy.protocol.netty; +import static com.velocitypowered.proxy.protocol.netty.MinecraftVarintLengthEncoder.IS_JAVA_CIPHER; + import com.velocitypowered.natives.compression.VelocityCompressor; import com.velocitypowered.natives.util.MoreByteBufUtils; import com.velocitypowered.proxy.protocol.ProtocolUtils; @@ -50,36 +52,37 @@ public class MinecraftCompressorAndLengthEncoder extends MessageToByteEncoder { public static final MinecraftVarintLengthEncoder INSTANCE = new MinecraftVarintLengthEncoder(); - private static final boolean IS_JAVA_CIPHER = Natives.cipher.get() == JavaVelocityCipher.FACTORY; + public static final boolean IS_JAVA_CIPHER = Natives.cipher.get() == JavaVelocityCipher.FACTORY; private MinecraftVarintLengthEncoder() { } diff --git a/proxy/src/test/java/com/velocitypowered/proxy/protocol/ProtocolUtilsTest.java b/proxy/src/test/java/com/velocitypowered/proxy/protocol/ProtocolUtilsTest.java index 57cd8f7d..58530a4a 100644 --- a/proxy/src/test/java/com/velocitypowered/proxy/protocol/ProtocolUtilsTest.java +++ b/proxy/src/test/java/com/velocitypowered/proxy/protocol/ProtocolUtilsTest.java @@ -71,6 +71,20 @@ public class ProtocolUtilsTest { assertEquals(test, ProtocolUtils.readVarIntSafely(buf)); } + @Test + void test3Bytes() { + ByteBuf buf = Unpooled.buffer(5); + for (int i = 0; i < 2097152; i += 31) { + writeReadTest3Bytes(buf, i); + } + } + + private void writeReadTest3Bytes(ByteBuf buf, int test) { + buf.clear(); + ProtocolUtils.writeVarIntAs3Bytes(buf, test); + assertEquals(test, ProtocolUtils.readVarInt(buf)); + } + @Test void testBytesWrittenAtBitBoundaries() { ByteBuf varintNew = Unpooled.buffer(5); From 6369a95ec99f4a1146ff3c38f1321770331dd4f5 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sat, 8 May 2021 18:40:23 -0400 Subject: [PATCH 3/6] Readd safe and slow compression handling and hide it behind a system property --- .../MinecraftCompressorAndLengthEncoder.java | 44 +++++++++++++++++-- 1 file changed, 40 insertions(+), 4 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressorAndLengthEncoder.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressorAndLengthEncoder.java index 73cea902..34d247bc 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressorAndLengthEncoder.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressorAndLengthEncoder.java @@ -29,6 +29,9 @@ import java.util.zip.DataFormatException; public class MinecraftCompressorAndLengthEncoder extends MessageToByteEncoder { + private static final boolean MUST_USE_SAFE_AND_SLOW_COMPRESSION_HANDLING = + Boolean.getBoolean("velocity.increased-compression-cap"); + private int threshold; private final VelocityCompressor compressor; @@ -46,29 +49,62 @@ public class MinecraftCompressorAndLengthEncoder extends MessageToByteEncoder= 1 << 21) { + throw new DataFormatException("The server sent a very large (over 2MiB compressed) packet. " + + "Please restart Velocity with the JVM flag -Dvelocity.increased-compression-cap=true " + + "to fix this issue."); + } int writerIndex = out.writerIndex(); int packetLength = out.readableBytes() - 3; out.writerIndex(0); - ProtocolUtils.writeVarIntAs3Bytes(out, packetLength); //Rewrite packet length + ProtocolUtils.writeVarIntAs3Bytes(out, packetLength); // Rewrite packet length out.writerIndex(writerIndex); } + private void handleCompressedSafe(ChannelHandlerContext ctx, ByteBuf msg, ByteBuf out) + throws DataFormatException { + int uncompressed = msg.readableBytes(); + ByteBuf tmpBuf = MoreByteBufUtils.preferredBuffer(ctx.alloc(), compressor, uncompressed - 1); + try { + ProtocolUtils.writeVarInt(tmpBuf, uncompressed); + ByteBuf compatibleIn = MoreByteBufUtils.ensureCompatible(ctx.alloc(), compressor, msg); + try { + compressor.deflate(compatibleIn, tmpBuf); + } finally { + compatibleIn.release(); + } + + ProtocolUtils.writeVarInt(out, tmpBuf.readableBytes()); + out.writeBytes(tmpBuf); + } finally { + tmpBuf.release(); + } + } + @Override protected ByteBuf allocateBuffer(ChannelHandlerContext ctx, ByteBuf msg, boolean preferDirect) throws Exception { From 150fd9a9cf239b769fe46f607c95a85a6c197377 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sat, 8 May 2021 23:26:43 -0400 Subject: [PATCH 4/6] Add highly-optimized VarInt writing method --- .../proxy/protocol/ProtocolUtils.java | 30 ++++++++----------- .../MinecraftCompressorAndLengthEncoder.java | 4 +-- .../proxy/protocol/ProtocolUtilsTest.java | 2 +- 3 files changed, 16 insertions(+), 20 deletions(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java index a40598e4..ee3482c9 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java @@ -120,7 +120,7 @@ public enum ProtocolUtils { * @param value the integer to write */ public static void writeVarInt(ByteBuf buf, int value) { - // Optimization: focus on 1-3 byte VarInts as they are the most common + // See https://steinborn.me/posts/performance/how-fast-can-you-write-a-varint/ if ((value & (0xFFFFFFFF << 7)) == 0) { buf.writeByte(value); } else if ((value & (0xFFFFFFFF << 14)) == 0) { @@ -129,30 +129,26 @@ public enum ProtocolUtils { } else if ((value & (0xFFFFFFFF << 21)) == 0) { int w = (value & 0x7F | 0x80) << 16 | ((value >>> 7) & 0x7F | 0x80) << 8 | (value >>> 14); buf.writeMedium(w); + } else if ((value & (0xFFFFFFFF << 28)) == 0) { + int w = (value & 0x7F | 0x80) << 24 | (((value >>> 7) & 0x7F | 0x80) << 16) + | ((value >>> 14) & 0x7F | 0x80) << 8 | (value >>> 21); + buf.writeInt(w); } else { - // 4 and 5 byte VarInts aren't common so split those cases off - writeVarIntUncommon(buf, value); - } - } - - private static void writeVarIntUncommon(ByteBuf buf, int value) { - while (true) { - if ((value & 0xFFFFFF80) == 0) { - buf.writeByte(value); - return; - } - - buf.writeByte(value & 0x7F | 0x80); - value >>>= 7; + int w = (value & 0x7F | 0x80) << 24 | ((value >>> 7) & 0x7F | 0x80) << 16 + | ((value >>> 14) & 0x7F | 0x80) << 8 | ((value >>> 21) & 0x7F | 0x80); + buf.writeInt(w); + buf.writeByte(value >>> 28); } } /** - * Writes all integers as 3 bytes Minecraft-style VarInt to the specified {@code buf}. + * Writes the specified {@code value} as a 21-bit Minecraft VarInt to the specified {@code buf}. + * The upper 11 bits will be discarded. * @param buf the buffer to read from * @param value the integer to write */ - public static void writeVarIntAs3Bytes(ByteBuf buf, int value) { + public static void write21BitVarInt(ByteBuf buf, int value) { + // See https://steinborn.me/posts/performance/how-fast-can-you-write-a-varint/ int w = (value & 0x7F | 0x80) << 16 | ((value >>> 7) & 0x7F | 0x80) << 8 | (value >>> 14); buf.writeMedium(w); } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressorAndLengthEncoder.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressorAndLengthEncoder.java index 34d247bc..28d00516 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressorAndLengthEncoder.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftCompressorAndLengthEncoder.java @@ -61,7 +61,7 @@ public class MinecraftCompressorAndLengthEncoder extends MessageToByteEncoder Date: Sun, 9 May 2021 02:57:01 -0400 Subject: [PATCH 5/6] Improve writeVarInt inlining by peeling the two most common cases --- .../proxy/protocol/ProtocolUtils.java | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java index ee3482c9..7ae34b9d 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java @@ -120,6 +120,19 @@ public enum ProtocolUtils { * @param value the integer to write */ public static void writeVarInt(ByteBuf buf, int value) { + // Peel the one and two byte count cases explicitly as they are the most common VarInt sizes + // that the proxy will write, to improve inlining. + if ((value & (0xFFFFFFFF << 7)) == 0) { + buf.writeByte(value); + } else if ((value & (0xFFFFFFFF << 14)) == 0) { + int w = (value & 0x7F | 0x80) << 8 | (value >>> 7); + buf.writeShort(w); + } else { + writeVarIntFull(buf, value); + } + } + + private static void writeVarIntFull(ByteBuf buf, int value) { // See https://steinborn.me/posts/performance/how-fast-can-you-write-a-varint/ if ((value & (0xFFFFFFFF << 7)) == 0) { buf.writeByte(value); @@ -135,7 +148,7 @@ public enum ProtocolUtils { buf.writeInt(w); } else { int w = (value & 0x7F | 0x80) << 24 | ((value >>> 7) & 0x7F | 0x80) << 16 - | ((value >>> 14) & 0x7F | 0x80) << 8 | ((value >>> 21) & 0x7F | 0x80); + | ((value >>> 14) & 0x7F | 0x80) | ((value >>> 21) & 0x7F | 0x80); buf.writeInt(w); buf.writeByte(value >>> 28); } From 11ed4b46e4b19a22a10063b03b41be0ae7d55a67 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Sun, 9 May 2021 02:57:52 -0400 Subject: [PATCH 6/6] whoops --- .../java/com/velocitypowered/proxy/protocol/ProtocolUtils.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java index 7ae34b9d..53daae29 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/ProtocolUtils.java @@ -148,7 +148,7 @@ public enum ProtocolUtils { buf.writeInt(w); } else { int w = (value & 0x7F | 0x80) << 24 | ((value >>> 7) & 0x7F | 0x80) << 16 - | ((value >>> 14) & 0x7F | 0x80) | ((value >>> 21) & 0x7F | 0x80); + | ((value >>> 14) & 0x7F | 0x80) << 8 | ((value >>> 21) & 0x7F | 0x80); buf.writeInt(w); buf.writeByte(value >>> 28); }