converter caching

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

@@ -26,6 +26,7 @@ import org.springframework.core.GenericCollectionTypeResolver;
 import org.springframework.core.MethodParameter;
 import org.springframework.util.Assert;
 import org.springframework.util.ClassUtils;
+import org.springframework.util.ObjectUtils;
 
 /**
  * Context about a type to convert to.
@@ -37,7 +38,7 @@ import org.springframework.util.ClassUtils;
  */
 public class TypeDescriptor {
 
-	/** Constant defining an 'unknown' TypeDescriptor */
+	/** Constant defining an 'unknown type' TypeDescriptor */
 	public static final TypeDescriptor NULL = new TypeDescriptor();
 
 	private static final Map<Class<?>, TypeDescriptor> typeDescriptorCache = new HashMap<Class<?>, TypeDescriptor>();
@@ -73,9 +74,6 @@ public class TypeDescriptor {
 
 	private Object value;
 
-	private Annotation[] cachedFieldAnnotations;
-
-
 	/**
 	 * Create a new type descriptor from a method or constructor parameter.
 	 * <p>Use this constructor when a target conversion point originates from a method parameter,
@@ -153,7 +151,7 @@ public class TypeDescriptor {
 		TypeDescriptor desc = typeDescriptorCache.get(type);
 		return (desc != null ? desc : new TypeDescriptor(type));
 	}
-	
+
 	/**
 	 * Return the wrapped MethodParameter, if any.
 	 * <p>Note: Either MethodParameter or Field is available.
@@ -370,10 +368,7 @@ public class TypeDescriptor {
 	 */
 	public Annotation[] getAnnotations() {
 		if (this.field != null) {
-			if (this.cachedFieldAnnotations == null) {
+			return this.field.getAnnotations();
-				this.cachedFieldAnnotations = this.field.getAnnotations();
-			}
-			return this.cachedFieldAnnotations;
 		}
 		else if (this.methodParameter != null) {
 			if (this.methodParameter.getParameterIndex() < 0) {
@@ -438,6 +433,32 @@ public class TypeDescriptor {
 			return TypeDescriptor.valueOf(elementType);
 		}
 	}
+
+	public boolean equals(Object obj) {
+		if (!(obj instanceof TypeDescriptor)) {
+			return false;
+		}
+		TypeDescriptor td = (TypeDescriptor) obj;
+		boolean annotatedTypeEquals = getType().equals(td.getType()) && ObjectUtils.nullSafeEquals(getAnnotations(), td.getAnnotations());
+		if (isCollection()) {
+			return annotatedTypeEquals && ObjectUtils.nullSafeEquals(getElementType(), td.getElementType());
+		} else if (isMap()) { 
+			return annotatedTypeEquals && ObjectUtils.nullSafeEquals(getMapKeyType(), td.getMapKeyType()) && ObjectUtils.nullSafeEquals(getMapValueType(), td.getMapValueType());
+		} else {
+			return annotatedTypeEquals; 
+		}
+	}
+	
+	public int hashCode() {
+		int annotatedTypeHash = getType().hashCode() + ObjectUtils.nullSafeHashCode(getAnnotations());
+		if (isCollection()) {
+			return annotatedTypeHash + ObjectUtils.nullSafeHashCode(getElementType()); 
+		} else if (isMap()) {
+			return annotatedTypeHash + ObjectUtils.nullSafeHashCode(getMapKeyType()) + ObjectUtils.nullSafeHashCode(getMapValueType());
+		} else {
+			return annotatedTypeHash;
+		}
+	}
 	
 	/**
 	 * A textual representation of the type descriptor (eg. Map<String,Foo>) for use in messages

org.springframework.core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java

@@ -25,6 +25,7 @@ import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.concurrent.ConcurrentHashMap;
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
@@ -65,11 +66,23 @@ public class GenericConversionService implements ConversionService, ConverterReg
 		}
 	};
 
+	private static final GenericConverter NO_MATCH = new GenericConverter() {
+		public Set<ConvertiblePair> getConvertibleTypes() {
+			throw new UnsupportedOperationException();
+		}
+		public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {
+			throw new UnsupportedOperationException();
+		}
+		public String toString() {
+			return "null";
+		}
+	};
 
 	private final Map<Class<?>, Map<Class<?>, MatchableConverters>> converters =
 			new HashMap<Class<?>, Map<Class<?>, MatchableConverters>>(36);
 
-
+	private final Map<ConverterCacheKey, GenericConverter> converterCache = new ConcurrentHashMap<ConverterCacheKey, GenericConverter>();
+	
 	// implementing ConverterRegistry
 
 	public void addConverter(GenericConverter converter) {
@@ -219,20 +232,45 @@ public class GenericConversionService implements ConversionService, ConverterReg
 
 	/**
 	 * Hook method to lookup the converter for a given sourceType/targetType pair.
-	 * First queries this ConversionService's converter map.
+	 * First queries this ConversionService's converter cache.
-	 * <p>Returns <code>null</code> if this ConversionService simply cannot convert
+	 * On a cache miss, then performs an exhaustive search for a matching converter.
-	 * between sourceType and targetType. Subclasses may override.
+	 * If no converter matches, returns the default converter.
+	 * Subclasses may override.
 	 * @param sourceType the source type to convert from
 	 * @param targetType the target type to convert to
 	 * @return the generic converter that will perform the conversion, or <code>null</code> if no suitable converter was found
+	 * @see #getDefaultConverter(TypeDescriptor, TypeDescriptor)
 	 */
 	protected GenericConverter getConverter(TypeDescriptor sourceType, TypeDescriptor targetType) {
-		GenericConverter converter = findConverterForClassPair(sourceType, targetType);
+		ConverterCacheKey key = new ConverterCacheKey(sourceType, targetType);
+		GenericConverter converter = converterCache.get(key);
 		if (converter != null) {
-			return converter;
+			if (logger.isDebugEnabled()) {
-		}
+				logger.debug("Matched cached converter " + converter);
-		else {
+			}
-			return getDefaultConverter(sourceType, targetType);
+			return converter != NO_MATCH ? converter : null; 
+		} else {			
+			converter = findConverterForClassPair(sourceType, targetType);
+			if (converter != null) {
+				if (logger.isTraceEnabled()) {
+					logger.trace("Caching under " + key);
+				}
+				converterCache.put(key, converter);
+				return converter;
+			}
+			converter = getDefaultConverter(sourceType, targetType);
+			if (converter != null) {
+				if (logger.isTraceEnabled()) {
+					logger.trace("Caching under " + key);
+				}
+				converterCache.put(key, converter);
+				return converter;				
+			}
+			if (logger.isTraceEnabled()) {
+				logger.trace("Caching NO_MATCH under " + key);
+			}
+			converterCache.put(key, NO_MATCH);
+			return null;
 		}
 	}
 
@@ -297,7 +335,7 @@ public class GenericConversionService implements ConversionService, ConverterReg
 			while (!classQueue.isEmpty()) {
 				Class<?> currentClass = classQueue.removeLast();
 				if (logger.isTraceEnabled()) {
-					logger.trace("Looking for converters indexed by sourceType [" + currentClass.getName() + "]");
+					logger.trace("Searching for converters indexed by sourceType [" + currentClass.getName() + "]");
 				}
 				Map<Class<?>, MatchableConverters> converters = getTargetConvertersForSource(currentClass);
 				GenericConverter converter = getMatchingConverterForTarget(sourceType, targetType, converters);
@@ -318,7 +356,7 @@ public class GenericConversionService implements ConversionService, ConverterReg
 			while (!classQueue.isEmpty()) {
 				Class<?> currentClass = classQueue.removeLast();
 				if (logger.isTraceEnabled()) {
-					logger.trace("Looking for converters indexed by sourceType [" + currentClass.getName() + "]");
+					logger.trace("Searching for converters indexed by sourceType [" + currentClass.getName() + "]");
 				}
 				Map<Class<?>, MatchableConverters> converters = getTargetConvertersForSource(currentClass);
 				GenericConverter converter = getMatchingConverterForTarget(sourceType, targetType, converters);
@@ -552,5 +590,33 @@ public class GenericConversionService implements ConversionService, ConverterReg
 			}
 		}
 	}
+	
+	private static class ConverterCacheKey {
+
+		private TypeDescriptor sourceType;
+		
+		private TypeDescriptor targetType;
+		
+		public ConverterCacheKey(TypeDescriptor sourceType, TypeDescriptor targetType) {
+			this.sourceType = sourceType;
+			this.targetType = targetType;
+		}
+		
+		public boolean equals(Object o) {
+			if (!(o instanceof ConverterCacheKey)) {
+				return false;
+			}
+			ConverterCacheKey key = (ConverterCacheKey) o;
+			return sourceType.equals(key.sourceType) && targetType.equals(key.targetType);
+		}
+		
+		public int hashCode() {
+			return sourceType.hashCode() + targetType.hashCode();
+		}
+		
+		public String toString() {
+			return "[ConverterCacheKey sourceType = " + sourceType + ", targetType = " + targetType + "]";
+		}
+	}
 
 }

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

@@ -16,11 +16,16 @@
 
 package org.springframework.core.convert;
 
-import java.util.List;
-
 import static junit.framework.Assert.assertEquals;
 import static junit.framework.Assert.assertTrue;
 import static org.junit.Assert.assertFalse;
+
+import java.util.ArrayList;
+import java.util.Date;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
 import org.junit.Test;
 
 /**
@@ -66,4 +71,32 @@ public class TypeDescriptorTests {
 		assertEquals("[TypeDescriptor java.util.List[]]",typeDescriptor.asString());
 	}
 	
+	@Test
+	public void testEquals() throws Exception {
+		TypeDescriptor t1 = TypeDescriptor.valueOf(String.class);
+		TypeDescriptor t2 = TypeDescriptor.valueOf(String.class);
+		TypeDescriptor t3 = TypeDescriptor.valueOf(Date.class);
+		TypeDescriptor t4 = TypeDescriptor.valueOf(Date.class);
+		TypeDescriptor t5 = TypeDescriptor.valueOf(List.class);
+		TypeDescriptor t6 = TypeDescriptor.valueOf(List.class);
+		TypeDescriptor t7 = TypeDescriptor.valueOf(Map.class);
+		TypeDescriptor t8 = TypeDescriptor.valueOf(Map.class);
+		assertEquals(t1, t2);
+		assertEquals(t3, t4);
+		assertEquals(t5, t6);
+		assertEquals(t7, t8);
+		
+		TypeDescriptor t9 = new TypeDescriptor(getClass().getField("listField"));
+		TypeDescriptor t10 = new TypeDescriptor(getClass().getField("listField"));
+		assertEquals(t9, t10);
+
+		TypeDescriptor t11 = new TypeDescriptor(getClass().getField("mapField"));
+		TypeDescriptor t12 = new TypeDescriptor(getClass().getField("mapField"));
+		assertEquals(t11, t12);
+	}
+	
+	public List<Integer> listField = new ArrayList<Integer>();
+	
+	public Map<String, Integer> mapField = new HashMap<String, Integer>();
+
 }