annotations need to be cached (jvm concurrency issue) [SPR-8737]

[spring-projects-issues]

George Baxter opened SPR-8737 and commented

Java has a concurrency issue when examining annotations on a method or method parameters (synchronized around a jam-wide weak hash map). In high concurrency environments, this becomes a bottleneck. The only current solution is to cache the annotations.

Currently, the MethodParameter class could cache the annotations based on the declaring class, the method and the parameter index. We might have been able to extend MethodParameter class to provide this caching, but unfortunately, the object is instantiated directly within the HandlerMethodInvoker.resolveHandlerArguments(..) (private method).

Currently a critical issue for us.

---

Affects: 3.0.5, 3.0.6

Issue Links:

• monitor contention at org.springframework.core.MethodParameter.getParameterAnnotations() [SPR-9298] #13936 monitor contention at org.springframework.core.MethodParameter.getParameterAnnotations() ("duplicates")

[spring-projects-issues]

George Baxter commented

Proposal.. caches method annotations as well, though those don't seem as necessary:

/*

• Copyright 2002-2010 the original author or authors.

• Licensed under the Apache License, Version 2.0 (the "License");

• you may not use this file except in compliance with the License.

• You may obtain a copy of the License at

  http://www.apache.org/licenses/LICENSE-2.0

• Unless required by applicable law or agreed to in writing, software

• distributed under the License is distributed on an "AS IS" BASIS,

• WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.

• See the License for the specific language governing permissions and

• limitations under the License.
  */

package org.springframework.core;

import java.lang.annotation.Annotation;
import java.lang.reflect.Constructor;
import java.lang.reflect.Method;
import java.lang.reflect.Type;
import java.lang.reflect.TypeVariable;
import java.util.HashMap;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

import org.springframework.util.Assert;

/**
*

• OVERRIDE FOR ANNOTATION CACHING

• Helper class that encapsulates the specification of a method parameter, i.e.

• a Method or Constructor plus a parameter index and a nested type index for

• a declared generic type. Useful as a specification object to pass along.

• @author Juergen Hoeller

• @author Rob Harrop

• @author Andy Clement

• @since 2.0

• @see GenericCollectionTypeResolver
  */
  public class MethodParameter {

  private Method method;

  private Constructor<?> constructor;

  private final int parameterIndex;

  private Class<?> parameterType;

  private Type genericParameterType;

  private Annotation[] parameterAnnotations;

  private ParameterNameDiscoverer parameterNameDiscoverer;

  private String parameterName;

  private int nestingLevel = 1;

  /** Map from Integer level to Integer type index */
  private Map<Integer,Integer> typeIndexesPerLevel;

  Map<TypeVariable<?>, Type> typeVariableMap;

  private static final Map<MethodParameterCacheKey, Annotation[]> parameterAnnotationCache = new ConcurrentHashMap<MethodParameterCacheKey, Annotation[]>();
  private static final Map<MethodCacheKey, Annotation[]> methodAnnotationCache = new ConcurrentHashMap<MethodCacheKey, Annotation[]>();

  /**

  • Create a new MethodParameter for the given method, with nesting level 1.
  • @param method the Method to specify a parameter for
  • @param parameterIndex the index of the parameter
    */
    public MethodParameter(Method method, int parameterIndex) {
    this(method, parameterIndex, 1);
    }

  /**

  • Create a new MethodParameter for the given method.
  • @param method the Method to specify a parameter for
  • @param parameterIndex the index of the parameter
  • (-1 for the method return type; 0 for the first method parameter,
  • 1 for the second method parameter, etc)
  • @param nestingLevel the nesting level of the target type
  • (typically 1; e.g. in case of a List of Lists, 1 would indicate the
  • nested List, whereas 2 would indicate the element of the nested List)
    */
    public MethodParameter(Method method, int parameterIndex, int nestingLevel) {
    Assert.notNull(method, "Method must not be null");
    this.method = method;
    this.parameterIndex = parameterIndex;
    this.nestingLevel = nestingLevel;
    }

  /**

  • Create a new MethodParameter for the given constructor, with nesting level 1.
  • @param constructor the Constructor to specify a parameter for
  • @param parameterIndex the index of the parameter
    */
    public MethodParameter(Constructor<?> constructor, int parameterIndex) {
    this(constructor, parameterIndex, 1);
    }

  /**

  • Create a new MethodParameter for the given constructor.
  • @param constructor the Constructor to specify a parameter for
  • @param parameterIndex the index of the parameter
  • @param nestingLevel the nesting level of the target type
  • (typically 1; e.g. in case of a List of Lists, 1 would indicate the
  • nested List, whereas 2 would indicate the element of the nested List)
    */
    public MethodParameter(Constructor<?> constructor, int parameterIndex, int nestingLevel) {
    Assert.notNull(constructor, "Constructor must not be null");
    this.constructor = constructor;
    this.parameterIndex = parameterIndex;
    this.nestingLevel = nestingLevel;
    }

  /**

  • Copy constructor, resulting in an independent MethodParameter object
  • based on the same metadata and cache state that the original object was in.
  • @param original the original MethodParameter object to copy from
    */
    public MethodParameter(MethodParameter original) {
    Assert.notNull(original, "Original must not be null");
    this.method = original.method;
    this.constructor = original.constructor;
    this.parameterIndex = original.parameterIndex;
    this.parameterType = original.parameterType;
    this.parameterAnnotations = original.parameterAnnotations;
    this.typeVariableMap = original.typeVariableMap;
    }

  /**

  • Return the wrapped Method, if any.

  <p>Note: Either Method or Constructor is available.

  • @return the Method, or <code>null</code> if none
    */
    public Method getMethod() {
    return this.method;
    }

  /**

  • Return the wrapped Constructor, if any.

  <p>Note: Either Method or Constructor is available.

  • @return the Constructor, or <code>null</code> if none
    */
    public Constructor<?> getConstructor() {
    return this.constructor;
    }

  /**

  • Return the class that declares the underlying Method or Constructor.
    */
    public Class<?> getDeclaringClass() {
    return (this.method != null ? this.method.getDeclaringClass() : this.constructor.getDeclaringClass());
    }

  /**

  • Return the index of the method/constructor parameter.
  • @return the parameter index (never negative)
    */
    public int getParameterIndex() {
    return this.parameterIndex;
    }

  /**

  • Set a resolved (generic) parameter type.
    */
    void setParameterType(Class<?> parameterType) {
    this.parameterType = parameterType;
    }

  /**

  • Return the type of the method/constructor parameter.
  • @return the parameter type (never <code>null</code>)
    */
    public Class<?> getParameterType() {
    if (this.parameterType == null) {
    if (this.parameterIndex < 0) {
    this.parameterType = (this.method != null ? this.method.getReturnType() : null);
    }
    else {
    this.parameterType = (this.method != null ?
    this.method.getParameterTypes()[this.parameterIndex] :
    this.constructor.getParameterTypes()[this.parameterIndex]);
    }
    }
    return this.parameterType;
    }

  /**

  • Return the generic type of the method/constructor parameter.
  • @return the parameter type (never <code>null</code>)
    */
    public Type getGenericParameterType() {
    if (this.genericParameterType == null) {
    if (this.parameterIndex < 0) {
    this.genericParameterType = (this.method != null ? this.method.getGenericReturnType() : null);
    }
    else {
    this.genericParameterType = (this.method != null ?
    this.method.getGenericParameterTypes()[this.parameterIndex] :
    this.constructor.getGenericParameterTypes()[this.parameterIndex]);
    }
    }
    return this.genericParameterType;
    }

  /**

  • Return the annotations associated with the target method/constructor itself.
    */
    public Annotation[] getMethodAnnotations() {
    //check the cache first
    MethodCacheKey key = new MethodCacheKey(this);
    Annotation[] annotations = methodAnnotationCache.get(key);
    if (annotations == null) {
    annotations = (this.method != null ? this.method.getAnnotations() : this.constructor.getAnnotations());
    methodAnnotationCache.put(key, annotations);
    }
    return annotations;
    }

  /**

  • Return the method/constructor annotation of the given type, if available.
  • @param annotationType the annotation type to look for
  • @return the annotation object, or <code>null</code> if not found
    */
    public <T extends Annotation> T getMethodAnnotation(Class<T> annotationType) {
    return (this.method != null ? this.method.getAnnotation(annotationType) :
    (T) this.constructor.getAnnotation(annotationType));
    }

  /**

  • Return the annotations associated with the specific method/constructor parameter.
    */
    public Annotation[] getParameterAnnotations() {
    if (this.parameterAnnotations == null) {
    // check the cache
    MethodParameterCacheKey key = new MethodParameterCacheKey(this);
    parameterAnnotations = parameterAnnotationCache.get(key);
    if (parameterAnnotations == null) {
    Annotation[][] annotationArray = (this.method != null ?
    this.method.getParameterAnnotations() : this.constructor.getParameterAnnotations());
    if (this.parameterIndex >= 0 && this.parameterIndex < annotationArray.length) {
    this.parameterAnnotations = annotationArray[this.parameterIndex];
    }
    else {
    this.parameterAnnotations = new Annotation[0];
    }
    parameterAnnotationCache.put(key, parameterAnnotations);
    }
    }
    return this.parameterAnnotations;
    }

  /**

  • Return the parameter annotation of the given type, if available.
  • @param annotationType the annotation type to look for
  • @return the annotation object, or <code>null</code> if not found
    */
    @SuppressWarnings("unchecked")
    public <T extends Annotation> T getParameterAnnotation(Class<T> annotationType) {
    Annotation[] anns = getParameterAnnotations();
    for (Annotation ann : anns) {
    if (annotationType.isInstance(ann)) {
    return (T) ann;
    }
    }
    return null;
    }

  /**

  • Initialize parameter name discovery for this method parameter.

  <p>This method does not actually try to retrieve the parameter name at

  • this point; it just allows discovery to happen when the application calls
  • {@link #getParameterName()} (if ever).
    */
    public void initParameterNameDiscovery(ParameterNameDiscoverer parameterNameDiscoverer) {
    this.parameterNameDiscoverer = parameterNameDiscoverer;
    }

  /**

  • Return the name of the method/constructor parameter.
  • @return the parameter name (may be <code>null</code> if no
  • parameter name metadata is contained in the class file or no
  • {@link #initParameterNameDiscovery ParameterNameDiscoverer}
  • has been set to begin with)
    */
    public String getParameterName() {
    if (this.parameterNameDiscoverer != null) {
    String[] parameterNames = (this.method != null ?
    this.parameterNameDiscoverer.getParameterNames(this.method) :
    this.parameterNameDiscoverer.getParameterNames(this.constructor));
    if (parameterNames != null) {
    this.parameterName = parameterNames[this.parameterIndex];
    }
    this.parameterNameDiscoverer = null;
    }
    return this.parameterName;
    }

  /**

  • Increase this parameter's nesting level.
  • @see #getNestingLevel()
    */
    public void increaseNestingLevel() {
    this.nestingLevel++;
    }

  /**

  • Decrease this parameter's nesting level.
  • @see #getNestingLevel()
    */
    public void decreaseNestingLevel() {
    getTypeIndexesPerLevel().remove(this.nestingLevel);
    this.nestingLevel--;
    }

  /**

  • Return the nesting level of the target type
  • (typically 1; e.g. in case of a List of Lists, 1 would indicate the
  • nested List, whereas 2 would indicate the element of the nested List).
    */
    public int getNestingLevel() {
    return this.nestingLevel;
    }

  /**

  • Set the type index for the current nesting level.
  • @param typeIndex the corresponding type index
  • (or <code>null</code> for the default type index)
  • @see #getNestingLevel()
    */
    public void setTypeIndexForCurrentLevel(int typeIndex) {
    getTypeIndexesPerLevel().put(this.nestingLevel, typeIndex);
    }

  /**

  • Return the type index for the current nesting level.
  • @return the corresponding type index, or <code>null</code>
  • if none specified (indicating the default type index)
  • @see #getNestingLevel()
    */
    public Integer getTypeIndexForCurrentLevel() {
    return getTypeIndexForLevel(this.nestingLevel);
    }

  /**

  • Return the type index for the specified nesting level.
  • @param nestingLevel the nesting level to check
  • @return the corresponding type index, or <code>null</code>
  • if none specified (indicating the default type index)
    */
    public Integer getTypeIndexForLevel(int nestingLevel) {
    return getTypeIndexesPerLevel().get(nestingLevel);
    }

  /**

  • Obtain the (lazily constructed) type-indexes-per-level Map.
    */
    private Map<Integer, Integer> getTypeIndexesPerLevel() {
    if (this.typeIndexesPerLevel == null) {
    this.typeIndexesPerLevel = new HashMap<Integer, Integer>(4);
    }
    return this.typeIndexesPerLevel;
    }

  /**

  • Create a new MethodParameter for the given method or constructor.

  <p>This is a convenience constructor for scenarios where a

  • Method or Constructor reference is treated in a generic fashion.
  • @param methodOrConstructor the Method or Constructor to specify a parameter for
  • @param parameterIndex the index of the parameter
  • @return the corresponding MethodParameter instance
    */
    public static MethodParameter forMethodOrConstructor(Object methodOrConstructor, int parameterIndex) {
    if (methodOrConstructor instanceof Method) {
    return new MethodParameter((Method) methodOrConstructor, parameterIndex);
    }
    else if (methodOrConstructor instanceof Constructor) {
    return new MethodParameter((Constructor<?>) methodOrConstructor, parameterIndex);
    }
    else {
    throw new IllegalArgumentException(
    "Given object [" + methodOrConstructor + "] is neither a Method nor a Constructor");
    }
    }

  /**

  • Key used for caching annotation information associated with this method parameter. MethodParameter objects themselves

  • are not cached, but the annotations associated with a particular parameter on a particular method for a particular

  • declaring class are cached. (Spring could someday cache the MethodParameter objects themselves using a key like this,

  • but this is a wider topic.
    **/
    private static final class MethodParameterCacheKey {

    private final Class declaringClass; private final Method method; private final int parameterIndex; private final Constructor constructor;

    /**

    • @param declaringClass
    • @param method
    • @param parameterIndex
      */
      public MethodParameterCacheKey(MethodParameter methodParameter) {
      super();
      this.declaringClass = methodParameter.getDeclaringClass();
      this.method = methodParameter.getMethod();
      this.constructor = methodParameter.getConstructor();
      this.parameterIndex = methodParameter.getParameterIndex();
      }

    /* (non-Javadoc)

    • @see java.lang.Object#hashCode()
      */
      @Override
      public int hashCode() {
      final int prime = 31;
      int result = 1;
      result = prime
      • result
      • ((declaringClass == null) ? 0 : declaringClass.hashCode());
        result = prime * result
      • ((method == null) ? 0 : method.hashCode());
        result = prime * result
      • ((constructor == null) ? 0 : constructor.hashCode());
        result = prime * result + parameterIndex;
        return result;
        }

    /* (non-Javadoc)

    • @see java.lang.Object#equals(java.lang.Object)
      */
      @Override
      public boolean equals(Object obj) {
      if (this == obj) {
      return true;
      }
      if (obj == null) {
      return false;
      }
      if (getClass() != obj.getClass()) {
      return false;
      }
      MethodParameterCacheKey other = (MethodParameterCacheKey) obj;

      if (parameterIndex != other.parameterIndex) {
      return false;
      }

      if (declaringClass == null) {
      if (other.declaringClass != null) {
      return false;
      }
      } else if (!declaringClass.equals(other.declaringClass)) {
      return false;
      }

      if (method == null) {
      if (other.method != null) {
      return false;
      }
      } else if (!method.equals(other.method)) {
      return false;
      }

      if (constructor == null) {
      if (other.constructor != null) {
      return false;
      }
      } else if (!constructor.equals(other.constructor)) {
      return false;
      }

      return true;
      }
      }

  /**

  • Key used for caching annotation information associated with this method parameter's method. MethodParameter objects themselves

  • are not cached, but the annotations associated with a particular method for a particular

  • declaring class are cached. (Spring could someday cache the MethodParameter objects themselves using a key like this,

  • but this is a wider topic.
    **/
    private static final class MethodCacheKey {

    private final Class declaringClass; private final Method method; private final Constructor constructor;

    /**

    • @param declaringClass
    • @param method
    • @param parameterIndex
      */
      public MethodCacheKey(MethodParameter methodParameter) {
      super();
      this.declaringClass = methodParameter.getDeclaringClass();
      this.method = methodParameter.getMethod();
      this.constructor = methodParameter.getConstructor();
      }

    /* (non-Javadoc)

    • @see java.lang.Object#hashCode()
      */
      @Override
      public int hashCode() {
      final int prime = 31;
      int result = 1;
      result = prime
      • result
      • ((declaringClass == null) ? 0 : declaringClass.hashCode());
        result = prime * result
      • ((method == null) ? 0 : method.hashCode());
        result = prime * result
      • ((constructor == null) ? 0 : constructor.hashCode());
        return result;
        }

    /* (non-Javadoc)

    • @see java.lang.Object#equals(java.lang.Object)
      */
      @Override
      public boolean equals(Object obj) {
      if (this == obj) {
      return true;
      }
      if (obj == null) {
      return false;
      }
      if (getClass() != obj.getClass()) {
      return false;
      }
      MethodCacheKey other = (MethodCacheKey) obj;

      if (declaringClass == null) {
      if (other.declaringClass != null) {
      return false;
      }
      } else if (!declaringClass.equals(other.declaringClass)) {
      return false;
      }

      if (method == null) {
      if (other.method != null) {
      return false;
      }
      } else if (!method.equals(other.method)) {
      return false;
      }

      if (constructor == null) {
      if (other.constructor != null) {
      return false;
      }
      } else if (!constructor.equals(other.constructor)) {
      return false;
      }
      return true;
      }

  }
  }

[spring-projects-issues]

George Baxter commented

Since the inserted code lost all formatting and really isn't clear what changes I made:

	private static final Map<MethodParameterCacheKey, Annotation[]> parameterAnnotationCache = new ConcurrentHashMap<MethodParameterCacheKey, Annotation[]>();
        // not entirely sure this is needed as much, since spring may already be caching the data, at least for mvc support.
	private static final Map<MethodCacheKey, Annotation[]> methodAnnotationCache = new ConcurrentHashMap<MethodCacheKey, Annotation[]>();

	/**
	 * Return the annotations associated with the target method/constructor itself.
	 */
	public Annotation[] getMethodAnnotations() {
		//check the cache first
		MethodCacheKey key = new MethodCacheKey(this);
		Annotation[] annotations = methodAnnotationCache.get(key);
		if (annotations == null) {
			annotations =  (this.method != null ? this.method.getAnnotations() : this.constructor.getAnnotations());
			methodAnnotationCache.put(key, annotations);
		}
		return annotations;
	}

	/**
	 * Return the annotations associated with the specific method/constructor parameter.
	 */
	public Annotation[] getParameterAnnotations() {
		if (this.parameterAnnotations == null) {
			// check the cache
			MethodParameterCacheKey key = new MethodParameterCacheKey(this);
			parameterAnnotations = parameterAnnotationCache.get(key);
			if (parameterAnnotations == null) {
				Annotation[][] annotationArray = (this.method != null ?
						this.method.getParameterAnnotations() : this.constructor.getParameterAnnotations());
				if (this.parameterIndex >= 0 && this.parameterIndex < annotationArray.length) {
					this.parameterAnnotations = annotationArray[this.parameterIndex];
				}
				else {
					this.parameterAnnotations = new Annotation[0];
				}
				parameterAnnotationCache.put(key, parameterAnnotations);
			}
		}
		return this.parameterAnnotations;
	}



/**
     * Key used for caching annotation information associated with this method parameter.  MethodParameter objects themselves
     * are not cached, but the annotations associated with a particular parameter on a particular method for a particular
     * declaring class are cached.  (Spring could someday cache the MethodParameter objects themselves using a key like this,
     * but this is a wider topic.
     **/
    private static final class MethodParameterCacheKey {

		private final Class<?> declaringClass;
    	private final Method method;
    	private final int parameterIndex;
    	private final Constructor<?> constructor;

		/**
		 * @param declaringClass
		 * @param method
		 * @param parameterIndex
		 */
		public MethodParameterCacheKey(MethodParameter methodParameter) {
			super();
			this.declaringClass = methodParameter.getDeclaringClass();
			this.method = methodParameter.getMethod();
			this.constructor = methodParameter.getConstructor();
			this.parameterIndex = methodParameter.getParameterIndex();
		}
    	
    	/* (non-Javadoc)
    	 * @see java.lang.Object#hashCode()
    	 */
    	@Override
		public int hashCode() {
			final int prime = 31;
			int result = 1;
			result = prime
					* result
					+ ((declaringClass == null) ? 0 : declaringClass.hashCode());
			result = prime * result
				+ ((method == null) ? 0 : method.hashCode());
			result = prime * result
				+ ((constructor == null) ? 0 : constructor.hashCode());
			result = prime * result + parameterIndex;
			return result;
		}

		/* (non-Javadoc)
		 * @see java.lang.Object#equals(java.lang.Object)
		 */
		@Override
		public boolean equals(Object obj) {
			if (this == obj) {
				return true;
			}
			if (obj == null) {
				return false;
			}
			if (getClass() != obj.getClass()) {
				return false;
			}
			MethodParameterCacheKey other = (MethodParameterCacheKey) obj;
			
			if (parameterIndex != other.parameterIndex) {
				return false;
			}
			
			if (declaringClass == null) {
				if (other.declaringClass != null) {
					return false;
				}
			} else if (!declaringClass.equals(other.declaringClass)) {
				return false;
			}
			
			if (method == null) {
				if (other.method != null) {
					return false;
				}
			} else if (!method.equals(other.method)) {
				return false;
			}
			
			if (constructor == null) {
				if (other.constructor != null) {
					return false;
				}
			} else if (!constructor.equals(other.constructor)) {
				return false;
			}

			return true;
		}
    }
    
    /**
     * Key used for caching annotation information associated with this method parameter's method.  MethodParameter objects themselves
     * are not cached, but the annotations associated with a  particular method for a particular
     * declaring class are cached.  (Spring could someday cache the MethodParameter objects themselves using a key like this,
     * but this is a wider topic.
     **/
    private static final class MethodCacheKey {

		private final Class<?> declaringClass;
    	private final Method method;
    	private final Constructor<?> constructor;
    	
		/**
		 * @param declaringClass
		 * @param method
		 * @param parameterIndex
		 */
		public MethodCacheKey(MethodParameter methodParameter) {
			super();
			this.declaringClass = methodParameter.getDeclaringClass();
			this.method = methodParameter.getMethod();
			this.constructor = methodParameter.getConstructor();
		}
    	
    	/* (non-Javadoc)
    	 * @see java.lang.Object#hashCode()
    	 */
    	@Override
		public int hashCode() {
			final int prime = 31;
			int result = 1;
			result = prime
					* result
					+ ((declaringClass == null) ? 0 : declaringClass.hashCode());
			result = prime * result
				+ ((method == null) ? 0 : method.hashCode());
			result = prime * result
				+ ((constructor == null) ? 0 : constructor.hashCode());
			return result;
		}

		/* (non-Javadoc)
		 * @see java.lang.Object#equals(java.lang.Object)
		 */
		@Override
		public boolean equals(Object obj) {
			if (this == obj) {
				return true;
			}
			if (obj == null) {
				return false;
			}
			if (getClass() != obj.getClass()) {
				return false;
			}
			MethodCacheKey other = (MethodCacheKey) obj;
						
			if (declaringClass == null) {
				if (other.declaringClass != null) {
					return false;
				}
			} else if (!declaringClass.equals(other.declaringClass)) {
				return false;
			}
			
			if (method == null) {
				if (other.method != null) {
					return false;
				}
			} else if (!method.equals(other.method)) {
				return false;
			}
			
			if (constructor == null) {
				if (other.constructor != null) {
					return false;
				}
			} else if (!constructor.equals(other.constructor)) {
				return false;
			}
			return true;
		}

    }

[spring-projects-issues]

Chris Beams commented

Thanks for the suggested fix, George. This issue has just been fixed based on #13936. Please give the latest 3.2.0.BUILD-SNAPSHOT a try and let us know (in comments on #13936) how it goes!