From abbdf70d5e631a85e8e2cf80076d4e6d97000041 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Tue, 7 Aug 2018 04:28:07 -0400 Subject: [PATCH] [Experimental] Remove PacketWrapper objects In both Velocity and BungeeCord, the most commonly created object is an object that encapsulates a Minecraft packet and a its associated byte data. At first, I considered trying to recycle these objects, but then I discovered that this object has no reason to exist, and actually somewhat complicates the implementation. Thus, this commit removes these objects, making Velocity more more GC-friendly by not allocating frequently-created objects. This is still an experimental change, but it's a fairly safe one to make. --- .../proxy/connection/MinecraftConnection.java | 19 +++++------- .../proxy/protocol/PacketWrapper.java | 30 ------------------- .../protocol/netty/LegacyPingDecoder.java | 6 ++-- .../protocol/netty/MinecraftDecoder.java | 6 ++-- 4 files changed, 12 insertions(+), 49 deletions(-) delete mode 100644 proxy/src/main/java/com/velocitypowered/proxy/protocol/PacketWrapper.java 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 e829c008e..3582801a7 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/connection/MinecraftConnection.java @@ -5,12 +5,12 @@ import com.velocitypowered.natives.compression.VelocityCompressor; import com.velocitypowered.natives.encryption.VelocityCipherFactory; import com.velocitypowered.natives.util.Natives; import com.velocitypowered.proxy.VelocityServer; -import com.velocitypowered.proxy.protocol.PacketWrapper; +import com.velocitypowered.proxy.protocol.MinecraftPacket; import com.velocitypowered.proxy.protocol.ProtocolConstants; import com.velocitypowered.proxy.protocol.StateRegistry; -import com.velocitypowered.natives.encryption.JavaVelocityCipher; import com.velocitypowered.natives.encryption.VelocityCipher; import com.velocitypowered.proxy.protocol.netty.*; +import io.netty.buffer.ByteBuf; import io.netty.channel.Channel; import io.netty.channel.ChannelFutureListener; import io.netty.channel.ChannelHandlerContext; @@ -79,18 +79,13 @@ public class MinecraftConnection extends ChannelInboundHandlerAdapter { @Override public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception { - if (msg instanceof PacketWrapper) { - PacketWrapper pw = (PacketWrapper) msg; + if (msg instanceof MinecraftPacket) { + sessionHandler.handle((MinecraftPacket) msg); + } else if (msg instanceof ByteBuf) { try { - if (sessionHandler != null) { - if (pw.getPacket() == null) { - sessionHandler.handleUnknown(pw.getBuffer()); - } else { - sessionHandler.handle(pw.getPacket()); - } - } + sessionHandler.handleUnknown((ByteBuf) msg); } finally { - ReferenceCountUtil.release(pw.getBuffer()); + ReferenceCountUtil.release(msg); } } } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/PacketWrapper.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/PacketWrapper.java deleted file mode 100644 index 5e1fa8bfb..000000000 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/PacketWrapper.java +++ /dev/null @@ -1,30 +0,0 @@ -package com.velocitypowered.proxy.protocol; - -import io.netty.buffer.ByteBuf; -import io.netty.buffer.ByteBufUtil; - -public class PacketWrapper { - private final MinecraftPacket packet; - private final ByteBuf buffer; - - public PacketWrapper(MinecraftPacket packet, ByteBuf buffer) { - this.packet = packet; - this.buffer = buffer; - } - - public MinecraftPacket getPacket() { - return packet; - } - - public ByteBuf getBuffer() { - return buffer; - } - - @Override - public String toString() { - return "PacketWrapper{" + - "packet=" + packet + - ", buffer=" + ByteBufUtil.hexDump(buffer) + - '}'; - } -} diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/LegacyPingDecoder.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/LegacyPingDecoder.java index 153421386..dfe396c7e 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/LegacyPingDecoder.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/LegacyPingDecoder.java @@ -1,10 +1,8 @@ package com.velocitypowered.proxy.protocol.netty; -import com.velocitypowered.proxy.protocol.PacketWrapper; import com.velocitypowered.proxy.protocol.packet.LegacyHandshake; import com.velocitypowered.proxy.protocol.packet.LegacyPing; import io.netty.buffer.ByteBuf; -import io.netty.buffer.Unpooled; import io.netty.channel.ChannelHandlerContext; import io.netty.handler.codec.ByteToMessageDecoder; @@ -21,10 +19,10 @@ public class LegacyPingDecoder extends ByteToMessageDecoder { short second = in.getUnsignedByte(in.readerIndex() + 1); if (first == 0xfe && second == 0x01) { in.skipBytes(in.readableBytes()); - out.add(new PacketWrapper(new LegacyPing(), Unpooled.EMPTY_BUFFER)); + out.add(new LegacyPing()); } else if (first == 0x02) { in.skipBytes(in.readableBytes()); - out.add(new PacketWrapper(new LegacyHandshake(), Unpooled.EMPTY_BUFFER)); + out.add(new LegacyHandshake()); } else { ctx.pipeline().remove(this); } diff --git a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftDecoder.java b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftDecoder.java index d8768987a..f4baba7ac 100644 --- a/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftDecoder.java +++ b/proxy/src/main/java/com/velocitypowered/proxy/protocol/netty/MinecraftDecoder.java @@ -26,13 +26,13 @@ public class MinecraftDecoder extends MessageToMessageDecoder { return; } - ByteBuf slice = msg.retainedSlice(); + ByteBuf slice = msg.slice(); int packetId = ProtocolUtils.readVarInt(msg); MinecraftPacket packet = this.protocolVersion.createPacket(packetId); if (packet == null) { msg.skipBytes(msg.readableBytes()); - out.add(new PacketWrapper(null, slice)); + out.add(slice.retain()); } else { try { packet.decode(msg, direction, protocolVersion.id); @@ -40,7 +40,7 @@ public class MinecraftDecoder extends MessageToMessageDecoder { throw new CorruptedFrameException("Error decoding " + packet.getClass() + " Direction " + direction + " Protocol " + protocolVersion + " State " + state + " ID " + Integer.toHexString(packetId), e); } - out.add(new PacketWrapper(packet, slice)); + out.add(packet); } }