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.

7 files changed, 240 insertions, 188 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 /* <URL,SoftRef(JavaClass)> */<URL, SoftReference<JavaClass>> localCache = new WeakHashMap<URL, SoftReference<JavaClass>>();
-	private static SoftHashMap /* <URL,JavaClass> */sharedCache = new SoftHashMap(Collections
-			.synchronizedMap(new HashMap<Object, SpecialValue>()));
+	private WeakHashMap<URL, SoftReference<JavaClass>> localCache = new WeakHashMap<URL, SoftReference<JavaClass>>();
+	private static SoftHashMap /* <URL,JavaClass> */sharedCache = new SoftHashMap(Collections.synchronizedMap(new HashMap<Object, SpecialValue>()));
 
 	// For fast translation of the classname *intentionally not static*
 	private SoftHashMap /* <String,URL> */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<WeakClassLoaderReference, ReflectionWorld> rworlds = Collections.synchronizedMap(new HashMap<WeakClassLoaderReference, ReflectionWorld>());
+
 	private WeakClassLoaderReference classLoaderReference;
 	private AnnotationFinder annotationFinder;
 	private boolean mustUseOneFourDelegates = false; // for testing
 	private Map<String,Class<?>> inProgressResolutionClasses = new HashMap<String,Class<?>>();
-
+	
+	public static ReflectionWorld getReflectionWorldFor(WeakClassLoaderReference classLoaderReference) {
+		synchronized (rworlds) {
+			// Tidyup any no longer relevant entries...
+			for (Iterator<Map.Entry<WeakClassLoaderReference, ReflectionWorld>> it = rworlds.entrySet().iterator();
+					it.hasNext();) {
+				Map.Entry<WeakClassLoaderReference, ReflectionWorld> 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<PointcutPrimitive> 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<PointcutPrimitive> 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<PointcutPrimitive> 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<? extends Annotation> annotationClass = (Class<? extends Annotation>) 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<ResolvedType> annSet = new HashSet<ResolvedType>();
-			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<UnresolvedType> annSet = new HashSet<UnresolvedType>();
+		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 3dcbf45b2..dd63c858e 100644
--- a/weaver5/java5-testsrc/org/aspectj/weaver/reflect/ReflectionWorldTest.java
+++ b/weaver5/java5-testsrc/org/aspectj/weaver/reflect/ReflectionWorldTest.java
@@ -11,14 +11,20 @@
  * ******************************************************************/
 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.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;
 
@@ -32,6 +38,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());