From ce22809f8bd6c1c4dc7e2629b58a0d43d415ad9d Mon Sep 17 00:00:00 2001 From: unknown Date: Tue, 3 Apr 2012 15:33:41 -0700 Subject: [PATCH] Fix hotspot i MethodParameter.getParameterAnnotations() MethodParameter.getParameterAnnotations() calls Method.getParameterAnnotations() on every invocation. Latter ends up contending for a monitor inside (Sun) JDK code. Fix by caching values returned by Method.getParameterAnnotations() in a static ConcurrentMap Issue: SPR-9298 --- .../springframework/core/MethodParameter.java | 49 ++++++++++++++++++- .../core/MethodParameterTests.java | 34 ++++++++++++- 2 files changed, 80 insertions(+), 3 deletions(-) diff --git a/spring-core/src/main/java/org/springframework/core/MethodParameter.java b/spring-core/src/main/java/org/springframework/core/MethodParameter.java index 9c33cb25783e..d9c52a749bfb 100644 --- a/spring-core/src/main/java/org/springframework/core/MethodParameter.java +++ b/spring-core/src/main/java/org/springframework/core/MethodParameter.java @@ -26,6 +26,8 @@ import java.lang.reflect.TypeVariable; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; import org.springframework.util.Assert; @@ -66,7 +68,10 @@ public class MethodParameter { Map typeVariableMap; private int hash = 0; - + + private static ConcurrentMap methodParamAnnotationsCache = new ConcurrentHashMap(); + final private static Annotation[][] EMPTY_ANNOTATION_MATRIX = new Annotation[0][0]; + final private static Annotation[] EMPTY_ANNOTATION_ARRAY = new Annotation[0]; /** * Create a new MethodParameter for the given method, with nesting level 1. @@ -280,7 +285,7 @@ public T getMethodAnnotation(Class annotationType) { public Annotation[] getParameterAnnotations() { if (this.parameterAnnotations == null) { Annotation[][] annotationArray = (this.method != null ? - this.method.getParameterAnnotations() : this.constructor.getParameterAnnotations()); + getMethodParameterAnnotations(this.method) : this.constructor.getParameterAnnotations()); if (this.parameterIndex >= 0 && this.parameterIndex < annotationArray.length) { this.parameterAnnotations = annotationArray[this.parameterIndex]; } @@ -290,6 +295,42 @@ public Annotation[] getParameterAnnotations() { } return this.parameterAnnotations; } + + /** + * + * @param m + * @return parameter annotations for given m. returns + */ + static Annotation[][] getMethodParameterAnnotations(Method m) { + assert m != null; + + Annotation[][] result = getMethodParamAnnotationsCache().get(m); + if (result == null) { + //no point in synchronizing call to below since it's an idempotent read + result = m.getParameterAnnotations(); //always returns a new copy + + //minimize cache size: do not store copies of empty arrays + if(result.length == 0) + { + result = EMPTY_ANNOTATION_MATRIX; + } else { + for (int i = 0; i < result.length; i++) { + if (result[i].length == 0) { + result[i] = EMPTY_ANNOTATION_ARRAY; + } + } + } + getMethodParamAnnotationsCache().put(m, result); + } + + //always return deep copy to prevent caller from modifying cache state + Annotation[][] resultCopy = new Annotation[result.length][0]; + for(int i = 0; i < result.length; i++) + { + resultCopy[i] = result[i].clone(); + } + return resultCopy; + } /** * Return the parameter annotation of the given type, if available. @@ -472,4 +513,8 @@ public int hashCode() { return result; } + static ConcurrentMap getMethodParamAnnotationsCache() { + return methodParamAnnotationsCache; + } + } diff --git a/spring-core/src/test/java/org/springframework/core/MethodParameterTests.java b/spring-core/src/test/java/org/springframework/core/MethodParameterTests.java index 2f1ce99bfd96..c6fbc67fe749 100644 --- a/spring-core/src/test/java/org/springframework/core/MethodParameterTests.java +++ b/spring-core/src/test/java/org/springframework/core/MethodParameterTests.java @@ -16,11 +16,20 @@ package org.springframework.core; +import java.lang.annotation.Annotation; +import java.lang.annotation.Retention; +import java.lang.annotation.Target; import java.lang.reflect.Method; import org.junit.Before; import org.junit.Test; +import static java.lang.annotation.ElementType.ANNOTATION_TYPE; +import static java.lang.annotation.ElementType.CONSTRUCTOR; +import static java.lang.annotation.ElementType.FIELD; +import static java.lang.annotation.ElementType.METHOD; +import static java.lang.annotation.ElementType.PARAMETER; +import static java.lang.annotation.RetentionPolicy.RUNTIME; import static org.junit.Assert.*; /** @@ -76,10 +85,33 @@ public void testHashCode() throws NoSuchMethodException { assertEquals(stringParameter.hashCode(), methodParameter.hashCode()); assertTrue(longParameter.hashCode() != methodParameter.hashCode()); } + + @Test + public void testGetMethodParamaterAnnotations() { + Method method = stringParameter.getMethod(); + Annotation[][] expectedAnnotations = method.getParameterAnnotations(); + assertEquals(2, expectedAnnotations.length); + assertEquals(DummyAnnotation.class, expectedAnnotations[0][0].annotationType()); + + //start with empty cache + MethodParameter.getMethodParamAnnotationsCache().clear(); + + //check correctness + assertArrayEquals(expectedAnnotations, MethodParameter.getMethodParameterAnnotations(method)); + //check that return value's been cached + assertArrayEquals(expectedAnnotations, MethodParameter.getMethodParamAnnotationsCache().get(method)); + } - public int method(String p1, long p2) { + public int method(@DummyAnnotation String p1, long p2) { return 42; } + + @Target({ CONSTRUCTOR, PARAMETER }) + @Retention(RUNTIME) + public @interface DummyAnnotation { + + } + }