Skip to content

Commit bf03d0d

Browse files
author
Keith Donald
committed
converter caching
1 parent 3325249 commit bf03d0d

File tree

3 files changed

+142
-22
lines changed

3 files changed

+142
-22
lines changed

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

Lines changed: 30 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import org.springframework.core.MethodParameter;
2727
import org.springframework.util.Assert;
2828
import org.springframework.util.ClassUtils;
29+
import org.springframework.util.ObjectUtils;
2930

3031
/**
3132
* Context about a type to convert to.
@@ -37,7 +38,7 @@
3738
*/
3839
public class TypeDescriptor {
3940

40-
/** Constant defining an 'unknown' TypeDescriptor */
41+
/** Constant defining an 'unknown type' TypeDescriptor */
4142
public static final TypeDescriptor NULL = new TypeDescriptor();
4243

4344
private static final Map<Class<?>, TypeDescriptor> typeDescriptorCache = new HashMap<Class<?>, TypeDescriptor>();
@@ -73,9 +74,6 @@ public class TypeDescriptor {
7374

7475
private Object value;
7576

76-
private Annotation[] cachedFieldAnnotations;
77-
78-
7977
/**
8078
* Create a new type descriptor from a method or constructor parameter.
8179
* <p>Use this constructor when a target conversion point originates from a method parameter,
@@ -153,7 +151,7 @@ public static TypeDescriptor valueOf(Class<?> type) {
153151
TypeDescriptor desc = typeDescriptorCache.get(type);
154152
return (desc != null ? desc : new TypeDescriptor(type));
155153
}
156-
154+
157155
/**
158156
* Return the wrapped MethodParameter, if any.
159157
* <p>Note: Either MethodParameter or Field is available.
@@ -370,10 +368,7 @@ public TypeDescriptor getMapValueTypeDescriptor(Object value) {
370368
*/
371369
public Annotation[] getAnnotations() {
372370
if (this.field != null) {
373-
if (this.cachedFieldAnnotations == null) {
374-
this.cachedFieldAnnotations = this.field.getAnnotations();
375-
}
376-
return this.cachedFieldAnnotations;
371+
return this.field.getAnnotations();
377372
}
378373
else if (this.methodParameter != null) {
379374
if (this.methodParameter.getParameterIndex() < 0) {
@@ -438,6 +433,32 @@ else if (this.field != null) {
438433
return TypeDescriptor.valueOf(elementType);
439434
}
440435
}
436+
437+
public boolean equals(Object obj) {
438+
if (!(obj instanceof TypeDescriptor)) {
439+
return false;
440+
}
441+
TypeDescriptor td = (TypeDescriptor) obj;
442+
boolean annotatedTypeEquals = getType().equals(td.getType()) && ObjectUtils.nullSafeEquals(getAnnotations(), td.getAnnotations());
443+
if (isCollection()) {
444+
return annotatedTypeEquals && ObjectUtils.nullSafeEquals(getElementType(), td.getElementType());
445+
} else if (isMap()) {
446+
return annotatedTypeEquals && ObjectUtils.nullSafeEquals(getMapKeyType(), td.getMapKeyType()) && ObjectUtils.nullSafeEquals(getMapValueType(), td.getMapValueType());
447+
} else {
448+
return annotatedTypeEquals;
449+
}
450+
}
451+
452+
public int hashCode() {
453+
int annotatedTypeHash = getType().hashCode() + ObjectUtils.nullSafeHashCode(getAnnotations());
454+
if (isCollection()) {
455+
return annotatedTypeHash + ObjectUtils.nullSafeHashCode(getElementType());
456+
} else if (isMap()) {
457+
return annotatedTypeHash + ObjectUtils.nullSafeHashCode(getMapKeyType()) + ObjectUtils.nullSafeHashCode(getMapValueType());
458+
} else {
459+
return annotatedTypeHash;
460+
}
461+
}
441462

442463
/**
443464
* 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

Lines changed: 77 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@
2525
import java.util.List;
2626
import java.util.Map;
2727
import java.util.Set;
28+
import java.util.concurrent.ConcurrentHashMap;
2829

2930
import org.apache.commons.logging.Log;
3031
import org.apache.commons.logging.LogFactory;
@@ -65,11 +66,23 @@ public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor t
6566
}
6667
};
6768

69+
private static final GenericConverter NO_MATCH = new GenericConverter() {
70+
public Set<ConvertiblePair> getConvertibleTypes() {
71+
throw new UnsupportedOperationException();
72+
}
73+
public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor targetType) {
74+
throw new UnsupportedOperationException();
75+
}
76+
public String toString() {
77+
return "null";
78+
}
79+
};
6880

6981
private final Map<Class<?>, Map<Class<?>, MatchableConverters>> converters =
7082
new HashMap<Class<?>, Map<Class<?>, MatchableConverters>>(36);
7183

72-
84+
private final Map<ConverterCacheKey, GenericConverter> converterCache = new ConcurrentHashMap<ConverterCacheKey, GenericConverter>();
85+
7386
// implementing ConverterRegistry
7487

7588
public void addConverter(GenericConverter converter) {
@@ -219,20 +232,45 @@ protected Object convertNullSource(TypeDescriptor sourceType, TypeDescriptor tar
219232

220233
/**
221234
* Hook method to lookup the converter for a given sourceType/targetType pair.
222-
* First queries this ConversionService's converter map.
223-
* <p>Returns <code>null</code> if this ConversionService simply cannot convert
224-
* between sourceType and targetType. Subclasses may override.
235+
* First queries this ConversionService's converter cache.
236+
* On a cache miss, then performs an exhaustive search for a matching converter.
237+
* If no converter matches, returns the default converter.
238+
* Subclasses may override.
225239
* @param sourceType the source type to convert from
226240
* @param targetType the target type to convert to
227241
* @return the generic converter that will perform the conversion, or <code>null</code> if no suitable converter was found
242+
* @see #getDefaultConverter(TypeDescriptor, TypeDescriptor)
228243
*/
229244
protected GenericConverter getConverter(TypeDescriptor sourceType, TypeDescriptor targetType) {
230-
GenericConverter converter = findConverterForClassPair(sourceType, targetType);
245+
ConverterCacheKey key = new ConverterCacheKey(sourceType, targetType);
246+
GenericConverter converter = converterCache.get(key);
231247
if (converter != null) {
232-
return converter;
233-
}
234-
else {
235-
return getDefaultConverter(sourceType, targetType);
248+
if (logger.isDebugEnabled()) {
249+
logger.debug("Matched cached converter " + converter);
250+
}
251+
return converter != NO_MATCH ? converter : null;
252+
} else {
253+
converter = findConverterForClassPair(sourceType, targetType);
254+
if (converter != null) {
255+
if (logger.isTraceEnabled()) {
256+
logger.trace("Caching under " + key);
257+
}
258+
converterCache.put(key, converter);
259+
return converter;
260+
}
261+
converter = getDefaultConverter(sourceType, targetType);
262+
if (converter != null) {
263+
if (logger.isTraceEnabled()) {
264+
logger.trace("Caching under " + key);
265+
}
266+
converterCache.put(key, converter);
267+
return converter;
268+
}
269+
if (logger.isTraceEnabled()) {
270+
logger.trace("Caching NO_MATCH under " + key);
271+
}
272+
converterCache.put(key, NO_MATCH);
273+
return null;
236274
}
237275
}
238276

@@ -297,7 +335,7 @@ private GenericConverter findConverterForClassPair(TypeDescriptor sourceType, Ty
297335
while (!classQueue.isEmpty()) {
298336
Class<?> currentClass = classQueue.removeLast();
299337
if (logger.isTraceEnabled()) {
300-
logger.trace("Looking for converters indexed by sourceType [" + currentClass.getName() + "]");
338+
logger.trace("Searching for converters indexed by sourceType [" + currentClass.getName() + "]");
301339
}
302340
Map<Class<?>, MatchableConverters> converters = getTargetConvertersForSource(currentClass);
303341
GenericConverter converter = getMatchingConverterForTarget(sourceType, targetType, converters);
@@ -318,7 +356,7 @@ private GenericConverter findConverterForClassPair(TypeDescriptor sourceType, Ty
318356
while (!classQueue.isEmpty()) {
319357
Class<?> currentClass = classQueue.removeLast();
320358
if (logger.isTraceEnabled()) {
321-
logger.trace("Looking for converters indexed by sourceType [" + currentClass.getName() + "]");
359+
logger.trace("Searching for converters indexed by sourceType [" + currentClass.getName() + "]");
322360
}
323361
Map<Class<?>, MatchableConverters> converters = getTargetConvertersForSource(currentClass);
324362
GenericConverter converter = getMatchingConverterForTarget(sourceType, targetType, converters);
@@ -552,5 +590,33 @@ public String toString() {
552590
}
553591
}
554592
}
593+
594+
private static class ConverterCacheKey {
595+
596+
private TypeDescriptor sourceType;
597+
598+
private TypeDescriptor targetType;
599+
600+
public ConverterCacheKey(TypeDescriptor sourceType, TypeDescriptor targetType) {
601+
this.sourceType = sourceType;
602+
this.targetType = targetType;
603+
}
604+
605+
public boolean equals(Object o) {
606+
if (!(o instanceof ConverterCacheKey)) {
607+
return false;
608+
}
609+
ConverterCacheKey key = (ConverterCacheKey) o;
610+
return sourceType.equals(key.sourceType) && targetType.equals(key.targetType);
611+
}
612+
613+
public int hashCode() {
614+
return sourceType.hashCode() + targetType.hashCode();
615+
}
616+
617+
public String toString() {
618+
return "[ConverterCacheKey sourceType = " + sourceType + ", targetType = " + targetType + "]";
619+
}
620+
}
555621

556622
}

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

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,11 +16,16 @@
1616

1717
package org.springframework.core.convert;
1818

19-
import java.util.List;
20-
2119
import static junit.framework.Assert.assertEquals;
2220
import static junit.framework.Assert.assertTrue;
2321
import static org.junit.Assert.assertFalse;
22+
23+
import java.util.ArrayList;
24+
import java.util.Date;
25+
import java.util.HashMap;
26+
import java.util.List;
27+
import java.util.Map;
28+
2429
import org.junit.Test;
2530

2631
/**
@@ -66,4 +71,32 @@ public void complexTypeDescriptors() throws Exception {
6671
assertEquals("[TypeDescriptor java.util.List[]]",typeDescriptor.asString());
6772
}
6873

74+
@Test
75+
public void testEquals() throws Exception {
76+
TypeDescriptor t1 = TypeDescriptor.valueOf(String.class);
77+
TypeDescriptor t2 = TypeDescriptor.valueOf(String.class);
78+
TypeDescriptor t3 = TypeDescriptor.valueOf(Date.class);
79+
TypeDescriptor t4 = TypeDescriptor.valueOf(Date.class);
80+
TypeDescriptor t5 = TypeDescriptor.valueOf(List.class);
81+
TypeDescriptor t6 = TypeDescriptor.valueOf(List.class);
82+
TypeDescriptor t7 = TypeDescriptor.valueOf(Map.class);
83+
TypeDescriptor t8 = TypeDescriptor.valueOf(Map.class);
84+
assertEquals(t1, t2);
85+
assertEquals(t3, t4);
86+
assertEquals(t5, t6);
87+
assertEquals(t7, t8);
88+
89+
TypeDescriptor t9 = new TypeDescriptor(getClass().getField("listField"));
90+
TypeDescriptor t10 = new TypeDescriptor(getClass().getField("listField"));
91+
assertEquals(t9, t10);
92+
93+
TypeDescriptor t11 = new TypeDescriptor(getClass().getField("mapField"));
94+
TypeDescriptor t12 = new TypeDescriptor(getClass().getField("mapField"));
95+
assertEquals(t11, t12);
96+
}
97+
98+
public List<Integer> listField = new ArrayList<Integer>();
99+
100+
public Map<String, Integer> mapField = new HashMap<String, Integer>();
101+
69102
}

0 commit comments

Comments
 (0)