Efficient type plus annotation comparisons during converter retrieval

Issue: SPR-14926
Issue: SPR-12926

spring-context/src/main/java/org/springframework/format/support/FormattingConversionService.java

@@ -52,11 +52,9 @@ public class FormattingConversionService extends GenericConversionService
 
 	private StringValueResolver embeddedValueResolver;
 
-	private final Map<AnnotationConverterKey, GenericConverter> cachedPrinters =
+	private final Map<AnnotationConverterKey, GenericConverter> cachedPrinters = new ConcurrentHashMap<>(64);
-			new ConcurrentHashMap<>(64);
 
-	private final Map<AnnotationConverterKey, GenericConverter> cachedParsers =
+	private final Map<AnnotationConverterKey, GenericConverter> cachedParsers = new ConcurrentHashMap<>(64);
-			new ConcurrentHashMap<>(64);
 
 
 	@Override
@@ -231,6 +229,7 @@ public class FormattingConversionService extends GenericConversionService
 
 		public AnnotationPrinterConverter(Class<? extends Annotation> annotationType,
 				AnnotationFormatterFactory<?> annotationFormatterFactory, Class<?> fieldType) {
+
 			this.annotationType = annotationType;
 			this.annotationFormatterFactory = annotationFormatterFactory;
 			this.fieldType = fieldType;
@@ -284,6 +283,7 @@ public class FormattingConversionService extends GenericConversionService
 
 		public AnnotationParserConverter(Class<? extends Annotation> annotationType,
 				AnnotationFormatterFactory<?> annotationFormatterFactory, Class<?> fieldType) {
+
 			this.annotationType = annotationType;
 			this.annotationFormatterFactory = annotationFormatterFactory;
 			this.fieldType = fieldType;
@@ -350,16 +350,13 @@ public class FormattingConversionService extends GenericConversionService
 			if (this == other) {
 				return true;
 			}
-			if (!(other instanceof AnnotationConverterKey)) {
-				return false;
-			}
 			AnnotationConverterKey otherKey = (AnnotationConverterKey) other;
-			return (this.annotation.equals(otherKey.annotation) && this.fieldType.equals(otherKey.fieldType));
+			return (this.fieldType == otherKey.fieldType && this.annotation.equals(otherKey.annotation));
 		}
 
 		@Override
 		public int hashCode() {
-			return (this.annotation.hashCode() + 29 * this.fieldType.hashCode());
+			return (this.fieldType.hashCode() * 29 + this.annotation.hashCode());
 		}
 	}
 

spring-core/src/main/java/org/springframework/core/convert/TypeDescriptor.java

@@ -68,7 +68,7 @@ public class TypeDescriptor implements Serializable {
 
 	private final ResolvableType resolvableType;
 
-	private final AnnotatedElement annotatedElement;
+	private final AnnotatedElementAdapter annotatedElement;
 
 
 	/**
@@ -78,7 +78,6 @@ public class TypeDescriptor implements Serializable {
 	 * @param methodParameter the method parameter
 	 */
 	public TypeDescriptor(MethodParameter methodParameter) {
-		Assert.notNull(methodParameter, "MethodParameter must not be null");
 		this.resolvableType = ResolvableType.forMethodParameter(methodParameter);
 		this.type = this.resolvableType.resolve(methodParameter.getParameterType());
 		this.annotatedElement = new AnnotatedElementAdapter(methodParameter.getParameterIndex() == -1 ?
@@ -91,7 +90,6 @@ public class TypeDescriptor implements Serializable {
 	 * @param field the field
 	 */
 	public TypeDescriptor(Field field) {
-		Assert.notNull(field, "Field must not be null");
 		this.resolvableType = ResolvableType.forField(field);
 		this.type = this.resolvableType.resolve(field.getType());
 		this.annotatedElement = new AnnotatedElementAdapter(field.getAnnotations());
@@ -117,6 +115,7 @@ public class TypeDescriptor implements Serializable {
 	 * @param resolvableType the resolvable type
 	 * @param type the backing type (or {@code null} if it should get resolved)
 	 * @param annotations the type annotations
+	 * @since 4.0
 	 */
 	protected TypeDescriptor(ResolvableType resolvableType, Class<?> type, Annotation[] annotations) {
 		this.resolvableType = resolvableType;
@@ -207,7 +206,7 @@ public class TypeDescriptor implements Serializable {
 	}
 
 	/**
-	 * Returns the name of this type: the fully qualified class name.
+	 * Return the name of this type: the fully qualified class name.
 	 */
 	public String getName() {
 		return ClassUtils.getQualifiedName(getType());
@@ -221,7 +220,7 @@ public class TypeDescriptor implements Serializable {
 	}
 
 	/**
-	 * The annotations associated with this type descriptor, if any.
+	 * Return the annotations associated with this type descriptor, if any.
 	 * @return the annotations, or an empty array if none
 	 */
 	public Annotation[] getAnnotations() {
@@ -236,6 +235,11 @@ public class TypeDescriptor implements Serializable {
 	 * @return <tt>true</tt> if the annotation is present
 	 */
 	public boolean hasAnnotation(Class<? extends Annotation> annotationType) {
+		if (this.annotatedElement.isEmpty()) {
+			// Shortcut: AnnotatedElementUtils would have to expect AnnotatedElement.getAnnotations()
+			// to return a copy of the array, whereas we can do it more efficiently here.
+			return false;
+		}
 		return AnnotatedElementUtils.isAnnotated(this.annotatedElement, annotationType);
 	}
 
@@ -247,6 +251,11 @@ public class TypeDescriptor implements Serializable {
 	 */
 	@SuppressWarnings("unchecked")
 	public <T extends Annotation> T getAnnotation(Class<T> annotationType) {
+		if (this.annotatedElement.isEmpty()) {
+			// Shortcut: AnnotatedElementUtils would have to expect AnnotatedElement.getAnnotations()
+			// to return a copy of the array, whereas we can do it more efficiently here.
+			return null;
+		}
 		return AnnotatedElementUtils.getMergedAnnotation(this.annotatedElement, annotationType);
 	}
 
@@ -430,32 +439,51 @@ public class TypeDescriptor implements Serializable {
 	}
 
 	@Override
-	public boolean equals(Object obj) {
+	public boolean equals(Object other) {
-		if (this == obj) {
+		if (this == other) {
 			return true;
 		}
-		if (!(obj instanceof TypeDescriptor)) {
+		if (!(other instanceof TypeDescriptor)) {
 			return false;
 		}
-		TypeDescriptor other = (TypeDescriptor) obj;
+		TypeDescriptor otherDesc = (TypeDescriptor) other;
-		if (!ObjectUtils.nullSafeEquals(getType(), other.getType())) {
+		if (getType() != otherDesc.getType()) {
 			return false;
 		}
-		if (!Arrays.equals(getAnnotations(), other.getAnnotations())) {
+		if (!annotationsMatch(otherDesc)) {
 			return false;
 		}
 		if (isCollection() || isArray()) {
-			return ObjectUtils.nullSafeEquals(getElementTypeDescriptor(), other.getElementTypeDescriptor());
+			return ObjectUtils.nullSafeEquals(getElementTypeDescriptor(), otherDesc.getElementTypeDescriptor());
 		}
 		else if (isMap()) {
-			return ObjectUtils.nullSafeEquals(getMapKeyTypeDescriptor(), other.getMapKeyTypeDescriptor()) &&
+			return (ObjectUtils.nullSafeEquals(getMapKeyTypeDescriptor(), otherDesc.getMapKeyTypeDescriptor()) &&
-					ObjectUtils.nullSafeEquals(getMapValueTypeDescriptor(), other.getMapValueTypeDescriptor());
+					ObjectUtils.nullSafeEquals(getMapValueTypeDescriptor(), otherDesc.getMapValueTypeDescriptor()));
 		}
 		else {
 			return true;
 		}
 	}
 
+	private boolean annotationsMatch(TypeDescriptor otherDesc) {
+		Annotation[] anns = getAnnotations();
+		Annotation[] otherAnns = otherDesc.getAnnotations();
+		if (anns == otherAnns) {
+			return true;
+		}
+		if (anns.length != otherAnns.length) {
+			return false;
+		}
+		if (anns.length > 0) {
+			for (int i = 0; i < anns.length; i++) {
+				if (anns[i] != otherAnns[i]) {
+					return false;
+				}
+			}
+		}
+		return true;
+	}
+
 	@Override
 	public int hashCode() {
 		return getType().hashCode();
@@ -471,6 +499,20 @@ public class TypeDescriptor implements Serializable {
 		return builder.toString();
 	}
 
+
+	/**
+	 * Create a new type descriptor for an object.
+	 * <p>Use this factory method to introspect a source object before asking the
+	 * conversion system to convert it to some another type.
+	 * <p>If the provided object is {@code null}, returns {@code null}, else calls
+	 * {@link #valueOf(Class)} to build a TypeDescriptor from the object's class.
+	 * @param source the source object
+	 * @return the type descriptor
+	 */
+	public static TypeDescriptor forObject(Object source) {
+		return (source != null ? valueOf(source.getClass()) : null);
+	}
+
 	/**
 	 * Create a new type descriptor from the given type.
 	 * <p>Use this to instruct the conversion system to convert an object to a
@@ -631,19 +673,6 @@ public class TypeDescriptor implements Serializable {
 		return nested(new TypeDescriptor(property), nestingLevel);
 	}
 
-	/**
-	 * Create a new type descriptor for an object.
-	 * <p>Use this factory method to introspect a source object before asking the
-	 * conversion system to convert it to some another type.
-	 * <p>If the provided object is {@code null}, returns {@code null}, else calls
-	 * {@link #valueOf(Class)} to build a TypeDescriptor from the object's class.
-	 * @param source the source object
-	 * @return the type descriptor
-	 */
-	public static TypeDescriptor forObject(Object source) {
-		return (source != null ? valueOf(source.getClass()) : null);
-	}
-
 	private static TypeDescriptor nested(TypeDescriptor typeDescriptor, int nestingLevel) {
 		ResolvableType nested = typeDescriptor.resolvableType;
 		for (int i = 0; i < nestingLevel; i++) {
@@ -714,6 +743,10 @@ public class TypeDescriptor implements Serializable {
 			return getAnnotations();
 		}
 
+		public boolean isEmpty() {
+			return ObjectUtils.isEmpty(this.annotations);
+		}
+
 		@Override
 		public boolean equals(Object other) {
 			return (this == other || (other instanceof AnnotatedElementAdapter &&

spring-core/src/test/java/org/springframework/core/convert/TypeDescriptorTests.java

@@ -598,15 +598,16 @@ public class TypeDescriptorTests {
 		TypeDescriptor t12 = new TypeDescriptor(getClass().getField("mapField"));
 		assertEquals(t11, t12);
 
-		TypeDescriptor t13 = new TypeDescriptor(new MethodParameter(getClass().getMethod("testAnnotatedMethod", String.class), 0));
+		MethodParameter testAnnotatedMethod = new MethodParameter(getClass().getMethod("testAnnotatedMethod", String.class), 0);
-		TypeDescriptor t14 = new TypeDescriptor(new MethodParameter(getClass().getMethod("testAnnotatedMethod", String.class), 0));
+		TypeDescriptor t13 = new TypeDescriptor(testAnnotatedMethod);
+		TypeDescriptor t14 = new TypeDescriptor(testAnnotatedMethod);
 		assertEquals(t13, t14);
 
-		TypeDescriptor t15 = new TypeDescriptor(new MethodParameter(getClass().getMethod("testAnnotatedMethod", String.class), 0));
+		TypeDescriptor t15 = new TypeDescriptor(testAnnotatedMethod);
 		TypeDescriptor t16 = new TypeDescriptor(new MethodParameter(getClass().getMethod("testAnnotatedMethodDifferentAnnotationValue", String.class), 0));
 		assertNotEquals(t15, t16);
 
-		TypeDescriptor t17 = new TypeDescriptor(new MethodParameter(getClass().getMethod("testAnnotatedMethod", String.class), 0));
+		TypeDescriptor t17 = new TypeDescriptor(testAnnotatedMethod);
 		TypeDescriptor t18 = new TypeDescriptor(new MethodParameter(getClass().getMethod("test5", String.class), 0));
 		assertNotEquals(t17, t18);
 	}