Skip to content

Commit 682a910

Browse files
jhoellerjhoeller
committed
ReflectionUtils caches Class.getDeclaredMethods() results; AnnotationUtils caches findAnnotation results
Issue: SPR-11882
1 parent b0979cb commit 682a910

File tree

2 files changed

+117
-44
lines changed

2 files changed

+117
-44
lines changed

‎spring-core/src/main/java/org/springframework/core/annotation/AnnotationUtils.java

Lines changed: 95 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -26,13 +26,13 @@
2626
import java.util.List;
2727
import java.util.Map;
2828
import java.util.Set;
29-
import java.util.WeakHashMap;
3029

3130
import org.apache.commons.logging.Log;
3231
import org.apache.commons.logging.LogFactory;
3332

3433
import org.springframework.core.BridgeMethodResolver;
3534
import org.springframework.util.Assert;
35+
import org.springframework.util.ConcurrentReferenceHashMap;
3636
import org.springframework.util.ObjectUtils;
3737
import org.springframework.util.ReflectionUtils;
3838

@@ -68,7 +68,11 @@ public abstract class AnnotationUtils {
6868

6969
private static final Log logger = LogFactory.getLog(AnnotationUtils.class);
7070

71-
private static final Map<Class<?>, Boolean> annotatedInterfaceCache = new WeakHashMap<Class<?>, Boolean>();
71+
private static final Map<AnnotationCacheKey, Annotation> findAnnotationCache =
72+
new ConcurrentReferenceHashMap<AnnotationCacheKey, Annotation>(256);
73+
74+
private static final Map<Class<?>, Boolean> annotatedInterfaceCache =
75+
new ConcurrentReferenceHashMap<Class<?>, Boolean>(256);
7276

7377

7478
/**
@@ -222,32 +226,40 @@ public static <A extends Annotation> Set<A> getRepeatableAnnotation(AnnotatedEle
222226
* @param annotationType the annotation type to look for
223227
* @return the annotation found, or {@code null} if none
224228
*/
229+
@SuppressWarnings("unchecked")
225230
public static <A extends Annotation> A findAnnotation(Method method, Class<A> annotationType) {
226-
A annotation = getAnnotation(method, annotationType);
227-
Class<?> clazz = method.getDeclaringClass();
228-
if (annotation == null) {
229-
annotation = searchOnInterfaces(method, annotationType, clazz.getInterfaces());
230-
}
231-
while (annotation == null) {
232-
clazz = clazz.getSuperclass();
233-
if (clazz == null || clazz.equals(Object.class)) {
234-
break;
235-
}
236-
try {
237-
Method equivalentMethod = clazz.getDeclaredMethod(method.getName(), method.getParameterTypes());
238-
annotation = getAnnotation(equivalentMethod, annotationType);
231+
AnnotationCacheKey cacheKey = new AnnotationCacheKey(method, annotationType);
232+
A result = (A) findAnnotationCache.get(cacheKey);
233+
if (result == null) {
234+
result = getAnnotation(method, annotationType);
235+
Class<?> clazz = method.getDeclaringClass();
236+
if (result == null) {
237+
result = searchOnInterfaces(method, annotationType, clazz.getInterfaces());
239238
}
240-
catch (NoSuchMethodException ex) {
241-
// No equivalent method found
239+
while (result == null) {
240+
clazz = clazz.getSuperclass();
241+
if (clazz == null || clazz.equals(Object.class)) {
242+
break;
243+
}
244+
try {
245+
Method equivalentMethod = clazz.getDeclaredMethod(method.getName(), method.getParameterTypes());
246+
result = getAnnotation(equivalentMethod, annotationType);
247+
}
248+
catch (NoSuchMethodException ex) {
249+
// No equivalent method found
250+
}
251+
if (result == null) {
252+
result = searchOnInterfaces(method, annotationType, clazz.getInterfaces());
253+
}
242254
}
243-
if (annotation == null) {
244-
annotation = searchOnInterfaces(method, annotationType, clazz.getInterfaces());
255+
if (result != null) {
256+
findAnnotationCache.put(cacheKey, result);
245257
}
246258
}
247-
return annotation;
259+
return result;
248260
}
249261

250-
private static <A extends Annotation> A searchOnInterfaces(Method method, Class<A> annotationType, Class<?>[] ifcs) {
262+
private static <A extends Annotation> A searchOnInterfaces(Method method, Class<A> annotationType, Class<?>... ifcs) {
251263
A annotation = null;
252264
for (Class<?> iface : ifcs) {
253265
if (isInterfaceWithAnnotatedMethods(iface)) {
@@ -267,30 +279,28 @@ private static <A extends Annotation> A searchOnInterfaces(Method method, Class<
267279
}
268280

269281
private static boolean isInterfaceWithAnnotatedMethods(Class<?> iface) {
270-
synchronized (annotatedInterfaceCache) {
271-
Boolean flag = annotatedInterfaceCache.get(iface);
272-
if (flag != null) {
273-
return flag;
274-
}
275-
boolean found = false;
276-
for (Method ifcMethod : iface.getMethods()) {
277-
try {
278-
if (ifcMethod.getAnnotations().length > 0) {
279-
found = true;
280-
break;
281-
}
282+
Boolean flag = annotatedInterfaceCache.get(iface);
283+
if (flag != null) {
284+
return flag;
285+
}
286+
boolean found = false;
287+
for (Method ifcMethod : iface.getMethods()) {
288+
try {
289+
if (ifcMethod.getAnnotations().length > 0) {
290+
found = true;
291+
break;
282292
}
283-
catch (Exception ex) {
284-
// Assuming nested Class values not resolvable within annotation attributes...
285-
// We're probably hitting a non-present optional arrangement - let's back out.
286-
if (logger.isInfoEnabled()) {
287-
logger.info("Failed to introspect annotations on [" + ifcMethod + "]: " + ex);
288-
}
293+
}
294+
catch (Exception ex) {
295+
// Assuming nested Class values not resolvable within annotation attributes...
296+
// We're probably hitting a non-present optional arrangement - let's back out.
297+
if (logger.isInfoEnabled()) {
298+
logger.info("Failed to introspect annotations on [" + ifcMethod + "]: " + ex);
289299
}
290300
}
291-
annotatedInterfaceCache.put(iface, found);
292-
return found;
293301
}
302+
annotatedInterfaceCache.put(iface, found);
303+
return found;
294304
}
295305

296306
/**
@@ -315,8 +325,17 @@ private static boolean isInterfaceWithAnnotatedMethods(Class<?> iface) {
315325
* @param annotationType the type of annotation to look for
316326
* @return the annotation if found, or {@code null} if not found
317327
*/
328+
@SuppressWarnings("unchecked")
318329
public static <A extends Annotation> A findAnnotation(Class<?> clazz, Class<A> annotationType) {
319-
return findAnnotation(clazz, annotationType, new HashSet<Annotation>());
330+
AnnotationCacheKey cacheKey = new AnnotationCacheKey(clazz, annotationType);
331+
A result = (A) findAnnotationCache.get(cacheKey);
332+
if (result == null) {
333+
result = findAnnotation(clazz, annotationType, new HashSet<Annotation>());
334+
if (result != null) {
335+
findAnnotationCache.put(cacheKey, result);
336+
}
337+
}
338+
return result;
320339
}
321340

322341
/**
@@ -676,6 +695,40 @@ public static Object getDefaultValue(Class<? extends Annotation> annotationType,
676695
}
677696

678697

698+
/**
699+
* Default cache key for the TransactionAttribute cache.
700+
*/
701+
private static class AnnotationCacheKey {
702+
703+
private final AnnotatedElement element;
704+
705+
private final Class<? extends Annotation> annotationType;
706+
707+
public AnnotationCacheKey(AnnotatedElement element, Class<? extends Annotation> annotationType) {
708+
this.element = element;
709+
this.annotationType = annotationType;
710+
}
711+
712+
@Override
713+
public boolean equals(Object other) {
714+
if (this == other) {
715+
return true;
716+
}
717+
if (!(other instanceof AnnotationCacheKey)) {
718+
return false;
719+
}
720+
AnnotationCacheKey otherKey = (AnnotationCacheKey) other;
721+
return (this.element.equals(otherKey.element) &&
722+
ObjectUtils.nullSafeEquals(this.annotationType, otherKey.annotationType));
723+
}
724+
725+
@Override
726+
public int hashCode() {
727+
return (this.element.hashCode() * 29 + this.annotationType.hashCode());
728+
}
729+
}
730+
731+
679732
private static class AnnotationCollector<A extends Annotation> {
680733

681734
private final Class<? extends Annotation> containerAnnotationType;

‎spring-core/src/main/java/org/springframework/util/ReflectionUtils.java

Lines changed: 22 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import java.util.ArrayList;
2727
import java.util.Arrays;
2828
import java.util.List;
29+
import java.util.Map;
2930
import java.util.regex.Pattern;
3031

3132
/**
@@ -56,6 +57,12 @@ public abstract class ReflectionUtils {
5657
*/
5758
private static final Pattern CGLIB_RENAMED_METHOD_PATTERN = Pattern.compile("(.+)\\$\\d+");
5859

60+
/**
61+
* Cache for {@link Class#getDeclaredMethods()}, allowing for fast resolution.
62+
*/
63+
private static final Map<Class<?>, Method[]> declaredMethodsCache =
64+
new ConcurrentReferenceHashMap<Class<?>, Method[]>(256);
65+
5966

6067
/**
6168
* Attempt to find a {@link Field field} on the supplied {@link Class} with the
@@ -162,7 +169,7 @@ public static Method findMethod(Class<?> clazz, String name, Class<?>... paramTy
162169
Assert.notNull(name, "Method name must not be null");
163170
Class<?> searchType = clazz;
164171
while (searchType != null) {
165-
Method[] methods = (searchType.isInterface() ? searchType.getMethods() : searchType.getDeclaredMethods());
172+
Method[] methods = (searchType.isInterface() ? searchType.getMethods() : getDeclaredMethods(searchType));
166173
for (Method method : methods) {
167174
if (name.equals(method.getName()) &&
168175
(paramTypes == null || Arrays.equals(paramTypes, method.getParameterTypes()))) {
@@ -479,7 +486,7 @@ public static void doWithMethods(Class<?> clazz, MethodCallback mc, MethodFilter
479486
throws IllegalArgumentException {
480487

481488
// Keep backing up the inheritance hierarchy.
482-
Method[] methods = clazz.getDeclaredMethods();
489+
Method[] methods = getDeclaredMethods(clazz);
483490
for (Method method : methods) {
484491
if (mf != null && !mf.matches(method)) {
485492
continue;
@@ -554,6 +561,19 @@ public void doWith(Method method) {
554561
return methods.toArray(new Method[methods.size()]);
555562
}
556563

564+
/**
565+
* This method retrieves {@link Class#getDeclaredMethods()} from a local cache
566+
* in order to avoid the JVM's SecurityManager check and defensive array copying.
567+
*/
568+
private static Method[] getDeclaredMethods(Class<?> clazz) {
569+
Method[] result = declaredMethodsCache.get(clazz);
570+
if (result == null) {
571+
result = clazz.getDeclaredMethods();
572+
declaredMethodsCache.put(clazz, result);
573+
}
574+
return result;
575+
}
576+
557577
/**
558578
* Invoke the given callback on all fields in the target class, going up the
559579
* class hierarchy to get all declared fields.

0 commit comments

Comments
 (0)