From ba551b09e4c873f30c0675193e70e0a0eb62c3ca Mon Sep 17 00:00:00 2001 From: Andy Clement Date: Thu, 28 Sep 2017 16:03:17 -0700 Subject: [PATCH] Fixes Bug 525293 - Spring AOP could be faster Multiple changes here: - annotation unpacking is smarter and if it only needs runtime retention annotations it uses reflection and doesn't unpack the bytes to discover class level retention annotations. - Reflection worlds are shared if for the same classloader. --- .../bcel/util/ClassLoaderRepository.java | 8 +- .../weaver/reflect/AnnotationFinder.java | 16 +- .../ReflectionBasedResolvedMemberImpl.java | 105 +++---------- .../weaver/reflect/ReflectionWorld.java | 52 ++++++- .../aspectj/weaver/tools/PointcutParser.java | 41 +++-- .../reflect/Java15AnnotationFinder.java | 142 +++++++++--------- .../weaver/reflect/ReflectionWorldTest.java | 70 ++++++++- 7 files changed, 242 insertions(+), 192 deletions(-) diff --git a/bcel-builder/src/org/aspectj/apache/bcel/util/ClassLoaderRepository.java b/bcel-builder/src/org/aspectj/apache/bcel/util/ClassLoaderRepository.java index eb33d8b4e..e3c59556b 100644 --- a/bcel-builder/src/org/aspectj/apache/bcel/util/ClassLoaderRepository.java +++ b/bcel-builder/src/org/aspectj/apache/bcel/util/ClassLoaderRepository.java @@ -88,15 +88,13 @@ public class ClassLoaderRepository implements Repository { private ClassLoaderReference loaderRef; // Choice of cache... - private WeakHashMap /* */> localCache = new WeakHashMap>(); - private static SoftHashMap /* */sharedCache = new SoftHashMap(Collections - .synchronizedMap(new HashMap())); + private WeakHashMap> localCache = new WeakHashMap>(); + private static SoftHashMap /* */sharedCache = new SoftHashMap(Collections.synchronizedMap(new HashMap())); // For fast translation of the classname *intentionally not static* private SoftHashMap /* */nameMap = new SoftHashMap(new HashMap(), false); - public static boolean useSharedCache = System.getProperty("org.aspectj.apache.bcel.useSharedCache", "true").equalsIgnoreCase( - "true"); + public static boolean useSharedCache = System.getProperty("org.aspectj.apache.bcel.useSharedCache", "true").equalsIgnoreCase("true"); private static int cacheHitsShared = 0; private static int missSharedEvicted = 0; // Misses in shared cache access due to reference GC diff --git a/org.aspectj.matcher/src/org/aspectj/weaver/reflect/AnnotationFinder.java b/org.aspectj.matcher/src/org/aspectj/weaver/reflect/AnnotationFinder.java index 7efeac9ae..90ce368d9 100644 --- a/org.aspectj.matcher/src/org/aspectj/weaver/reflect/AnnotationFinder.java +++ b/org.aspectj.matcher/src/org/aspectj/weaver/reflect/AnnotationFinder.java @@ -1,18 +1,16 @@ /* ******************************************************************* - * Copyright (c) 2005 Contributors. + * Copyright (c) 2005, 2017 Contributors. + * * All rights reserved. * This program and the accompanying materials are made available * under the terms of the Eclipse Public License v1.0 * which accompanies this distribution and is available at * http://eclipse.org/legal/epl-v10.html * - * Contributors: - * Adrian Colyer Initial implementation * ******************************************************************/ package org.aspectj.weaver.reflect; import java.lang.reflect.Member; -import java.util.Set; import org.aspectj.weaver.AnnotationAJ; import org.aspectj.weaver.ResolvedType; @@ -20,7 +18,8 @@ import org.aspectj.weaver.UnresolvedType; import org.aspectj.weaver.World; /** - * @author colyer Used in 1.4 code to access annotations safely + * @author Adrian Colyer + * @author Andy Clement */ public interface AnnotationFinder { @@ -32,14 +31,13 @@ public interface AnnotationFinder { Object getAnnotationFromMember(ResolvedType annotationType, Member aMember); - public AnnotationAJ getAnnotationOfType(UnresolvedType ofType, - Member onMember); + public AnnotationAJ getAnnotationOfType(UnresolvedType ofType, Member onMember); public String getAnnotationDefaultValue(Member onMember); - Object getAnnotationFromClass(ResolvedType annotationType, Class aClass); + Object getAnnotationFromClass(ResolvedType annotationType, Class aClass); - Set/* ResolvedType */getAnnotations(Member onMember); + ResolvedType[] getAnnotations(Member onMember, boolean runtimeAnnotationsOnly); ResolvedType[][] getParameterAnnotationTypes(Member onMember); } diff --git a/org.aspectj.matcher/src/org/aspectj/weaver/reflect/ReflectionBasedResolvedMemberImpl.java b/org.aspectj.matcher/src/org/aspectj/weaver/reflect/ReflectionBasedResolvedMemberImpl.java index 28c8aacf6..ba8f1330e 100644 --- a/org.aspectj.matcher/src/org/aspectj/weaver/reflect/ReflectionBasedResolvedMemberImpl.java +++ b/org.aspectj.matcher/src/org/aspectj/weaver/reflect/ReflectionBasedResolvedMemberImpl.java @@ -1,18 +1,15 @@ /* ******************************************************************* - * Copyright (c) 2005 Contributors. + * Copyright (c) 2005, 2017 Contributors. * All rights reserved. * This program and the accompanying materials are made available * under the terms of the Eclipse Public License v1.0 * which accompanies this distribution and is available at * http://eclipse.org/legal/epl-v10.html * - * Contributors: - * Adrian Colyer Initial implementation * ******************************************************************/ package org.aspectj.weaver.reflect; import java.lang.reflect.Member; -import java.util.Set; import org.aspectj.weaver.AnnotationAJ; import org.aspectj.weaver.MemberKind; @@ -24,37 +21,29 @@ import org.aspectj.weaver.UnresolvedType; /** * Subtype of ResolvedMemberImpl used in reflection world. Knows how to get annotations from a java.lang.reflect.Member * + * @author Adrian Colyer + * @author Andy Clement */ public class ReflectionBasedResolvedMemberImpl extends ResolvedMemberImpl { private AnnotationFinder annotationFinder = null; private GenericSignatureInformationProvider gsigInfoProvider = new Java14GenericSignatureInformationProvider(); + + /** + * If true then only runtime visible annotations have been resolved via reflection. If class retention + * annotations are also required (later) then the cache will have to be rebuilt using a more detailed + * dig into the class file. + */ + private boolean onlyRuntimeAnnotationsCached; private Member reflectMember; - /** - * @param kind - * @param declaringType - * @param modifiers - * @param returnType - * @param name - * @param parameterTypes - */ public ReflectionBasedResolvedMemberImpl(MemberKind kind, UnresolvedType declaringType, int modifiers, UnresolvedType returnType, String name, UnresolvedType[] parameterTypes, Member reflectMember) { super(kind, declaringType, modifiers, returnType, name, parameterTypes); this.reflectMember = reflectMember; } - /** - * @param kind - * @param declaringType - * @param modifiers - * @param returnType - * @param name - * @param parameterTypes - * @param checkedExceptions - */ public ReflectionBasedResolvedMemberImpl(MemberKind kind, UnresolvedType declaringType, int modifiers, UnresolvedType returnType, String name, UnresolvedType[] parameterTypes, UnresolvedType[] checkedExceptions, Member reflectMember) { @@ -62,16 +51,6 @@ public class ReflectionBasedResolvedMemberImpl extends ResolvedMemberImpl { this.reflectMember = reflectMember; } - /** - * @param kind - * @param declaringType - * @param modifiers - * @param returnType - * @param name - * @param parameterTypes - * @param checkedExceptions - * @param backingGenericMember - */ public ReflectionBasedResolvedMemberImpl(MemberKind kind, UnresolvedType declaringType, int modifiers, UnresolvedType returnType, String name, UnresolvedType[] parameterTypes, UnresolvedType[] checkedExceptions, ResolvedMember backingGenericMember, Member reflectMember) { @@ -79,13 +58,6 @@ public class ReflectionBasedResolvedMemberImpl extends ResolvedMemberImpl { this.reflectMember = reflectMember; } - /** - * @param kind - * @param declaringType - * @param modifiers - * @param name - * @param signature - */ public ReflectionBasedResolvedMemberImpl(MemberKind kind, UnresolvedType declaringType, int modifiers, String name, String signature, Member reflectMember) { super(kind, declaringType, modifiers, name, signature); @@ -96,89 +68,64 @@ public class ReflectionBasedResolvedMemberImpl extends ResolvedMemberImpl { return this.reflectMember; } - // generic signature support - public void setGenericSignatureInformationProvider(GenericSignatureInformationProvider gsigProvider) { this.gsigInfoProvider = gsigProvider; } - /* - * (non-Javadoc) - * - * @see org.aspectj.weaver.ResolvedMemberImpl#getGenericParameterTypes() - */ @Override public UnresolvedType[] getGenericParameterTypes() { return this.gsigInfoProvider.getGenericParameterTypes(this); } - /* - * (non-Javadoc) - * - * @see org.aspectj.weaver.ResolvedMemberImpl#getGenericReturnType() - */ @Override public UnresolvedType getGenericReturnType() { return this.gsigInfoProvider.getGenericReturnType(this); } - /* - * (non-Javadoc) - * - * @see org.aspectj.weaver.ResolvedMemberImpl#isSynthetic() - */ @Override public boolean isSynthetic() { return this.gsigInfoProvider.isSynthetic(this); } - /* - * (non-Javadoc) - * - * @see org.aspectj.weaver.ResolvedMemberImpl#isVarargsMethod() - */ @Override public boolean isVarargsMethod() { return this.gsigInfoProvider.isVarArgs(this); } - /* - * (non-Javadoc) - * - * @see org.aspectj.weaver.ResolvedMemberImpl#isBridgeMethod() - */ @Override public boolean isBridgeMethod() { return this.gsigInfoProvider.isBridge(this); } - // annotation support - public void setAnnotationFinder(AnnotationFinder finder) { this.annotationFinder = finder; } @Override public boolean hasAnnotation(UnresolvedType ofType) { - unpackAnnotations(); + boolean areRuntimeRetentionAnnotationsSufficient = false; + if (ofType instanceof ResolvedType) { + areRuntimeRetentionAnnotationsSufficient = ((ResolvedType)ofType).isAnnotationWithRuntimeRetention(); + } + unpackAnnotations(areRuntimeRetentionAnnotationsSufficient); return super.hasAnnotation(ofType); } @Override public boolean hasAnnotations() { - unpackAnnotations(); + unpackAnnotations(false); return super.hasAnnotations(); } @Override public ResolvedType[] getAnnotationTypes() { - unpackAnnotations(); + unpackAnnotations(false); return super.getAnnotationTypes(); } @Override public AnnotationAJ getAnnotationOfType(UnresolvedType ofType) { - unpackAnnotations(); + unpackAnnotations(false); if (annotationFinder == null || annotationTypes == null) { return null; } @@ -206,18 +153,10 @@ public class ReflectionBasedResolvedMemberImpl extends ResolvedMemberImpl { return parameterAnnotationTypes; } - private void unpackAnnotations() { - if (annotationTypes == null && annotationFinder != null) { - Set s = annotationFinder.getAnnotations(reflectMember); - if (s.size() == 0) { - annotationTypes = ResolvedType.EMPTY_ARRAY; - } else { - annotationTypes = new ResolvedType[s.size()]; - int i = 0; - for (Object o : s) { - annotationTypes[i++] = (ResolvedType) o; - } - } + private void unpackAnnotations(boolean areRuntimeRetentionAnnotationsSufficient) { + if (annotationFinder != null && (annotationTypes == null || (!areRuntimeRetentionAnnotationsSufficient && onlyRuntimeAnnotationsCached))) { + annotationTypes = annotationFinder.getAnnotations(reflectMember, areRuntimeRetentionAnnotationsSufficient); + onlyRuntimeAnnotationsCached = areRuntimeRetentionAnnotationsSufficient; } } } diff --git a/org.aspectj.matcher/src/org/aspectj/weaver/reflect/ReflectionWorld.java b/org.aspectj.matcher/src/org/aspectj/weaver/reflect/ReflectionWorld.java index 98e800222..d06db52d1 100644 --- a/org.aspectj.matcher/src/org/aspectj/weaver/reflect/ReflectionWorld.java +++ b/org.aspectj.matcher/src/org/aspectj/weaver/reflect/ReflectionWorld.java @@ -1,17 +1,16 @@ /* ******************************************************************* - * Copyright (c) 2005 Contributors. + * Copyright (c) 2005-2017 Contributors. * All rights reserved. * This program and the accompanying materials are made available * under the terms of the Eclipse Public License v1.0 * which accompanies this distribution and is available at * http://eclipse.org/legal/epl-v10.html - * - * Contributors: - * Adrian Colyer Initial implementation * ******************************************************************/ package org.aspectj.weaver.reflect; +import java.util.Collections; import java.util.HashMap; +import java.util.Iterator; import java.util.Map; import org.aspectj.bridge.AbortException; @@ -31,14 +30,46 @@ import org.aspectj.weaver.World; * A ReflectionWorld is used solely for purposes of type resolution based on the runtime classpath (java.lang.reflect). It does not * support weaving operations (creation of mungers etc..). * + * @author Adrian Colyer + * @author Andy Clement */ public class ReflectionWorld extends World implements IReflectionWorld { + private static Map rworlds = Collections.synchronizedMap(new HashMap()); + private WeakClassLoaderReference classLoaderReference; private AnnotationFinder annotationFinder; private boolean mustUseOneFourDelegates = false; // for testing private Map> inProgressResolutionClasses = new HashMap>(); - + + public static ReflectionWorld getReflectionWorldFor(WeakClassLoaderReference classLoaderReference) { + synchronized (rworlds) { + // Tidyup any no longer relevant entries... + for (Iterator> it = rworlds.entrySet().iterator(); + it.hasNext();) { + Map.Entry entry = it.next(); + if (entry.getKey().getClassLoader() == null) { + it.remove(); + } + } + ReflectionWorld rworld = null; + if (classLoaderReference.getClassLoader() != null) { + rworld = rworlds.get(classLoaderReference); + if (rworld == null) { + rworld = new ReflectionWorld(classLoaderReference); + rworlds.put(classLoaderReference, rworld); + } + } + return rworld; + } + } + + public static void cleanUpWorlds() { + synchronized (rworlds) { + rworlds.clear(); + } + } + private ReflectionWorld() { // super(); // this.setMessageHandler(new ExceptionBasedMessageHandler()); @@ -49,6 +80,13 @@ public class ReflectionWorld extends World implements IReflectionWorld { // makeAnnotationFinderIfAny(classLoaderReference.getClassLoader(), // this); } + + public ReflectionWorld(WeakClassLoaderReference classloaderRef) { + this.setMessageHandler(new ExceptionBasedMessageHandler()); + setBehaveInJava5Way(LangUtil.is15VMOrGreater()); + classLoaderReference = classloaderRef; + annotationFinder = makeAnnotationFinderIfAny(classLoaderReference.getClassLoader(), this); + } public ReflectionWorld(ClassLoader aClassLoader) { super(); @@ -71,7 +109,7 @@ public class ReflectionWorld extends World implements IReflectionWorld { AnnotationFinder annotationFinder = null; try { if (LangUtil.is15VMOrGreater()) { - Class java15AnnotationFinder = Class.forName("org.aspectj.weaver.reflect.Java15AnnotationFinder"); + Class java15AnnotationFinder = Class.forName("org.aspectj.weaver.reflect.Java15AnnotationFinder"); annotationFinder = (AnnotationFinder) java15AnnotationFinder.newInstance(); annotationFinder.setClassLoader(loader); annotationFinder.setWorld(world); @@ -99,7 +137,7 @@ public class ReflectionWorld extends World implements IReflectionWorld { return resolve(this, aClass); } - public static ResolvedType resolve(World world, Class aClass) { + public static ResolvedType resolve(World world, Class aClass) { // classes that represent arrays return a class name that is the // signature of the array type, ho-hum... String className = aClass.getName(); diff --git a/org.aspectj.matcher/src/org/aspectj/weaver/tools/PointcutParser.java b/org.aspectj.matcher/src/org/aspectj/weaver/tools/PointcutParser.java index 649aa0ba9..4e3b2bd59 100644 --- a/org.aspectj.matcher/src/org/aspectj/weaver/tools/PointcutParser.java +++ b/org.aspectj.matcher/src/org/aspectj/weaver/tools/PointcutParser.java @@ -1,5 +1,5 @@ /******************************************************************************* - * Copyright (c) 2004 IBM Corporation and others. + * Copyright (c) 2004, 2017 Contributors * All rights reserved. This program and the accompanying materials * are made available under the terms of the Eclipse Public License v1.0 * which accompanies this distribution, and is available at @@ -51,6 +51,9 @@ import org.aspectj.weaver.reflect.ReflectionWorld; /** * A PointcutParser can be used to build PointcutExpressions for a user-defined subset of AspectJ's pointcut language + * + * @author Adrian Colyer + * @author Andy Clement */ public class PointcutParser { @@ -123,7 +126,7 @@ public class PointcutParser { * @throws UnsupportedOperationException if the set contains if, cflow, or cflow below */ public static PointcutParser getPointcutParserSupportingSpecifiedPrimitivesAndUsingContextClassloaderForResolution( - Set supportedPointcutKinds) { + Set supportedPointcutKinds) { PointcutParser p = new PointcutParser(supportedPointcutKinds); p.setClassLoader(Thread.currentThread().getContextClassLoader()); return p; @@ -163,7 +166,7 @@ public class PointcutParser { * @throws UnsupportedOperationException if the set contains if, cflow, or cflow below */ public static PointcutParser getPointcutParserSupportingSpecifiedPrimitivesAndUsingSpecifiedClassLoaderForResolution( - Set supportedPointcutKinds, ClassLoader classLoader) { + Set supportedPointcutKinds, ClassLoader classLoader) { PointcutParser p = new PointcutParser(supportedPointcutKinds); p.setClassLoader(classLoader); return p; @@ -216,7 +219,22 @@ public class PointcutParser { */ protected void setClassLoader(ClassLoader aLoader) { this.classLoaderReference = new WeakClassLoaderReference(aLoader); - world = new ReflectionWorld(this.classLoaderReference.getClassLoader()); + world = ReflectionWorld.getReflectionWorldFor(this.classLoaderReference); + } + + /** + * Set the classloader that this parser should use for type resolution. + * + * @param aLoader + * @param shareWorlds if true then two PointcutParsers operating using the same classloader will share a ReflectionWorld + */ + protected void setClassLoader(ClassLoader aLoader, boolean shareWorlds) { + this.classLoaderReference = new WeakClassLoaderReference(aLoader); + if (shareWorlds) { + world = ReflectionWorld.getReflectionWorldFor(this.classLoaderReference); + } else { + world = new ReflectionWorld(classLoaderReference); + } } /** @@ -261,7 +279,7 @@ public class PointcutParser { * @param type * @return */ - public PointcutParameter createPointcutParameter(String name, Class type) { + public PointcutParameter createPointcutParameter(String name, Class type) { return new PointcutParameterImpl(name, type); } @@ -287,7 +305,7 @@ public class PointcutParser { * supported by this PointcutParser. * @throws IllegalArgumentException if the expression is not a well-formed pointcut expression */ - public PointcutExpression parsePointcutExpression(String expression, Class inScope, PointcutParameter[] formalParameters) + public PointcutExpression parsePointcutExpression(String expression, Class inScope, PointcutParameter[] formalParameters) throws UnsupportedPointcutPrimitiveException, IllegalArgumentException { PointcutExpressionImpl pcExpr = null; try { @@ -303,7 +321,7 @@ public class PointcutParser { return pcExpr; } - protected Pointcut resolvePointcutExpression(String expression, Class inScope, PointcutParameter[] formalParameters) { + protected Pointcut resolvePointcutExpression(String expression, Class inScope, PointcutParameter[] formalParameters) { try { PatternParser parser = new PatternParser(expression); parser.setPointcutDesignatorHandlers(pointcutDesignators, world); @@ -317,7 +335,7 @@ public class PointcutParser { } } - protected Pointcut concretizePointcutExpression(Pointcut pc, Class inScope, PointcutParameter[] formalParameters) { + protected Pointcut concretizePointcutExpression(Pointcut pc, Class inScope, PointcutParameter[] formalParameters) { ResolvedType declaringTypeForResolution = null; if (inScope != null) { declaringTypeForResolution = getWorld().resolve(inScope.getName()); @@ -355,7 +373,7 @@ public class PointcutParser { } /* for testing */ - Set getSupportedPrimitives() { + Set getSupportedPrimitives() { return supportedPrimitives; } @@ -366,7 +384,7 @@ public class PointcutParser { return current; } - private IScope buildResolutionScope(Class inScope, PointcutParameter[] formalParameters) { + private IScope buildResolutionScope(Class inScope, PointcutParameter[] formalParameters) { if (formalParameters == null) { formalParameters = new PointcutParameter[0]; } @@ -398,7 +416,7 @@ public class PointcutParser { } } - private UnresolvedType toUnresolvedType(Class clazz) { + private UnresolvedType toUnresolvedType(Class clazz) { if (clazz.isArray()) { return UnresolvedType.forSignature(clazz.getName().replace('.', '/')); } else { @@ -560,4 +578,5 @@ public class PointcutParser { msg.append("\n"); return msg.toString(); } + } diff --git a/weaver5/java5-src/org/aspectj/weaver/reflect/Java15AnnotationFinder.java b/weaver5/java5-src/org/aspectj/weaver/reflect/Java15AnnotationFinder.java index a978d9605..016becf87 100644 --- a/weaver5/java5-src/org/aspectj/weaver/reflect/Java15AnnotationFinder.java +++ b/weaver5/java5-src/org/aspectj/weaver/reflect/Java15AnnotationFinder.java @@ -1,13 +1,10 @@ /* ******************************************************************* - * Copyright (c) 2005 Contributors. + * Copyright (c) 2005, 2017 Contributors. * All rights reserved. * This program and the accompanying materials are made available * under the terms of the Eclipse Public License v1.0 * which accompanies this distribution and is available at * http://eclipse.org/legal/epl-v10.html - * - * Contributors: - * Adrian Colyer Initial implementation * ******************************************************************/ package org.aspectj.weaver.reflect; @@ -17,15 +14,13 @@ import java.lang.reflect.Constructor; import java.lang.reflect.Field; import java.lang.reflect.Member; import java.lang.reflect.Method; -import java.util.Collections; -import java.util.HashSet; -import java.util.Set; import org.aspectj.apache.bcel.classfile.AnnotationDefault; import org.aspectj.apache.bcel.classfile.Attribute; import org.aspectj.apache.bcel.classfile.JavaClass; import org.aspectj.apache.bcel.classfile.LocalVariable; import org.aspectj.apache.bcel.classfile.LocalVariableTable; +import org.aspectj.apache.bcel.util.ClassLoaderRepository; import org.aspectj.apache.bcel.util.NonCachingClassLoaderRepository; import org.aspectj.apache.bcel.util.Repository; import org.aspectj.weaver.AnnotationAJ; @@ -36,36 +31,44 @@ import org.aspectj.weaver.bcel.BcelAnnotation; import org.aspectj.weaver.bcel.BcelWeakClassLoaderReference; /** - * Find the given annotation (if present) on the given object * + * @author Adrian Colyer + * @author Andy Clement */ public class Java15AnnotationFinder implements AnnotationFinder, ArgNameFinder { + public static final ResolvedType[][] NO_PARAMETER_ANNOTATIONS = new ResolvedType[][] {}; + private Repository bcelRepository; private BcelWeakClassLoaderReference classLoaderRef; private World world; + private static boolean useCachingClassLoaderRepository; + + static { + try { + useCachingClassLoaderRepository = System.getProperty("Xset:bcelRepositoryCaching","true").equalsIgnoreCase("true"); + } catch (Throwable t) { + useCachingClassLoaderRepository = false; + } + } // must have no-arg constructor for reflective construction public Java15AnnotationFinder() { } public void setClassLoader(ClassLoader aLoader) { - // TODO: No easy way to ask the world factory for the right kind of - // repository so - // default to the safe one! (pr160674) this.classLoaderRef = new BcelWeakClassLoaderReference(aLoader); - this.bcelRepository = new NonCachingClassLoaderRepository(classLoaderRef); + if (useCachingClassLoaderRepository) { + this.bcelRepository = new ClassLoaderRepository(classLoaderRef); + } else { + this.bcelRepository = new NonCachingClassLoaderRepository(classLoaderRef); + } } public void setWorld(World aWorld) { this.world = aWorld; } - /* - * (non-Javadoc) - * - * @see org.aspectj.weaver.reflect.AnnotationFinder#getAnnotation(org.aspectj .weaver.ResolvedType, java.lang.Object) - */ public Object getAnnotation(ResolvedType annotationType, Object onObject) { try { Class annotationClass = (Class) Class.forName(annotationType.getName(), @@ -155,7 +158,6 @@ public class Java15AnnotationFinder implements AnnotationFinder, ArgNameFinder { } catch (ClassNotFoundException cnfEx) { // just use reflection then } - return null; } @@ -186,80 +188,76 @@ public class Java15AnnotationFinder implements AnnotationFinder, ArgNameFinder { } catch (ClassNotFoundException cnfEx) { // just use reflection then } - return null; } - public Set getAnnotations(Member onMember) { - if (!(onMember instanceof AccessibleObject)) - return Collections.EMPTY_SET; - // here we really want both the runtime visible AND the class visible - // annotations - // so we bail out to Bcel and then chuck away the JavaClass so that we - // don't hog - // memory. - try { - JavaClass jc = bcelRepository.loadClass(onMember.getDeclaringClass()); - org.aspectj.apache.bcel.classfile.annotation.AnnotationGen[] anns = new org.aspectj.apache.bcel.classfile.annotation.AnnotationGen[0]; - if (onMember instanceof Method) { - org.aspectj.apache.bcel.classfile.Method bcelMethod = jc.getMethod((Method) onMember); - if (bcelMethod == null) { - // fallback on reflection - see pr220430 - // System.err.println( - // "Unexpected problem in Java15AnnotationFinder: cannot retrieve annotations on method '" - // + - // onMember.getName()+"' in class '"+jc.getClassName()+"'"); - } else { - anns = bcelMethod.getAnnotations(); + public ResolvedType[] getAnnotations(Member onMember, boolean areRuntimeAnnotationsSufficient) { + if (!(onMember instanceof AccessibleObject)) { + return ResolvedType.NONE; + } + // If annotations with class level retention are required then we need to open + // open the class file. If only runtime retention annotations are required + // we can just use reflection. + if (!areRuntimeAnnotationsSufficient) { + try { + JavaClass jc = bcelRepository.loadClass(onMember.getDeclaringClass()); + org.aspectj.apache.bcel.classfile.annotation.AnnotationGen[] anns = null; + if (onMember instanceof Method) { + org.aspectj.apache.bcel.classfile.Method bcelMethod = jc.getMethod((Method) onMember); + if (bcelMethod != null) { + anns = bcelMethod.getAnnotations(); + } + } else if (onMember instanceof Constructor) { + org.aspectj.apache.bcel.classfile.Method bcelCons = jc.getMethod((Constructor) onMember); + anns = bcelCons.getAnnotations(); + } else if (onMember instanceof Field) { + org.aspectj.apache.bcel.classfile.Field bcelField = jc.getField((Field) onMember); + anns = bcelField.getAnnotations(); } - } else if (onMember instanceof Constructor) { - org.aspectj.apache.bcel.classfile.Method bcelCons = jc.getMethod((Constructor) onMember); - anns = bcelCons.getAnnotations(); - } else if (onMember instanceof Field) { - org.aspectj.apache.bcel.classfile.Field bcelField = jc.getField((Field) onMember); - anns = bcelField.getAnnotations(); - } - // the answer is cached and we don't want to hold on to memory - bcelRepository.clear(); - // OPTIMIZE make this a constant 0 size array - if (anns == null) - anns = new org.aspectj.apache.bcel.classfile.annotation.AnnotationGen[0]; - // convert to our Annotation type - Set annSet = new HashSet(); - for (int i = 0; i < anns.length; i++) { - annSet.add(world.resolve(UnresolvedType.forSignature(anns[i].getTypeSignature()))); + // the answer is cached and we don't want to hold on to memory + bcelRepository.clear(); + if (anns == null || anns.length == 0) { + return ResolvedType.NONE; + } + ResolvedType[] annotationTypes = new ResolvedType[anns.length]; + for (int i = 0; i < anns.length; i++) { + annotationTypes[i] = world.resolve(UnresolvedType.forSignature(anns[i].getTypeSignature())); + } + return annotationTypes; + } catch (ClassNotFoundException cnfEx) { + // just use reflection then } - return annSet; - } catch (ClassNotFoundException cnfEx) { - // just use reflection then } AccessibleObject ao = (AccessibleObject) onMember; Annotation[] anns = ao.getDeclaredAnnotations(); - Set annSet = new HashSet(); + if (anns.length == 0) { + return ResolvedType.NONE; + } + ResolvedType[] annotationTypes = new ResolvedType[anns.length]; for (int i = 0; i < anns.length; i++) { - annSet.add(UnresolvedType.forName(anns[i].annotationType().getName()).resolve(world)); + annotationTypes[i] = UnresolvedType.forName(anns[i].annotationType().getName()).resolve(world); } - return annSet; + return annotationTypes; } public ResolvedType[] getAnnotations(Class forClass, World inWorld) { // here we really want both the runtime visible AND the class visible - // annotations - // so we bail out to Bcel and then chuck away the JavaClass so that we - // don't hog - // memory. + // annotations so we bail out to Bcel and then chuck away the JavaClass so that we + // don't hog memory. try { JavaClass jc = bcelRepository.loadClass(forClass); org.aspectj.apache.bcel.classfile.annotation.AnnotationGen[] anns = jc.getAnnotations(); bcelRepository.clear(); - if (anns == null) + if (anns == null) { return ResolvedType.NONE; - ResolvedType[] ret = new ResolvedType[anns.length]; - for (int i = 0; i < ret.length; i++) { - ret[i] = inWorld.resolve(UnresolvedType.forSignature(anns[i].getTypeSignature())); + } else { + ResolvedType[] ret = new ResolvedType[anns.length]; + for (int i = 0; i < ret.length; i++) { + ret[i] = inWorld.resolve(UnresolvedType.forSignature(anns[i].getTypeSignature())); + } + return ret; } - return ret; } catch (ClassNotFoundException cnfEx) { // just use reflection then } @@ -313,8 +311,6 @@ public class Java15AnnotationFinder implements AnnotationFinder, ArgNameFinder { return ret; } - public static final ResolvedType[][] NO_PARAMETER_ANNOTATIONS = new ResolvedType[][] {}; - public ResolvedType[][] getParameterAnnotationTypes(Member onMember) { if (!(onMember instanceof AccessibleObject)) return NO_PARAMETER_ANNOTATIONS; diff --git a/weaver5/java5-testsrc/org/aspectj/weaver/reflect/ReflectionWorldTest.java b/weaver5/java5-testsrc/org/aspectj/weaver/reflect/ReflectionWorldTest.java index 6c65c1e64..db3e44711 100644 --- a/weaver5/java5-testsrc/org/aspectj/weaver/reflect/ReflectionWorldTest.java +++ b/weaver5/java5-testsrc/org/aspectj/weaver/reflect/ReflectionWorldTest.java @@ -8,17 +8,21 @@ * ******************************************************************/ package org.aspectj.weaver.reflect; +import java.lang.reflect.Field; import java.lang.reflect.Method; import java.lang.reflect.Type; - +import java.net.URL; +import java.net.URLClassLoader; import java.util.List; +import java.util.Map; -import org.aspectj.weaver.ResolvedType; -import org.aspectj.weaver.UnresolvedType; -import org.aspectj.weaver.World; import org.aspectj.bridge.IMessageHandler; import org.aspectj.weaver.ReferenceType; import org.aspectj.weaver.ResolvedMember; +import org.aspectj.weaver.ResolvedType; +import org.aspectj.weaver.UnresolvedType; +import org.aspectj.weaver.WeakClassLoaderReference; +import org.aspectj.weaver.World; import org.aspectj.weaver.bcel.BcelWorld; import junit.framework.TestCase; @@ -35,6 +39,64 @@ public class ReflectionWorldTest extends TestCase { assertNotNull(rt); assertEquals("Ljava/lang/Object;", rt.getSignature()); } + + public void testReflectionWorldFactory() throws Exception { + ClassLoader parent = getClass().getClassLoader(); + ClassLoader cl1 = new URLClassLoader(new URL[] {}, parent); + ClassLoader cl2 = new URLClassLoader(new URL[] {}, parent); + + WeakClassLoaderReference wcl1 = new WeakClassLoaderReference(cl1); + WeakClassLoaderReference wcl2 = new WeakClassLoaderReference(cl2); + ReflectionWorld a = ReflectionWorld.getReflectionWorldFor(wcl1); + + ResolvedType stringClass1 = a.resolve(String.class); + assertNotNull(stringClass1); + + ReflectionWorld b = ReflectionWorld.getReflectionWorldFor(wcl1); + + // They should be the same because the classloader has not gone away + assertTrue(a==b); + + cl1 = null; + for (int i=0;i<100;i++) { + System.gc(); // How robust is it that this should be causing the reference to be collected? + } + b = ReflectionWorld.getReflectionWorldFor(wcl1); + + assertFalse(a==b); + + cl1 = new URLClassLoader(new URL[] {}, parent); + wcl1 = new WeakClassLoaderReference(cl1); + a = ReflectionWorld.getReflectionWorldFor(wcl1); + b = ReflectionWorld.getReflectionWorldFor(wcl2); + assertFalse(a==b); + + Field declaredField = ReflectionWorld.class.getDeclaredField("rworlds"); + declaredField.setAccessible(true); + Map worlds = (Map)declaredField.get(null); + assertEquals(2, worlds.size()); + + cl2 = null; + for (int i=0;i<100;i++) { + System.gc(); // How robust is it that this should be causing the reference to be collected? + } + ReflectionWorld.getReflectionWorldFor(wcl1); // need to call this to trigger tidyup + assertEquals(1, worlds.size()); + + cl1 = null; + for (int i=0;i<100;i++) { + System.gc(); // How robust is it that this should be causing the reference to be collected? + } + ReflectionWorld.getReflectionWorldFor(wcl1); // need to call this to trigger tidyup + assertEquals(0, worlds.size()); + + cl1 = new URLClassLoader(new URL[] {}, parent); + wcl1 = new WeakClassLoaderReference(cl1); + ReflectionWorld reflectionWorldFor = ReflectionWorld.getReflectionWorldFor(wcl1); + assertEquals(1, worlds.size()); + ReflectionWorld.cleanUpWorlds(); + assertEquals(0, worlds.size()); + } public void testArrayTypes() { IReflectionWorld world = new ReflectionWorld(getClass().getClassLoader()); -- 2.39.5