Skip to content

Commit 72b1abd

Browse files
committed
Reintroduce synthesized annotation attribute value caching
Prior to the introduction of the MergedAnnotation API in Spring Framework 5.2, our SynthesizedAnnotationInvocationHandler utilized a cache for annotation attribute values; whereas, the new SynthesizedMergedAnnotationInvocationHandler has no such caching in place. Issues such as gh-24961 indicate a regression in performance caused by the lack of such an attribute value cache. For example, the required attribute in @RequestParam is looked up using the internal meta-model in the MergedAnnotation API twice per request for each @RequestParam in a given controller handler method. This commit reintroduces the attribute value cache to avoid the unnecessary performance overhead associated with multiple lookups of the same attribute in a synthesized annotation. This applies not only to direct attribute method invocations but also to invocations of equals() and hashCode() on a synthesized annotation. Note, however, that this commit does NOT address multiple lookups of annotation attribute values for invocations of toString(). That behavior currently remains unchanged in the implementation of org.springframework.core.annotation.TypeMappedAnnotation.toString(). Closes gh-24970
1 parent 0e9da17 commit 72b1abd

File tree

1 file changed

+53
-5
lines changed

1 file changed

+53
-5
lines changed

spring-core/src/main/java/org/springframework/core/annotation/SynthesizedMergedAnnotationInvocationHandler.java

Lines changed: 53 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,14 @@
1818

1919
import java.lang.annotation.Annotation;
2020
import java.lang.reflect.AnnotatedElement;
21+
import java.lang.reflect.Array;
2122
import java.lang.reflect.InvocationHandler;
2223
import java.lang.reflect.Method;
2324
import java.lang.reflect.Proxy;
2425
import java.util.Arrays;
26+
import java.util.Map;
2527
import java.util.NoSuchElementException;
28+
import java.util.concurrent.ConcurrentHashMap;
2629

2730
import org.springframework.lang.Nullable;
2831
import org.springframework.util.Assert;
@@ -50,6 +53,8 @@ final class SynthesizedMergedAnnotationInvocationHandler<A extends Annotation> i
5053

5154
private final AttributeMethods attributes;
5255

56+
private final Map<String, Object> valueCache = new ConcurrentHashMap<>(8);
57+
5358
@Nullable
5459
private volatile Integer hashCode;
5560

@@ -167,11 +172,53 @@ private int getValueHashCode(Object value) {
167172
}
168173

169174
private Object getAttributeValue(Method method) {
170-
String name = method.getName();
171-
Class<?> type = ClassUtils.resolvePrimitiveIfNecessary(method.getReturnType());
172-
return this.annotation.getValue(name, type).orElseThrow(
173-
() -> new NoSuchElementException("No value found for attribute named '" + name +
174-
"' in merged annotation " + this.annotation.getType().getName()));
175+
Object value = this.valueCache.computeIfAbsent(method.getName(), attributeName -> {
176+
Class<?> type = ClassUtils.resolvePrimitiveIfNecessary(method.getReturnType());
177+
return this.annotation.getValue(attributeName, type).orElseThrow(
178+
() -> new NoSuchElementException("No value found for attribute named '" + attributeName +
179+
"' in merged annotation " + this.annotation.getType().getName()));
180+
});
181+
182+
// Clone non-empty arrays so that users cannot alter the contents of values in our cache.
183+
if (value.getClass().isArray() && Array.getLength(value) > 0) {
184+
value = cloneArray(value);
185+
}
186+
187+
return value;
188+
}
189+
190+
/**
191+
* Clone the provided array, ensuring that original component type is retained.
192+
* @param array the array to clone
193+
*/
194+
private Object cloneArray(Object array) {
195+
if (array instanceof boolean[]) {
196+
return ((boolean[]) array).clone();
197+
}
198+
if (array instanceof byte[]) {
199+
return ((byte[]) array).clone();
200+
}
201+
if (array instanceof char[]) {
202+
return ((char[]) array).clone();
203+
}
204+
if (array instanceof double[]) {
205+
return ((double[]) array).clone();
206+
}
207+
if (array instanceof float[]) {
208+
return ((float[]) array).clone();
209+
}
210+
if (array instanceof int[]) {
211+
return ((int[]) array).clone();
212+
}
213+
if (array instanceof long[]) {
214+
return ((long[]) array).clone();
215+
}
216+
if (array instanceof short[]) {
217+
return ((short[]) array).clone();
218+
}
219+
220+
// else
221+
return ((Object[]) array).clone();
175222
}
176223

177224
@SuppressWarnings("unchecked")
@@ -183,6 +230,7 @@ static <A extends Annotation> A createProxy(MergedAnnotation<A> annotation, Clas
183230
return (A) Proxy.newProxyInstance(classLoader, interfaces, handler);
184231
}
185232

233+
186234
private static boolean isVisible(ClassLoader classLoader, Class<?> interfaceClass) {
187235
if (classLoader == interfaceClass.getClassLoader()) {
188236
return true;

0 commit comments

Comments
 (0)