DATAMONGO-712 - Another round of performance improvements.

Refactored CustomConversions to unify locked access to the cached types. Added a cache for raw-write-targets so that they’re cached, too.

DBObjectAccessor now avoids expensive code paths for both reads and writes in case of simple field names.

MappingMongoConverter now eagerly skips conversions of simple types in case the value is already assignable to the target type.

QueryMapper now checks the ConversionService and only triggers a conversion if it’s actually capable of doing so instead of catching a more expensive exception.

CachingMongoPersistentProperty now also caches usePropertyAccess() and isTransient() as they’re used quite frequently.

Related ticket: DATACMNS-637.

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/CustomConversions.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2011-2014 the original author or authors.
+ * Copyright 2011-2015 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.
@@ -17,14 +17,15 @@ package org.springframework.data.mongodb.core.convert;
 
 import java.util.ArrayList;
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.LinkedHashSet;
 import java.util.List;
 import java.util.Locale;
+import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.ConcurrentMap;
 
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -36,9 +37,9 @@ import org.springframework.core.convert.converter.GenericConverter;
 import org.springframework.core.convert.converter.GenericConverter.ConvertiblePair;
 import org.springframework.core.convert.support.GenericConversionService;
 import org.springframework.data.convert.JodaTimeConverters;
-import org.springframework.data.convert.ThreeTenBackPortConverters;
 import org.springframework.data.convert.Jsr310Converters;
 import org.springframework.data.convert.ReadingConverter;
+import org.springframework.data.convert.ThreeTenBackPortConverters;
 import org.springframework.data.convert.WritingConverter;
 import org.springframework.data.mapping.model.SimpleTypeHolder;
 import org.springframework.data.mongodb.core.convert.MongoConverters.BigDecimalToStringConverter;
@@ -50,6 +51,7 @@ import org.springframework.data.mongodb.core.convert.MongoConverters.StringToURL
 import org.springframework.data.mongodb.core.convert.MongoConverters.TermToStringConverter;
 import org.springframework.data.mongodb.core.convert.MongoConverters.URLToStringConverter;
 import org.springframework.data.mongodb.core.mapping.MongoSimpleTypes;
+import org.springframework.data.util.CacheValue;
 import org.springframework.util.Assert;
 
 /**
@@ -73,10 +75,13 @@ public class CustomConversions {
 	private final Set<ConvertiblePair> writingPairs;
 	private final Set<Class<?>> customSimpleTypes;
 	private final SimpleTypeHolder simpleTypeHolder;
-	private final ConcurrentMap<ConvertiblePair, CacheValue> customReadTargetTypes;
 
 	private final List<Object> converters;
 
+	private final Map<ConvertiblePair, CacheValue<Class<?>>> customReadTargetTypes;
+	private final Map<ConvertiblePair, CacheValue<Class<?>>> customWriteTargetTypes;
+	private final Map<Class<?>, CacheValue<Class<?>>> rawWriteTargetTypes;
+
 	/**
 	 * Creates an empty {@link CustomConversions} object.
 	 */
@@ -96,7 +101,9 @@ public class CustomConversions {
 		this.readingPairs = new LinkedHashSet<ConvertiblePair>();
 		this.writingPairs = new LinkedHashSet<ConvertiblePair>();
 		this.customSimpleTypes = new HashSet<Class<?>>();
-		this.customReadTargetTypes = new ConcurrentHashMap<GenericConverter.ConvertiblePair, CacheValue>();
+		this.customReadTargetTypes = new ConcurrentHashMap<ConvertiblePair, CacheValue<Class<?>>>();
+		this.customWriteTargetTypes = new ConcurrentHashMap<ConvertiblePair, CacheValue<Class<?>>>();
+		this.rawWriteTargetTypes = new ConcurrentHashMap<Class<?>, CacheValue<Class<?>>>();
 
 		List<Object> toRegister = new ArrayList<Object>();
 
@@ -242,70 +249,103 @@ public class CustomConversions {
 	 * @param sourceType must not be {@literal null}
 	 * @return
 	 */
-	public Class<?> getCustomWriteTarget(Class<?> sourceType) {
-		return getCustomWriteTarget(sourceType, null);
+	public Class<?> getCustomWriteTarget(final Class<?> sourceType) {
+
+		return getOrCreateAndCache(sourceType, rawWriteTargetTypes, new Producer() {
+
+			@Override
+			public Class<?> get() {
+				return getCustomTarget(sourceType, null, writingPairs);
+			}
+		});
 	}
 
 	/**
-	 * Returns the target type we can write an inject of the given source type to. The returned type might be a subclass
-	 * of the given expected type though. If {@code expectedTargetType} is {@literal null} we will simply return the first
-	 * target type matching or {@literal null} if no conversion can be found.
+	 * Returns the target type we can readTargetWriteLocl an inject of the given source type to. The returned type might
+	 * be a subclass of the given expected type though. If {@code expectedTargetType} is {@literal null} we will simply
+	 * return the first target type matching or {@literal null} if no conversion can be found.
 	 * 
 	 * @param sourceType must not be {@literal null}
 	 * @param requestedTargetType
 	 * @return
 	 */
-	public Class<?> getCustomWriteTarget(Class<?> sourceType, Class<?> requestedTargetType) {
+	public Class<?> getCustomWriteTarget(final Class<?> sourceType, final Class<?> requestedTargetType) {
 
-		Assert.notNull(sourceType);
+		if (requestedTargetType == null) {
+			return getCustomWriteTarget(sourceType);
+		}
 
-		return getCustomTarget(sourceType, requestedTargetType, writingPairs);
+		return getOrCreateAndCache(new ConvertiblePair(sourceType, requestedTargetType), customWriteTargetTypes,
+				new Producer() {
+
+					@Override
+					public Class<?> get() {
+						return getCustomTarget(sourceType, requestedTargetType, writingPairs);
+					}
+				});
 	}
 
 	/**
-	 * Returns whether we have a custom conversion registered to write into a Mongo native type. The returned type might
-	 * be a subclass of the given expected type though.
+	 * Returns whether we have a custom conversion registered to readTargetWriteLocl into a Mongo native type. The
+	 * returned type might be a subclass of the given expected type though.
 	 * 
 	 * @param sourceType must not be {@literal null}
 	 * @return
 	 */
 	public boolean hasCustomWriteTarget(Class<?> sourceType) {
-
-		Assert.notNull(sourceType);
 		return hasCustomWriteTarget(sourceType, null);
 	}
 
 	/**
-	 * Returns whether we have a custom conversion registered to write an object of the given source type into an object
-	 * of the given Mongo native target type.
+	 * Returns whether we have a custom conversion registered to readTargetWriteLocl an object of the given source type
+	 * into an object of the given Mongo native target type.
 	 * 
 	 * @param sourceType must not be {@literal null}.
 	 * @param requestedTargetType
 	 * @return
 	 */
 	public boolean hasCustomWriteTarget(Class<?> sourceType, Class<?> requestedTargetType) {
-
-		Assert.notNull(sourceType);
 		return getCustomWriteTarget(sourceType, requestedTargetType) != null;
 	}
 
 	/**
-	 * Returns whether we have a custom conversion registered to read the given source into the given target type.
+	 * Returns whether we have a custom conversion registered to readTargetReadLock the given source into the given target
+	 * type.
 	 * 
 	 * @param sourceType must not be {@literal null}
 	 * @param requestedTargetType must not be {@literal null}
 	 * @return
 	 */
 	public boolean hasCustomReadTarget(Class<?> sourceType, Class<?> requestedTargetType) {
-
-		Assert.notNull(sourceType);
-		Assert.notNull(requestedTargetType);
-
 		return getCustomReadTarget(sourceType, requestedTargetType) != null;
 	}
 
 	/**
-	 * Inspects the given {@link ConvertiblePair} for ones that have a source compatible type as source. Additionally
+	 * Returns the actual target type for the given {@code sourceType} and {@code requestedTargetType}. Note that the
+	 * returned {@link Class} could be an assignable type to the given {@code requestedTargetType}.
+	 * 
+	 * @param sourceType must not be {@literal null}.
+	 * @param requestedTargetType can be {@literal null}.
+	 * @return
+	 */
+	private Class<?> getCustomReadTarget(final Class<?> sourceType, final Class<?> requestedTargetType) {
+
+		if (requestedTargetType == null) {
+			return null;
+		}
+
+		return getOrCreateAndCache(new ConvertiblePair(sourceType, requestedTargetType), customReadTargetTypes,
+				new Producer() {
+
+					@Override
+					public Class<?> get() {
+						return getCustomTarget(sourceType, requestedTargetType, readingPairs);
+					}
+				});
+	}
+
+	/**
+	 * Inspects the given {@link ConvertiblePair}s for ones that have a source compatible type as source. Additionally
 	 * checks assignability of the target type if one is given.
 	 * 
 	 * @param sourceType must not be {@literal null}.
@@ -314,11 +354,15 @@ public class CustomConversions {
 	 * @return
 	 */
 	private static Class<?> getCustomTarget(Class<?> sourceType, Class<?> requestedTargetType,
-			Iterable<ConvertiblePair> pairs) {
+			Collection<ConvertiblePair> pairs) {
 
 		Assert.notNull(sourceType);
 		Assert.notNull(pairs);
 
+		if (requestedTargetType != null && pairs.contains(new ConvertiblePair(sourceType, requestedTargetType))) {
+			return requestedTargetType;
+		}
+
 		for (ConvertiblePair typePair : pairs) {
 			if (typePair.getSourceType().isAssignableFrom(sourceType)) {
 				Class<?> targetType = typePair.getTargetType();
@@ -332,32 +376,31 @@ public class CustomConversions {
 	}
 
 	/**
-	 * Returns the actual target type for the given {@code sourceType} and {@code requestedTargetType}. Note that the
-	 * returned {@link Class} could be an assignable type to the given {@code requestedTargetType}.
+	 * Will try to find a value for the given key in the given cache or produce one using the given {@link Producer} and
+	 * store it in the cache.
 	 * 
-	 * @param sourceType must not be {@literal null}.
-	 * @param requestedTargetType can be {@literal null}.
+	 * @param key the key to lookup a potentially existing value, must not be {@literal null}.
+	 * @param cache the cache to find the value in, must not be {@literal null}.
+	 * @param producer the {@link Producer} to create values to cache, must not be {@literal null}.
 	 * @return
 	 */
-	private Class<?> getCustomReadTarget(Class<?> sourceType, Class<?> requestedTargetType) {
+	private static <T> Class<?> getOrCreateAndCache(T key, Map<T, CacheValue<Class<?>>> cache, Producer producer) {
 
-		Assert.notNull(sourceType);
+		CacheValue<Class<?>> cacheValue = cache.get(key);
 
-		if (requestedTargetType == null) {
-			return null;
+		if (cacheValue != null) {
+			return cacheValue.getValue();
 		}
 
-		ConvertiblePair lookupKey = new ConvertiblePair(sourceType, requestedTargetType);
-		CacheValue readTargetTypeValue = customReadTargetTypes.get(lookupKey);
+		Class<?> type = producer.get();
+		cache.put(key, CacheValue.<Class<?>> ofNullable(type));
 
-		if (readTargetTypeValue != null) {
-			return readTargetTypeValue.getType();
-		}
+		return type;
+	}
 
-		readTargetTypeValue = CacheValue.of(getCustomTarget(sourceType, requestedTargetType, readingPairs));
-		CacheValue cacheValue = customReadTargetTypes.putIfAbsent(lookupKey, readTargetTypeValue);
+	private interface Producer {
 
-		return cacheValue != null ? cacheValue.getType() : readTargetTypeValue.getType();
+		Class<?> get();
 	}
 
 	@WritingConverter
@@ -377,30 +420,4 @@ public class CustomConversions {
 			return source.toString();
 		}
 	}
-
-	/**
-	 * Wrapper to safely store {@literal null} values in the type cache.
-	 * 
-	 * @author Patryk Wasik
-	 * @author Oliver Gierke
-	 * @author Thomas Darimont
-	 */
-	private static class CacheValue {
-
-		private static final CacheValue ABSENT = new CacheValue(null);
-
-		private final Class<?> type;
-
-		public CacheValue(Class<?> type) {
-			this.type = type;
-		}
-
-		public Class<?> getType() {
-			return type;
-		}
-
-		static CacheValue of(Class<?> type) {
-			return type == null ? ABSENT : new CacheValue(type);
-		}
-	}
 }

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DBObjectAccessor.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2013 the original author or authors.
+ * Copyright 2013-2015 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.
@@ -34,7 +34,7 @@ import com.mongodb.DBObject;
  */
 class DBObjectAccessor {
 
-	private final DBObject dbObject;
+	private final BasicDBObject dbObject;
 
 	/**
 	 * Creates a new {@link DBObjectAccessor} for the given {@link DBObject}.
@@ -46,7 +46,7 @@ class DBObjectAccessor {
 		Assert.notNull(dbObject, "DBObject must not be null!");
 		Assert.isInstanceOf(BasicDBObject.class, dbObject, "Given DBObject must be a BasicDBObject!");
 
-		this.dbObject = dbObject;
+		this.dbObject = (BasicDBObject) dbObject;
 	}
 
 	/**
@@ -62,6 +62,11 @@ class DBObjectAccessor {
 		Assert.notNull(prop, "MongoPersistentProperty must not be null!");
 		String fieldName = prop.getFieldName();
 
+		if (!fieldName.contains(".")) {
+			dbObject.put(fieldName, value);
+			return;
+		}
+
 		Iterator<String> parts = Arrays.asList(fieldName.split("\\.")).iterator();
 		DBObject dbObject = this.dbObject;
 
@@ -87,12 +92,16 @@ class DBObjectAccessor {
 	 * @param property must not be {@literal null}.
 	 * @return
 	 */
-	@SuppressWarnings("unchecked")
 	public Object get(MongoPersistentProperty property) {
 
 		String fieldName = property.getFieldName();
+
+		if (!fieldName.contains(".")) {
+			return this.dbObject.get(fieldName);
+		}
+
 		Iterator<String> parts = Arrays.asList(fieldName.split("\\.")).iterator();
-		Map<Object, Object> source = this.dbObject.toMap();
+		Map<String, Object> source = this.dbObject;
 		Object result = null;
 
 		while (source != null && parts.hasNext()) {
@@ -108,14 +117,14 @@ class DBObjectAccessor {
 	}
 
 	@SuppressWarnings("unchecked")
-	private Map<Object, Object> getAsMap(Object source) {
+	private Map<String, Object> getAsMap(Object source) {
 
 		if (source instanceof BasicDBObject) {
-			return ((DBObject) source).toMap();
+			return (BasicDBObject) source;
 		}
 
 		if (source instanceof Map) {
-			return (Map<Object, Object>) source;
+			return (Map<String, Object>) source;
 		}
 
 		return null;

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java

@@ -803,7 +803,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 	@SuppressWarnings({ "rawtypes", "unchecked" })
 	private Object getPotentiallyConvertedSimpleRead(Object value, Class<?> target) {
 
-		if (value == null || target == null) {
+		if (value == null || target == null || target.isAssignableFrom(value.getClass())) {
 			return value;
 		}
 
@@ -815,7 +815,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App
 			return Enum.valueOf((Class<Enum>) target, value.toString());
 		}
 
-		return target.isAssignableFrom(value.getClass()) ? value : conversionService.convert(value, target);
+		return conversionService.convert(value, target);
 	}
 
 	protected DBRef createDBRef(Object target, MongoPersistentProperty property) {

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java

@@ -464,13 +464,20 @@ public class QueryMapper {
 	 */
 	public Object convertId(Object id) {
 
-		try {
-			return conversionService.convert(id, ObjectId.class);
-		} catch (ConversionException e) {
-			// Ignore
+		if (id == null) {
+			return null;
 		}
 
-		return delegateConvertToMongoType(id, null);
+		if (id instanceof String) {
+			return ObjectId.isValid(id.toString()) ? conversionService.convert(id, ObjectId.class) : id;
+		}
+
+		try {
+			return conversionService.canConvert(id.getClass(), ObjectId.class) ? conversionService
+					.convert(id, ObjectId.class) : delegateConvertToMongoType(id, null);
+		} catch (ConversionException o_O) {
+			return delegateConvertToMongoType(id, null);
+		}
 	}
 
 	/**

spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/CachingMongoPersistentProperty.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2011-2014 the original author or authors.
+ * Copyright 2011-2015 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.
@@ -31,6 +31,8 @@ public class CachingMongoPersistentProperty extends BasicMongoPersistentProperty
 	private Boolean isIdProperty;
 	private Boolean isAssociation;
 	private String fieldName;
+	private Boolean usePropertyAccess;
+	private Boolean isTransient;
 
 	/**
 	 * Creates a new {@link CachingMongoPersistentProperty}.
@@ -85,4 +87,32 @@ public class CachingMongoPersistentProperty extends BasicMongoPersistentProperty
 
 		return this.fieldName;
 	}
+
+	/* 
+	 * (non-Javadoc)
+	 * @see org.springframework.data.mapping.model.AnnotationBasedPersistentProperty#usePropertyAccess()
+	 */
+	@Override
+	public boolean usePropertyAccess() {
+
+		if (this.usePropertyAccess == null) {
+			this.usePropertyAccess = super.usePropertyAccess();
+		}
+
+		return this.usePropertyAccess;
+	}
+
+	/* 
+	 * (non-Javadoc)
+	 * @see org.springframework.data.mapping.model.AnnotationBasedPersistentProperty#isTransient()
+	 */
+	@Override
+	public boolean isTransient() {
+
+		if (this.isTransient == null) {
+			this.isTransient = super.isTransient();
+		}
+
+		return this.isTransient;
+	}
 }