From 1ea548f05d102b0a723a641438e3c04758aa93f5 Mon Sep 17 00:00:00 2001 From: KennyTV Date: Sun, 6 Sep 2020 10:24:14 +0200 Subject: [PATCH] Avoid List creation and collection reversal in transforming --- .../myles/ViaVersion/api/PacketWrapper.java | 72 ++++++++++++------- .../ViaVersion/api/protocol/Protocol.java | 8 +++ .../api/protocol/ProtocolPipeline.java | 51 +++++++------ .../api/protocol/ProtocolRegistry.java | 1 + 4 files changed, 80 insertions(+), 52 deletions(-) diff --git a/common/src/main/java/us/myles/ViaVersion/api/PacketWrapper.java b/common/src/main/java/us/myles/ViaVersion/api/PacketWrapper.java index 695d00c9a..de6154a7a 100644 --- a/common/src/main/java/us/myles/ViaVersion/api/PacketWrapper.java +++ b/common/src/main/java/us/myles/ViaVersion/api/PacketWrapper.java @@ -16,13 +16,13 @@ import us.myles.ViaVersion.util.PipelineUtil; import java.io.IOException; import java.util.ArrayList; -import java.util.Collections; import java.util.LinkedList; import java.util.List; import java.util.NoSuchElementException; public class PacketWrapper { public static final int PASSTHROUGH_ID = 1000; + private static final Protocol[] PROTOCOL_ARRAY = new Protocol[0]; private final ByteBuf inputBuffer; private final UserConnection userConnection; @@ -317,30 +317,33 @@ public class PacketWrapper { * @throws Exception if it fails to write */ private ByteBuf constructPacket(Class packetProtocol, boolean skipCurrentPipeline, Direction direction) throws Exception { - // Apply current pipeline - List protocols = new ArrayList<>(user().getProtocolInfo().getPipeline().pipes()); - if (direction == Direction.OUTGOING) { - // Other way if outgoing - Collections.reverse(protocols); - } + // Apply current pipeline - for outgoing protocol, the collection will be reversed in the apply method + Protocol[] protocols = user().getProtocolInfo().getPipeline().pipes().toArray(PROTOCOL_ARRAY); + boolean reverse = direction == Direction.OUTGOING; int index = -1; - for (int i = 0; i < protocols.size(); i++) { - if (protocols.get(i).getClass().equals(packetProtocol)) { - index = skipCurrentPipeline ? (i + 1) : (i); + for (int i = 0; i < protocols.length; i++) { + if (protocols[i].getClass() == packetProtocol) { + index = i; break; } } - if (index == -1) throw new NoSuchElementException(packetProtocol.getCanonicalName()); + + if (index == -1) { + // The given protocol is not in the pipeline + throw new NoSuchElementException(packetProtocol.getCanonicalName()); + } + + if (skipCurrentPipeline) { + index = reverse ? index - 1 : index + 1; + } // Reset reader before we start resetReader(); // Apply other protocols - apply(direction, user().getProtocolInfo().getState(), index, protocols); - // Send + apply(direction, user().getProtocolInfo().getState(), index, protocols, reverse); ByteBuf output = inputBuffer == null ? user().getChannel().alloc().buffer() : inputBuffer.alloc().buffer(); writeToBuffer(output); - return output; } @@ -417,22 +420,41 @@ public class PacketWrapper { } /** - * Applies a pipeline from an index to the wrapper + * Applies a pipeline from an index to the wrapper. * - * @param direction The direction - * @param state The state - * @param index The index to start from - * @param pipeline The pipeline + * @param direction protocol direction + * @param state protocol state + * @param index index to start from, will be reversed depending on the reverse parameter + * @param pipeline protocol pipeline + * @param reverse whether the array should be looped in reverse, will also reverse the given index * @return The current packetwrapper * @throws Exception If it fails to transform a packet, exception will be thrown */ - public PacketWrapper apply(Direction direction, State state, int index, List pipeline) throws Exception { - for (int i = index; i < pipeline.size(); i++) { // Copy to prevent from removal. - pipeline.get(i).transform(direction, state, this); - // Reset the reader for the packetWrapper (So it can be recycled across packets) - resetReader(); - } + public PacketWrapper apply(Direction direction, State state, int index, List pipeline, boolean reverse) throws Exception { + Protocol[] array = pipeline.toArray(PROTOCOL_ARRAY); + return apply(direction, state, reverse ? array.length - 1 : index, array, reverse); // Copy to prevent from removal + } + /** + * @see #apply(Direction, State, int, List, boolean) + */ + public PacketWrapper apply(Direction direction, State state, int index, List pipeline) throws Exception { + return apply(direction, state, index, pipeline.toArray(PROTOCOL_ARRAY), false); + } + + private PacketWrapper apply(Direction direction, State state, int index, Protocol[] pipeline, boolean reverse) throws Exception { + // Reset the reader after every transformation for the packetWrapper, so it can be recycled across packets + if (reverse) { + for (int i = index; i >= 0; i--) { + pipeline[i].transform(direction, state, this); + resetReader(); + } + } else { + for (int i = index; i < pipeline.length; i++) { + pipeline[i].transform(direction, state, this); + resetReader(); + } + } return this; } 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 e833e3cf6..b4914f571 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 @@ -459,6 +459,14 @@ public abstract class Protocol + * This does *not* necessarily mean that {@link #getMappingData()} is non-null, since this may be + * overriden, depending on special cases. + * + * @return true if this Protocol's {@link #loadMappingData()} method should be called + */ public boolean hasMappingDataToLoad() { return getMappingData() != null; } 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 6aa3501d7..8e4473599 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 @@ -9,7 +9,6 @@ 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; @@ -19,7 +18,6 @@ public class ProtocolPipeline extends SimpleProtocol { private UserConnection userConnection; public ProtocolPipeline(UserConnection userConnection) { - super(); init(userConnection); } @@ -72,39 +70,38 @@ public class ProtocolPipeline extends SimpleProtocol { @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); // Apply protocols - packetWrapper.apply(direction, state, 0, protocols); + packetWrapper.apply(direction, state, 0, protocolList, direction == Direction.OUTGOING); 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 * diff --git a/common/src/main/java/us/myles/ViaVersion/api/protocol/ProtocolRegistry.java b/common/src/main/java/us/myles/ViaVersion/api/protocol/ProtocolRegistry.java index 08a519484..b838046a0 100644 --- a/common/src/main/java/us/myles/ViaVersion/api/protocol/ProtocolRegistry.java +++ b/common/src/main/java/us/myles/ViaVersion/api/protocol/ProtocolRegistry.java @@ -372,6 +372,7 @@ public class ProtocolRegistry { } private static void shutdownLoaderExecutor() { + Via.getPlatform().getLogger().info("Shutting down mapping loader executor!"); mappingsLoaded = true; mappingLoaderExecutor.shutdown(); mappingLoaderExecutor = null;