This is an automated email from the ASF dual-hosted git repository. rmannibucau pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/openwebbeans.git
The following commit(s) were added to refs/heads/master by this push: new e51e11e trying to optimize defineClass lookup strategies e51e11e is described below commit e51e11e61d99ef998f985623017dddb4893e203d Author: Romain Manni-Bucau <rmannibu...@gmail.com> AuthorDate: Tue Mar 16 11:41:25 2021 +0100 trying to optimize defineClass lookup strategies --- pom.xml | 2 + .../java/org/apache/webbeans/proxy/Unsafe.java | 244 ++++++++++++--------- 2 files changed, 144 insertions(+), 102 deletions(-) diff --git a/pom.xml b/pom.xml index 77503e5..1be053d 100644 --- a/pom.xml +++ b/pom.xml @@ -476,6 +476,7 @@ <include>**/*TestCase.java</include> <include>**/*Tests*.java</include> </includes> + <trimStackTrace>false</trimStackTrace> </configuration> </plugin> <!-- force generating a *-sources.jar when building a jar, e.g. for a snapshot release --> @@ -507,6 +508,7 @@ <configLocation>openwebbeans/owb-checks-default.xml</configLocation> <headerLocation>openwebbeans/owb-header.txt</headerLocation> <consoleOutput>true</consoleOutput> + <excludes>**/Unsafe*</excludes> <!-- we abuse of switch and checkstyle is broken for it --> </configuration> <dependencies> <dependency> diff --git a/webbeans-impl/src/main/java/org/apache/webbeans/proxy/Unsafe.java b/webbeans-impl/src/main/java/org/apache/webbeans/proxy/Unsafe.java index 7ba6d20..724002e 100644 --- a/webbeans-impl/src/main/java/org/apache/webbeans/proxy/Unsafe.java +++ b/webbeans-impl/src/main/java/org/apache/webbeans/proxy/Unsafe.java @@ -48,7 +48,7 @@ public class Unsafe private final AtomicReference<Method> unsafeDefineClass = new AtomicReference<>(); // defineClass method on ClassLoader - private volatile boolean useDefineClassMethod = true; + private volatile byte defineClassImpl = 0; // 0 = unset, 1 = classloader, 2 = lookup, 3 = unsafe (unlikely on all jvm) private volatile Method defineClassMethod = null; // java 16 @@ -146,131 +146,171 @@ public class Unsafe Class<?> parent) throws ProxyGenerationException { - - if (defineClassMethod == null && useDefineClassMethod) - { - Method defineClassMethodTmp = null; - try - { - // defineClass is a final method on the abstract base ClassLoader - // thus we need to cache it only once - defineClassMethodTmp = ClassLoader.class.getDeclaredMethod("defineClass", String.class, byte[].class, int.class, int.class); - } - catch (NoSuchMethodException e) - { - // all fine, we just skip over from here - } - - - if (defineClassMethodTmp == null) - { - // This ClassLoader does not have any accessible defineClass method - useDefineClassMethod = false; - } - else if (!defineClassMethodTmp.isAccessible()) - { - try - { - defineClassMethodTmp.setAccessible(true); - } - catch (RuntimeException re) - { - // likely j9 or not accessible via security, let's use unsafe or MethodHandle as fallbacks - defineClassMethodTmp = null; - useDefineClassMethod = false; - } - } - - defineClassMethod = defineClassMethodTmp; - } - + Class<?> definedClass = null; try { - Class<T> definedClass; - - if (defineClassMethod != null) - { - definedClass = (Class<T>) defineClassMethod.invoke(classLoader, proxyName, proxyBytes, 0, proxyBytes.length); - useDefineClassMethod = Boolean.TRUE; - } - else - { - definedClass = (Class<T>) unsafeDefineClass().invoke(internalUnsafe, proxyName, proxyBytes, 0, proxyBytes.length, classLoader, null); - } - - return (Class<T>) Class.forName(definedClass.getName(), true, classLoader); - } - catch (InvocationTargetException le) // if concurrent calls are done then ensure to just reload the created one - { - if (LinkageError.class.isInstance(le.getCause())) - { - try - { - return (Class<T>) Class.forName(proxyName.replace('/', '.'), true, classLoader); - } - catch (ClassNotFoundException e) + // CHECKSTYLE:OFF + switch (defineClassImpl) { + case 0: // unset + case 1: // classloader { - // default error handling + if (defineClassMethod == null) + { + Method defineClassMethodTmp; + try + { + // defineClass is a final method on the abstract base ClassLoader + // thus we need to cache it only once + defineClassMethodTmp = ClassLoader.class.getDeclaredMethod( + "defineClass", String.class, byte[].class, int.class, int.class); + if (!defineClassMethodTmp.isAccessible()) + { + try + { + defineClassMethodTmp.setAccessible(true); + defineClassMethod = defineClassMethodTmp; + } + catch (final RuntimeException re) + { + // likely j9 or not accessible via security, let's use unsafe or MethodHandle as fallbacks + } + } + } + catch (final NoSuchMethodException e) + { + // all fine, we just skip over from here + } + } + + if (defineClassMethod != null) + { + try + { + definedClass = Class.class.cast(defineClassMethod.invoke( + classLoader, proxyName, proxyBytes, 0, proxyBytes.length)); + defineClassImpl = 1; + break; + } + catch (final Throwable t) + { + definedClass = handleLinkageError(t, proxyName, classLoader); + if (definedClass != null) + { + defineClassImpl = 1; + break; + } + } + } } - } - throw onProxyGenerationError(le); - } - catch (Throwable e) - { - // we can also defineHiddenClass but what would be the real point? let's keep it simple for now - try - { - if (privateLookup == null) + case 2: // lookup { - synchronized (this) + if (privateLookup == null) + { + synchronized (this) + { + if (privateLookup == null) + { + try + { + lookup = MethodHandles.lookup(); + defineClass = lookup.getClass().getMethod("defineClass", byte[].class); + privateLookup = MethodHandles.class.getDeclaredMethod( + "privateLookupIn", Class.class, MethodHandles.Lookup.class); + } + catch (final Exception re) + { + // no-op + } + } + } + } + + if (privateLookup != null) { - if (privateLookup == null) + try + { + final MethodHandles.Lookup lookupInstance = MethodHandles.Lookup.class.cast( + privateLookup.invoke( + null, + proxyName.startsWith("org.apache.webbeans.custom.signed.") ? + CustomSignedProxyPackageMarker.class : + proxyName.startsWith("org.apache.webbeans.custom.") ? + CustomProxyPackageMarker.class : parent, + lookup)); + definedClass = (Class<T>) defineClass.invoke(lookupInstance, proxyBytes); + defineClassImpl = 2; + break; + } + catch (final Exception e) { - lookup = MethodHandles.lookup(); - privateLookup = MethodHandles.class.getDeclaredMethod( - "privateLookupIn", Class.class, MethodHandles.Lookup.class); - defineClass = lookup.getClass().getMethod("defineClass", byte[].class); + definedClass = handleLinkageError(e, proxyName, classLoader); + if (definedClass != null) + { + defineClassImpl = 2; + break; + } } } } - final MethodHandles.Lookup lookupInstance = MethodHandles.Lookup.class.cast( - privateLookup.invoke( - null, - proxyName.startsWith("org.apache.webbeans.custom.signed.") ? - CustomSignedProxyPackageMarker.class : - proxyName.startsWith("org.apache.webbeans.custom.") ? - CustomProxyPackageMarker.class : parent, - lookup)); - return (Class<T>) defineClass.invoke(lookupInstance, proxyBytes); - } - catch (final Exception exception) - { - if (LinkageError.class.isInstance(exception.getCause())) - { + case 3: // unlikely - unsafe try { - return (Class<T>) Class.forName(proxyName.replace('/', '.'), true, classLoader); + definedClass = Class.class.cast(unsafeDefineClass().invoke( + internalUnsafe, proxyName, proxyBytes, 0, proxyBytes.length, classLoader, null)); + defineClassImpl = 3; } - catch (ClassNotFoundException ignored) + catch (final Throwable t) { - // default error handling + definedClass = handleLinkageError(t, proxyName, classLoader); } - } - final ProxyGenerationException proxyGenerationException = onProxyGenerationError(e); - proxyGenerationException.addSuppressed(exception); - throw proxyGenerationException; + break; + default: + throw new IllegalAccessError("Unknown defineclass impl: " + defineClassImpl); + } + + // CHECKSTYLE:ON + if (definedClass == null) + { + throw new IllegalStateException("Can't define proxy " + proxyName); } + + return (Class<T>) Class.forName(definedClass.getName(), true, classLoader); + } + catch (final Throwable e) + { + return onProxyGenerationError(e, proxyName, classLoader); } } - private ProxyGenerationException onProxyGenerationError(final Throwable throwable) + private <T> Class<T> onProxyGenerationError(final Throwable throwable, final String name, final ClassLoader loader) { - return new ProxyGenerationException( + final Class<T> clazz = handleLinkageError(throwable, name, loader); + if (clazz != null) + { + return clazz; + } + throw new ProxyGenerationException( throwable.getMessage() + (isJava16OrMore() ? "\n" + - "On Java 16 ensure to set --add-exports java.base/jdk.internal.misc=ALL-UNNAMED on the JVM" : ""), + "On Java 16 you can set --add-exports java.base/jdk.internal.misc=ALL-UNNAMED on the JVM" : ""), throwable.getCause()); } + private <T> Class<T> handleLinkageError(final Throwable throwable, final String name, final ClassLoader loader) + { + if (LinkageError.class.isInstance(throwable) || LinkageError.class.isInstance(throwable.getCause())) + { + try + { + return (Class<T>) Class.forName(name.replace('/', '.'), true, loader); + } + catch (ClassNotFoundException e) + { + // default error handling + } + } + return null; + } + private boolean isJava16OrMore() { final String version = System.getProperty("java.version", "-1");