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.

4 files changed, 104 insertions, 110 deletions

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();
 	}
+	
 }