From 7d08b4fc357e12ee2487e87d8fdcbeee1152e5a0 Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Tue, 21 Feb 2017 20:33:11 +0100 Subject: [PATCH] Remove optional dependency on javassist Motivation: We shipped a javassist based implementation for typematching and logged a confusing debug message about missing javassist. We never were able to prove it really gives any perf improvements so we should just remove it. Modifications: - Remove javassist dependency and impl - Fix possible classloader deadlock as reported by intellij Result: Less code to maintain and less confusing log message. --- common/pom.xml | 7 -- ...avassistTypeParameterMatcherGenerator.java | 103 ------------------ .../util/internal/PlatformDependent.java | 36 ------ .../util/internal/TypeParameterMatcher.java | 26 ++--- 4 files changed, 8 insertions(+), 164 deletions(-) delete mode 100644 common/src/main/java/io/netty/util/internal/JavassistTypeParameterMatcherGenerator.java diff --git a/common/pom.xml b/common/pom.xml index a7cb0758ec..f54b86e39c 100644 --- a/common/pom.xml +++ b/common/pom.xml @@ -37,13 +37,6 @@ - - - org.javassist - javassist - compile - true - org.jctools jctools-core diff --git a/common/src/main/java/io/netty/util/internal/JavassistTypeParameterMatcherGenerator.java b/common/src/main/java/io/netty/util/internal/JavassistTypeParameterMatcherGenerator.java deleted file mode 100644 index 37ef6ea39a..0000000000 --- a/common/src/main/java/io/netty/util/internal/JavassistTypeParameterMatcherGenerator.java +++ /dev/null @@ -1,103 +0,0 @@ -/* - * Copyright 2013 The Netty Project - * - * The Netty Project licenses this file to you under the Apache License, - * version 2.0 (the "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at: - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT - * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the - * License for the specific language governing permissions and limitations - * under the License. - */ - -package io.netty.util.internal; - -import io.netty.util.internal.logging.InternalLogger; -import io.netty.util.internal.logging.InternalLoggerFactory; -import javassist.ClassClassPath; -import javassist.ClassPath; -import javassist.ClassPool; -import javassist.CtClass; -import javassist.Modifier; -import javassist.NotFoundException; - -import java.lang.reflect.Method; - -public final class JavassistTypeParameterMatcherGenerator { - - private static final InternalLogger logger = - InternalLoggerFactory.getInstance(JavassistTypeParameterMatcherGenerator.class); - - private static final ClassPool classPool = new ClassPool(true); - - static { - classPool.appendClassPath(new ClassClassPath(NoOpTypeParameterMatcher.class)); - } - - public static void appendClassPath(ClassPath classpath) { - classPool.appendClassPath(classpath); - } - - public static void appendClassPath(String pathname) throws NotFoundException { - classPool.appendClassPath(pathname); - } - - public static ClassPool classPool() { - return classPool; - } - - public static TypeParameterMatcher generate(Class type) { - ClassLoader classLoader = PlatformDependent.getContextClassLoader(); - if (classLoader == null) { - classLoader = PlatformDependent.getSystemClassLoader(); - } - return generate(type, classLoader); - } - - public static TypeParameterMatcher generate(Class type, ClassLoader classLoader) { - final String typeName = typeName(type); - final String className = "io.netty.util.internal.__matchers__." + typeName + "Matcher"; - try { - try { - return (TypeParameterMatcher) Class.forName(className, true, classLoader).newInstance(); - } catch (Exception e) { - // Not defined in the specified class loader. - } - - CtClass c = classPool.getAndRename(NoOpTypeParameterMatcher.class.getName(), className); - c.setModifiers(c.getModifiers() | Modifier.FINAL); - c.getDeclaredMethod("match").setBody("{ return $1 instanceof " + typeName + "; }"); - byte[] byteCode = c.toBytecode(); - c.detach(); - Method method = ClassLoader.class.getDeclaredMethod( - "defineClass", String.class, byte[].class, int.class, int.class); - method.setAccessible(true); - - Class generated = (Class) method.invoke(classLoader, className, byteCode, 0, byteCode.length); - if (type != Object.class) { - logger.debug("Generated: {}", generated.getName()); - } else { - // Object.class is only used when checking if Javassist is available. - } - return (TypeParameterMatcher) generated.newInstance(); - } catch (RuntimeException e) { - throw e; - } catch (Exception e) { - throw new RuntimeException(e); - } - } - - private static String typeName(Class type) { - if (type.isArray()) { - return typeName(type.getComponentType()) + "[]"; - } - - return type.getName(); - } - - private JavassistTypeParameterMatcherGenerator() { } -} diff --git a/common/src/main/java/io/netty/util/internal/PlatformDependent.java b/common/src/main/java/io/netty/util/internal/PlatformDependent.java index 1b2ebfb149..012f2eae24 100644 --- a/common/src/main/java/io/netty/util/internal/PlatformDependent.java +++ b/common/src/main/java/io/netty/util/internal/PlatformDependent.java @@ -98,8 +98,6 @@ public final class PlatformDependent { private static final long BYTE_ARRAY_BASE_OFFSET = byteArrayBaseOffset0(); - private static final boolean HAS_JAVASSIST = hasJavassist0(); - private static final File TMPDIR = tmpdir0(); private static final int BIT_MODE = bitMode0(); @@ -243,13 +241,6 @@ public final class PlatformDependent { return MAX_DIRECT_MEMORY; } - /** - * Returns {@code true} if and only if Javassist is available. - */ - public static boolean hasJavassist() { - return HAS_JAVASSIST; - } - /** * Returns the temporary directory. */ @@ -1174,33 +1165,6 @@ public final class PlatformDependent { return maxDirectMemory; } - private static boolean hasJavassist0() { - if (isAndroid()) { - return false; - } - - boolean noJavassist = SystemPropertyUtil.getBoolean("io.netty.noJavassist", false); - logger.debug("-Dio.netty.noJavassist: {}", noJavassist); - - if (noJavassist) { - logger.debug("Javassist: unavailable (io.netty.noJavassist)"); - return false; - } - - try { - JavassistTypeParameterMatcherGenerator.generate(Object.class, getClassLoader(PlatformDependent.class)); - logger.debug("Javassist: available"); - return true; - } catch (Throwable t) { - // Failed to generate a Javassist-based matcher. - logger.debug("Javassist: unavailable"); - logger.debug( - "You don't have Javassist in your class path or you don't have enough permission " + - "to load dynamically generated classes. Please check the configuration for better performance."); - return false; - } - } - private static File tmpdir0() { File f; try { diff --git a/common/src/main/java/io/netty/util/internal/TypeParameterMatcher.java b/common/src/main/java/io/netty/util/internal/TypeParameterMatcher.java index a10b149d78..f76e6719f3 100644 --- a/common/src/main/java/io/netty/util/internal/TypeParameterMatcher.java +++ b/common/src/main/java/io/netty/util/internal/TypeParameterMatcher.java @@ -26,8 +26,12 @@ import java.util.Map; public abstract class TypeParameterMatcher { - private static final TypeParameterMatcher NOOP = new NoOpTypeParameterMatcher(); - private static final Object TEST_OBJECT = new Object(); + private static final TypeParameterMatcher NOOP = new TypeParameterMatcher() { + @Override + public boolean match(Object msg) { + return true; + } + }; public static TypeParameterMatcher get(final Class parameterType) { final Map, TypeParameterMatcher> getCache = @@ -37,23 +41,9 @@ public abstract class TypeParameterMatcher { if (matcher == null) { if (parameterType == Object.class) { matcher = NOOP; - } else if (PlatformDependent.hasJavassist()) { - try { - matcher = JavassistTypeParameterMatcherGenerator.generate(parameterType); - matcher.match(TEST_OBJECT); - } catch (IllegalAccessError e) { - // Happens if parameterType is not public. - matcher = null; - } catch (Exception e) { - // Will not usually happen, but just in case. - matcher = null; - } - } - - if (matcher == null) { + } else { matcher = new ReflectiveMatcher(parameterType); } - getCache.put(parameterType, matcher); } @@ -172,5 +162,5 @@ public abstract class TypeParameterMatcher { } } - protected TypeParameterMatcher() { } + TypeParameterMatcher() { } }