From bf03d0d56f9fcfc4757bd3ab35e40c3f528fe710 Mon Sep 17 00:00:00 2001 From: Keith Donald Date: Sun, 18 Apr 2010 21:43:00 +0000 Subject: [PATCH] converter caching --- .../core/convert/TypeDescriptor.java | 39 ++++++-- .../support/GenericConversionService.java | 88 ++++++++++++++++--- .../core/convert/TypeDescriptorTests.java | 37 +++++++- 3 files changed, 142 insertions(+), 22 deletions(-) diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/TypeDescriptor.java b/org.springframework.core/src/main/java/org/springframework/core/convert/TypeDescriptor.java index 487fe25b71..ee63dbb050 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/convert/TypeDescriptor.java +++ b/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, TypeDescriptor> typeDescriptorCache = new HashMap, 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. *

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. *

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) { - this.cachedFieldAnnotations = this.field.getAnnotations(); - } - return this.cachedFieldAnnotations; + return this.field.getAnnotations(); } 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) for use in messages diff --git a/org.springframework.core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java b/org.springframework.core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java index 5d05efb145..defab1c80e 100644 --- a/org.springframework.core/src/main/java/org/springframework/core/convert/support/GenericConversionService.java +++ b/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 getConvertibleTypes() { + throw new UnsupportedOperationException(); + } + public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) { + throw new UnsupportedOperationException(); + } + public String toString() { + return "null"; + } + }; private final Map, Map, MatchableConverters>> converters = new HashMap, Map, MatchableConverters>>(36); - + private final Map converterCache = new ConcurrentHashMap(); + // 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. - *

Returns null if this ConversionService simply cannot convert - * between sourceType and targetType. Subclasses may override. + * First queries this ConversionService's converter cache. + * On a cache miss, then performs an exhaustive search for a matching converter. + * 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 null 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; - } - else { - return getDefaultConverter(sourceType, targetType); + if (logger.isDebugEnabled()) { + logger.debug("Matched cached converter " + converter); + } + 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, 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, 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 + "]"; + } + } } diff --git a/org.springframework.core/src/test/java/org/springframework/core/convert/TypeDescriptorTests.java b/org.springframework.core/src/test/java/org/springframework/core/convert/TypeDescriptorTests.java index 08155227c1..4d00866c08 100644 --- a/org.springframework.core/src/test/java/org/springframework/core/convert/TypeDescriptorTests.java +++ b/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 listField = new ArrayList(); + + public Map mapField = new HashMap(); + }