From fc7a7396ef2dab4e6995432fcd10245f00380164 Mon Sep 17 00:00:00 2001 From: Andrew Steinborn Date: Mon, 17 Aug 2020 19:37:55 -0400 Subject: [PATCH] Make several tweaks to Protocol to improve inlining and performance on hot paths --- .../ViaVersion/api/protocol/Protocol.java | 48 ++++++----- .../api/protocol/ProtocolPipeline.java | 80 ++++++++++--------- 2 files changed, 67 insertions(+), 61 deletions(-) diff --git a/common/src/main/java/us/myles/ViaVersion/api/protocol/Protocol.java b/common/src/main/java/us/myles/ViaVersion/api/protocol/Protocol.java index c6f9c5b6a..362e45371 100644 --- a/common/src/main/java/us/myles/ViaVersion/api/protocol/Protocol.java +++ b/common/src/main/java/us/myles/ViaVersion/api/protocol/Protocol.java @@ -411,34 +411,38 @@ public abstract class Protocol packetTypeClass = state == State.PLAY ? (direction == Direction.OUTGOING ? oldClientboundPacketEnum : newServerboundPacketEnum) : null; - if (packetTypeClass != null) { - PacketType[] enumConstants = packetTypeClass.getEnumConstants(); - PacketType packetType = oldId < enumConstants.length && oldId >= 0 ? enumConstants[oldId] : null; - Via.getPlatform().getLogger().warning("ERROR IN " + getClass().getSimpleName() + " IN REMAP OF " + packetType + " (" + toNiceHex(oldId) + ")"); - } else { - Via.getPlatform().getLogger().warning("ERROR IN " + getClass().getSimpleName() - + " IN REMAP OF " + toNiceHex(oldId) + "->" + toNiceHex(newId)); - } + private void throwRemapError(Direction direction, State state, int oldId, int newId, InformativeException e) throws InformativeException { + // Don't print errors during handshake + if (state == State.HANDSHAKE) { throw e; } - if (packetWrapper.isCancelled()) { - throw CancelException.generate(); + Class packetTypeClass = state == State.PLAY ? (direction == Direction.OUTGOING ? oldClientboundPacketEnum : newServerboundPacketEnum) : null; + if (packetTypeClass != null) { + PacketType[] enumConstants = packetTypeClass.getEnumConstants(); + PacketType packetType = oldId < enumConstants.length && oldId >= 0 ? enumConstants[oldId] : null; + Via.getPlatform().getLogger().warning("ERROR IN " + getClass().getSimpleName() + " IN REMAP OF " + packetType + " (" + toNiceHex(oldId) + ")"); + } else { + Via.getPlatform().getLogger().warning("ERROR IN " + getClass().getSimpleName() + + " IN REMAP OF " + toNiceHex(oldId) + "->" + toNiceHex(newId)); } + throw e; } private String toNiceHex(int id) { diff --git a/common/src/main/java/us/myles/ViaVersion/api/protocol/ProtocolPipeline.java b/common/src/main/java/us/myles/ViaVersion/api/protocol/ProtocolPipeline.java index 184ae225e..f73d071cc 100644 --- a/common/src/main/java/us/myles/ViaVersion/api/protocol/ProtocolPipeline.java +++ b/common/src/main/java/us/myles/ViaVersion/api/protocol/ProtocolPipeline.java @@ -1,5 +1,6 @@ package us.myles.ViaVersion.api.protocol; +import com.google.common.collect.Lists; import us.myles.ViaVersion.api.PacketWrapper; import us.myles.ViaVersion.api.Via; import us.myles.ViaVersion.api.data.UserConnection; @@ -9,13 +10,13 @@ import us.myles.ViaVersion.packets.State; import us.myles.ViaVersion.protocols.base.ProtocolInfo; import java.util.ArrayList; -import java.util.Collections; import java.util.List; import java.util.concurrent.CopyOnWriteArrayList; import java.util.logging.Level; public class ProtocolPipeline extends SimpleProtocol { - private List protocolList; + private List incoming; + private List outgoing; private UserConnection userConnection; public ProtocolPipeline(UserConnection userConnection) { @@ -25,9 +26,10 @@ public class ProtocolPipeline extends SimpleProtocol { @Override protected void registerPackets() { - protocolList = new CopyOnWriteArrayList<>(); + incoming = new CopyOnWriteArrayList<>(); + outgoing = Lists.reverse(incoming); // This is a pipeline so we register basic pipes - protocolList.add(ProtocolRegistry.BASE_PROTOCOL); + incoming.add(ProtocolRegistry.BASE_PROTOCOL); } @Override @@ -40,7 +42,7 @@ public class ProtocolPipeline extends SimpleProtocol { userConnection.setProtocolInfo(protocolInfo); /* Init through all our pipes */ - for (Protocol protocol : protocolList) { + for (Protocol protocol : incoming) { protocol.init(userConnection); } } @@ -52,59 +54,59 @@ public class ProtocolPipeline extends SimpleProtocol { * @param protocol The protocol to add to the end */ public void add(Protocol protocol) { - if (protocolList != null) { - protocolList.add(protocol); + if (incoming != null) { + incoming.add(protocol); protocol.init(userConnection); // Move base Protocols to the end, so the login packets can be modified by other protocols List toMove = new ArrayList<>(); - for (Protocol p : protocolList) { + for (Protocol p : incoming) { if (ProtocolRegistry.isBaseProtocol(p)) { toMove.add(p); } } - protocolList.removeAll(toMove); - protocolList.addAll(toMove); + incoming.removeAll(toMove); + incoming.addAll(toMove); } else { - throw new NullPointerException("Tried to add protocol to early"); + throw new NullPointerException("Tried to add protocol too early"); } } @Override public void transform(Direction direction, State state, PacketWrapper packetWrapper) throws Exception { int originalID = packetWrapper.getId(); - List protocols = new ArrayList<>(protocolList); - // Other way if outgoing - if (direction == Direction.OUTGOING) - Collections.reverse(protocols); + List protocols = direction == Direction.OUTGOING ? outgoing : incoming; // Apply protocols packetWrapper.apply(direction, state, 0, protocols); super.transform(direction, state, packetWrapper); if (Via.getManager().isDebug()) { - // Debug packet - int serverProtocol = userConnection.getProtocolInfo().getServerProtocolVersion(); - int clientProtocol = userConnection.getProtocolInfo().getProtocolVersion(); - ViaPlatform platform = Via.getPlatform(); - - String actualUsername = packetWrapper.user().getProtocolInfo().getUsername(); - String username = actualUsername != null ? actualUsername + " " : ""; - - platform.getLogger().log(Level.INFO, "{0}{1} {2}: {3} (0x{4}) -> {5} (0x{6}) [{7}] {8}", - new Object[]{ - username, - direction, - state, - originalID, - Integer.toHexString(originalID), - packetWrapper.getId(), - Integer.toHexString(packetWrapper.getId()), - Integer.toString(clientProtocol), - packetWrapper - }); + logPacket(direction, state, packetWrapper, originalID); } } + private void logPacket(Direction direction, State state, PacketWrapper packetWrapper, int originalID) { + // Debug packet + int clientProtocol = userConnection.getProtocolInfo().getProtocolVersion(); + ViaPlatform platform = Via.getPlatform(); + + String actualUsername = packetWrapper.user().getProtocolInfo().getUsername(); + String username = actualUsername != null ? actualUsername + " " : ""; + + platform.getLogger().log(Level.INFO, "{0}{1} {2}: {3} (0x{4}) -> {5} (0x{6}) [{7}] {8}", + new Object[]{ + username, + direction, + state, + originalID, + Integer.toHexString(originalID), + packetWrapper.getId(), + Integer.toHexString(packetWrapper.getId()), + Integer.toString(clientProtocol), + packetWrapper + }); + } + /** * Check if the pipeline contains a protocol * @@ -112,14 +114,14 @@ public class ProtocolPipeline extends SimpleProtocol { * @return True if the protocol class is in the pipeline */ public boolean contains(Class pipeClass) { - for (Protocol protocol : protocolList) { + for (Protocol protocol : incoming) { if (protocol.getClass().equals(pipeClass)) return true; } return false; } public

P getProtocol(Class

pipeClass) { - for (Protocol protocol : protocolList) { + for (Protocol protocol : incoming) { if (protocol.getClass() == pipeClass) return (P) protocol; } return null; @@ -134,7 +136,7 @@ public class ProtocolPipeline extends SimpleProtocol { * @throws Exception If it failed to convert / packet cancelld. */ public boolean filter(Object o, List list) throws Exception { - for (Protocol protocol : protocolList) { + for (Protocol protocol : incoming) { if (protocol.isFiltered(o.getClass())) { protocol.filterPacket(userConnection, o, list); return true; @@ -145,7 +147,7 @@ public class ProtocolPipeline extends SimpleProtocol { } public List pipes() { - return protocolList; + return incoming; } /**