From 0c6fa4687159c8b81e0d4ad9606872c31cc73d50 Mon Sep 17 00:00:00 2001 From: Dan Mulloy Date: Sat, 25 Mar 2023 23:16:04 -0500 Subject: [PATCH] Optimize class lookups --- .../comphenix/protocol/async/AsyncMarker.java | 2 +- .../protocol/injector/StructureCache.java | 4 +- .../netty/channel/NettyChannelInjector.java | 7 +- .../protocol/utility/CachedPackage.java | 32 ++++++-- .../protocol/utility/ClassSource.java | 43 ++++------ .../protocol/utility/MinecraftReflection.java | 79 ++++++++++--------- .../protocol/utility/MinecraftVersion.java | 4 +- .../comphenix/protocol/utility/Optionals.java | 45 +++++++++++ .../protocol/wrappers/TroveWrapper.java | 15 ++-- .../wrappers/WrappedAttributeModifier.java | 2 +- 10 files changed, 140 insertions(+), 93 deletions(-) create mode 100644 src/main/java/com/comphenix/protocol/utility/Optionals.java diff --git a/src/main/java/com/comphenix/protocol/async/AsyncMarker.java b/src/main/java/com/comphenix/protocol/async/AsyncMarker.java index 563c4312..b4681965 100644 --- a/src/main/java/com/comphenix/protocol/async/AsyncMarker.java +++ b/src/main/java/com/comphenix/protocol/async/AsyncMarker.java @@ -414,7 +414,7 @@ public class AsyncMarker implements Serializable, Comparable { } else if (methods.size() == 1) { // We're in 1.2.5 alwaysSync = true; - } else if (MinecraftVersion.getCurrentVersion().isAtLeast(MinecraftVersion.BOUNTIFUL_UPDATE)) { + } else if (MinecraftVersion.BOUNTIFUL_UPDATE.atOrAbove()) { // The centralized async marker was removed in 1.8 // Incoming chat packets can be async if (event.getPacketType() == PacketType.Play.Client.CHAT) { diff --git a/src/main/java/com/comphenix/protocol/injector/StructureCache.java b/src/main/java/com/comphenix/protocol/injector/StructureCache.java index 624c6f93..292dca95 100644 --- a/src/main/java/com/comphenix/protocol/injector/StructureCache.java +++ b/src/main/java/com/comphenix/protocol/injector/StructureCache.java @@ -137,8 +137,8 @@ public class StructureCache { Class packetClass = PacketRegistry.getPacketClassFromType(type); // We need to map the Bundle Delimiter to the synthetic bundle packet which contains a list of all packets in a bundle - if (MinecraftVersion.atOrAbove(MinecraftVersion.FEATURE_PREVIEW_2) && packetClass.equals(MinecraftReflection.getBundleDelimiterClass())) { - packetClass = MinecraftReflection.getPackedBundlePacketClass(); + if (MinecraftReflection.isBundleDelimiter(packetClass)) { + packetClass = MinecraftReflection.getPackedBundlePacketClass().get(); } return new StructureModifier<>(packetClass, MinecraftReflection.getPacketClass(), true); diff --git a/src/main/java/com/comphenix/protocol/injector/netty/channel/NettyChannelInjector.java b/src/main/java/com/comphenix/protocol/injector/netty/channel/NettyChannelInjector.java index 22ea4c8f..91e55ae9 100644 --- a/src/main/java/com/comphenix/protocol/injector/netty/channel/NettyChannelInjector.java +++ b/src/main/java/com/comphenix/protocol/injector/netty/channel/NettyChannelInjector.java @@ -204,15 +204,10 @@ public class NettyChannelInjector implements Injector { return false; } - String anchorHandler = "decoder"; - if (MinecraftVersion.FEATURE_PREVIEW_2.atOrAbove()) { - anchorHandler = "unbundler"; - } - // inject our handlers this.wrappedChannel.pipeline().addAfter("encoder", WIRE_PACKET_ENCODER_NAME, WIRE_PACKET_ENCODER); this.wrappedChannel.pipeline().addAfter( - anchorHandler, + "decoder", INTERCEPTOR_NAME, new InboundPacketInterceptor(this, this.channelListener)); diff --git a/src/main/java/com/comphenix/protocol/utility/CachedPackage.java b/src/main/java/com/comphenix/protocol/utility/CachedPackage.java index 23bbff2d..9f375759 100644 --- a/src/main/java/com/comphenix/protocol/utility/CachedPackage.java +++ b/src/main/java/com/comphenix/protocol/utility/CachedPackage.java @@ -36,7 +36,7 @@ final class CachedPackage { * @param packageName - the name of the current package. * @param source - the class source. */ - public CachedPackage(String packageName, ClassSource source) { + CachedPackage(String packageName, ClassSource source) { this.source = source; this.packageName = packageName; this.cache = new ConcurrentHashMap<>(); @@ -71,6 +71,16 @@ final class CachedPackage { } } + private Optional> resolveClass(String className) { + return source.loadClass(combine(packageName, className)); + } + + public Class requireClass(String className) throws ClassNotFoundException { + String canonicalName = combine(packageName, className); + return source.loadClass(canonicalName) + .orElseThrow(() -> new ClassNotFoundException(className)); + } + /** * Retrieve the class object of a specific class in the current package. * @@ -78,13 +88,21 @@ final class CachedPackage { * @return Class object. * @throws RuntimeException If we are unable to find the given class. */ - public Optional> getPackageClass(final String className) { - return this.cache.computeIfAbsent(className, x -> { - try { - return Optional.ofNullable(this.source.loadClass(combine(this.packageName, className))); - } catch (ClassNotFoundException ex) { - return Optional.empty(); + public Optional> getPackageClass(String className, String... aliases) { + return cache.computeIfAbsent(className, x -> { + Optional> clazz = resolveClass(className); + if (clazz.isPresent()) { + return clazz; } + + for (String alias : aliases) { + clazz = resolveClass(className); + if (clazz.isPresent()) { + return clazz; + } + } + + return Optional.empty(); }); } } diff --git a/src/main/java/com/comphenix/protocol/utility/ClassSource.java b/src/main/java/com/comphenix/protocol/utility/ClassSource.java index 1ddc9956..b35bb91b 100644 --- a/src/main/java/com/comphenix/protocol/utility/ClassSource.java +++ b/src/main/java/com/comphenix/protocol/utility/ClassSource.java @@ -2,6 +2,8 @@ package com.comphenix.protocol.utility; import java.util.Collections; import java.util.Map; +import java.util.Optional; +import java.util.function.Supplier; /** * Represents an abstract class loader that can only retrieve classes by their canonical name. @@ -37,27 +39,25 @@ public interface ClassSource { * @return The corresponding class source. */ static ClassSource fromClassLoader(final ClassLoader loader) { - return loader::loadClass; + return canonicalName -> { + try { + return Optional.of(loader.loadClass(canonicalName)); + } catch (ClassNotFoundException ignored) { + return Optional.empty(); + } + }; } /** * Construct a class source from a mapping of canonical names and the corresponding classes. If the map is null, it * will be interpreted as an empty map. If the map does not contain a Class with the specified name, or that string - * maps to NULL explicitly, a {@link ClassNotFoundException} will be thrown. + * maps to NULL explicitly, an empty optional will be returned * * @param map - map of class names and classes. * @return The class source. */ static ClassSource fromMap(final Map> map) { - return canonicalName -> { - Class loaded = map == null ? null : map.get(canonicalName); - if (loaded == null) { - // Throw the appropriate exception if we can't load the class - throw new ClassNotFoundException("The specified class could not be found by this ClassLoader."); - } - - return loaded; - }; + return canonicalName -> Optional.ofNullable(map.get(canonicalName)); } /** @@ -94,14 +94,8 @@ public interface ClassSource { * @param other - the other class source. * @return A new class source. */ - default ClassSource retry(final ClassSource other) { - return canonicalName -> { - try { - return ClassSource.this.loadClass(canonicalName); - } catch (ClassNotFoundException e) { - return other.loadClass(canonicalName); - } - }; + default ClassSource retry(ClassSource other) { + return canonicalName -> Optionals.or(loadClass(canonicalName), () -> other.loadClass(canonicalName)); } /** @@ -115,12 +109,9 @@ public interface ClassSource { } /** - * Retrieve a class by name. - * - * @param canonicalName - the full canonical name of the class. - * @return The corresponding class. If the class is not found, NULL should not be returned, instead a {@code - * ClassNotFoundException} exception should be thrown. - * @throws ClassNotFoundException If the class could not be found. + * Retrieve a class by its canonical name + * @param canonicalName The class's canonical name, i.e. java.lang.Object + * @return Optional that may contain a Class */ - Class loadClass(String canonicalName) throws ClassNotFoundException; + Optional> loadClass(String canonicalName); } diff --git a/src/main/java/com/comphenix/protocol/utility/MinecraftReflection.java b/src/main/java/com/comphenix/protocol/utility/MinecraftReflection.java index 20a22fbd..9a7cccd6 100644 --- a/src/main/java/com/comphenix/protocol/utility/MinecraftReflection.java +++ b/src/main/java/com/comphenix/protocol/utility/MinecraftReflection.java @@ -25,6 +25,7 @@ import java.lang.reflect.ParameterizedType; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.logging.Level; import java.util.regex.Matcher; import java.util.regex.Pattern; @@ -628,16 +629,20 @@ public final class MinecraftReflection { return getMinecraftClass("network.chat.IChatBaseComponent", "network.chat.IChatbaseComponent", "network.chat.Component", "IChatBaseComponent"); } - public static Class getPackedBundlePacketClass() { - return getMinecraftClass("network.protocol.game.ClientboundBundlePacket", "ClientboundBundlePacket"); + public static Optional> getPackedBundlePacketClass() { + return getOptionalNMS("network.protocol.game.ClientboundBundlePacket", "ClientboundBundlePacket"); } public static boolean isBundlePacket(Class packetClass) { - return MinecraftVersion.FEATURE_PREVIEW_2.atOrAbove() && packetClass.equals(getPackedBundlePacketClass()); + return Optionals.Equals(getPackedBundlePacketClass(), packetClass); } - public static Class getBundleDelimiterClass() { - return getMinecraftClass("network.protocol.BundleDelimiterPacket","BundleDelimiterPacket"); + public static boolean isBundleDelimiter(Class packetClass) { + return Optionals.Equals(getBundleDelimiterClass(), packetClass); + } + + public static Optional> getBundleDelimiterClass() { + return getOptionalNMS("network.protocol.BundleDelimiterPacket","BundleDelimiterPacket"); } public static Class getIChatBaseComponentArrayClass() { @@ -1337,11 +1342,8 @@ public final class MinecraftReflection { * @return The class. */ private static Class getClass(String className) { - try { - return getClassSource().loadClass(className); - } catch (ClassNotFoundException e) { - throw new RuntimeException("Cannot find class " + className, e); - } + return getClassSource().loadClass(className) + .orElseThrow(() -> new RuntimeException("Cannot find class " + className)); } /** @@ -1376,12 +1378,34 @@ public final class MinecraftReflection { .orElseThrow(() -> new RuntimeException("Failed to find NMS class: " + className)); } - public static Class getNullableNMS(String className, String... aliases) { - try { - return getMinecraftClass(className, aliases); - } catch (RuntimeException ex) { - return null; + /** + * Optionally retrieve the class object of a NMS (net.minecraft.server) class. + * If the class does not exist, the optional will be empty + * + * @param className NMS class name + * @param aliases Potential aliases + * @return Optional that may contain the class + */ + private static Optional> getOptionalNMS(String className, String... aliases) { + if (minecraftPackage == null) { + minecraftPackage = new CachedPackage(getMinecraftPackage(), getClassSource()); } + + return minecraftPackage.getPackageClass(className, aliases); + } + + /** + * Retrieves a nullable NMS (net.minecraft.server) class. We will attempt to + * look up the class and its aliases, but will return null if none is found. + * + * @deprecated - Use getOptionalNMS where possible + * @param className NMS class name + * @param aliases Potential aliases + * @return The class, or null if not found + */ + @Deprecated + public static Class getNullableNMS(String className, String... aliases) { + return getOptionalNMS(className, aliases).orElse(null); } /** @@ -1418,29 +1442,8 @@ public final class MinecraftReflection { * @throws RuntimeException If we are unable to find any of the given classes. */ public static Class getMinecraftClass(String className, String... aliases) { - if (minecraftPackage == null) { - minecraftPackage = new CachedPackage(getMinecraftPackage(), getClassSource()); - } - - return minecraftPackage.getPackageClass(className).orElseGet(() -> { - Class resolved = null; - for (String alias : aliases) { - // try to resolve the class and stop searching if we found it - resolved = minecraftPackage.getPackageClass(alias).orElse(null); - if (resolved != null) { - break; - } - } - - // if we resolved the class cache it and return the result - if (resolved != null) { - minecraftPackage.setPackageClass(className, resolved); - return resolved; - } - - // unable to find the class - throw new RuntimeException(String.format("Unable to find %s (%s)", className, String.join(", ", aliases))); - }); + return getOptionalNMS(className, aliases) + .orElseThrow(() -> new RuntimeException(String.format("Unable to find %s (%s)", className, String.join(", ", aliases)))); } /** diff --git a/src/main/java/com/comphenix/protocol/utility/MinecraftVersion.java b/src/main/java/com/comphenix/protocol/utility/MinecraftVersion.java index 99ff0f2f..ec58dd84 100644 --- a/src/main/java/com/comphenix/protocol/utility/MinecraftVersion.java +++ b/src/main/java/com/comphenix/protocol/utility/MinecraftVersion.java @@ -273,7 +273,7 @@ public final class MinecraftVersion implements Comparable, Ser currentVersion = version; } - public static boolean atOrAbove(MinecraftVersion version) { + private static boolean atOrAbove(MinecraftVersion version) { return getCurrentVersion().isAtLeast(version); } @@ -354,7 +354,7 @@ public final class MinecraftVersion implements Comparable, Ser */ public boolean atOrAbove() { if (this.atCurrentOrAbove == null) { - this.atCurrentOrAbove = MinecraftVersion.atOrAbove(this); + this.atCurrentOrAbove = atOrAbove(this); } return this.atCurrentOrAbove; diff --git a/src/main/java/com/comphenix/protocol/utility/Optionals.java b/src/main/java/com/comphenix/protocol/utility/Optionals.java new file mode 100644 index 00000000..48bee481 --- /dev/null +++ b/src/main/java/com/comphenix/protocol/utility/Optionals.java @@ -0,0 +1,45 @@ +package com.comphenix.protocol.utility; + +import java.util.Optional; +import java.util.function.Predicate; +import java.util.function.Supplier; + +/** + * Utility methods for operating with Optionals + */ +public final class Optionals { + + /** + * Chains two optionals together by returning the secondary + * optional if the primary does not contain a value + * @param primary Primary optional + * @param secondary Supplier of secondary optional + * @return The resulting optional + * @param Type + */ + public static Optional or(Optional primary, Supplier> secondary) { + return primary.isPresent() ? primary : secondary.get(); + } + + /** + * Evaluates the provided predicate against the optional only if it is present + * @param optional Optional + * @param predicate Test to run against potential value + * @return True if the optional is present and the predicate passes + * @param Type + */ + public static boolean TestIfPresent(Optional optional, Predicate predicate) { + return optional.isPresent() && predicate.test(optional.get()); + } + + /** + * Check if the optional has a value and its value equals the provided value + * @param optional Optional + * @param contents Contents to test for + * @return True if the optional has a value and that value equals the parameter + * @param Type + */ + public static boolean Equals(Optional optional, Class contents) { + return optional.isPresent() && contents.equals(optional.get()); + } +} diff --git a/src/main/java/com/comphenix/protocol/wrappers/TroveWrapper.java b/src/main/java/com/comphenix/protocol/wrappers/TroveWrapper.java index c1eb558f..cab69cf1 100644 --- a/src/main/java/com/comphenix/protocol/wrappers/TroveWrapper.java +++ b/src/main/java/com/comphenix/protocol/wrappers/TroveWrapper.java @@ -4,6 +4,7 @@ import java.lang.reflect.InvocationTargetException; import java.lang.reflect.Method; import java.util.List; import java.util.Map; +import java.util.Optional; import java.util.Set; import javax.annotation.Nonnull; @@ -142,17 +143,11 @@ public class TroveWrapper { private static Object getDecorated(@Nonnull Object trove) { if (trove == null) throw new IllegalArgumentException("trove instance cannot be non-null."); - + AbstractFuzzyMatcher> match = FuzzyMatchers.matchSuper(trove.getClass()); - Class decorators = null; - - try { - // Attempt to get decorator class - decorators = getClassSource(trove.getClass()).loadClass("TDecorators"); - } catch (ClassNotFoundException e) { - throw new IllegalStateException(e.getMessage(), e); - } - + Class decorators = getClassSource(trove.getClass()).loadClass("TDecorator") + .orElseThrow(() -> new IllegalStateException("TDecorators class not found")); + // Find an appropriate wrapper method in TDecorators for (Method method : decorators.getMethods()) { Class[] types = method.getParameterTypes(); diff --git a/src/main/java/com/comphenix/protocol/wrappers/WrappedAttributeModifier.java b/src/main/java/com/comphenix/protocol/wrappers/WrappedAttributeModifier.java index 3d61fb39..c3f9fb46 100644 --- a/src/main/java/com/comphenix/protocol/wrappers/WrappedAttributeModifier.java +++ b/src/main/java/com/comphenix/protocol/wrappers/WrappedAttributeModifier.java @@ -22,7 +22,7 @@ import com.google.common.base.Preconditions; * @author Kristian */ public class WrappedAttributeModifier extends AbstractWrapper { - private static final boolean OPERATION_ENUM = MinecraftVersion.atOrAbove(MinecraftVersion.VILLAGE_UPDATE); + private static final boolean OPERATION_ENUM = MinecraftVersion.VILLAGE_UPDATE.atOrAbove(); private static final Class OPERATION_CLASS; private static final EquivalentConverter OPERATION_CONVERTER;