From edd9b58d1c4dfbcb367783b46249d1e16d06f5c6 Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Sun, 25 Jan 2015 17:08:07 +0100 Subject: [PATCH] DATAMONGO-712 - Another round of performance improvements. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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. --- .../core/convert/CustomConversions.java | 128 ++++++++++++------ .../core/convert/DBObjectAccessor.java | 25 ++-- .../core/convert/MappingMongoConverter.java | 4 +- .../mongodb/core/convert/QueryMapper.java | 17 ++- .../CachingMongoPersistentProperty.java | 32 ++++- 5 files changed, 147 insertions(+), 59 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/CustomConversions.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/CustomConversions.java index b3b5c2218..3e4c1ad36 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/CustomConversions.java +++ b/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; @@ -69,10 +70,13 @@ public class CustomConversions { private final Set writingPairs; private final Set> customSimpleTypes; private final SimpleTypeHolder simpleTypeHolder; - private final ConcurrentMap customReadTargetTypes; private final List converters; + private final Map customReadTargetTypes; + private final Map customWriteTargetTypes; + private final Map, CacheValue> rawWriteTargetTypes; + /** * Creates an empty {@link CustomConversions} object. */ @@ -92,7 +96,9 @@ public class CustomConversions { this.readingPairs = new LinkedHashSet(); this.writingPairs = new LinkedHashSet(); this.customSimpleTypes = new HashSet>(); - this.customReadTargetTypes = new ConcurrentHashMap(); + this.customReadTargetTypes = new ConcurrentHashMap(); + this.customWriteTargetTypes = new ConcurrentHashMap(); + this.rawWriteTargetTypes = new ConcurrentHashMap, CacheValue>(); List toRegister = new ArrayList(); @@ -235,70 +241,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}. @@ -307,11 +346,15 @@ public class CustomConversions { * @return */ private static Class getCustomTarget(Class sourceType, Class requestedTargetType, - Iterable pairs) { + Collection 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(); @@ -325,32 +368,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 Class getOrCreateAndCache(T key, Map cache, Producer producer) { - Assert.notNull(sourceType); + CacheValue cacheValue = cache.get(key); - if (requestedTargetType == null) { - return null; + if (cacheValue != null) { + return cacheValue.getType(); } - ConvertiblePair lookupKey = new ConvertiblePair(sourceType, requestedTargetType); - CacheValue readTargetTypeValue = customReadTargetTypes.get(lookupKey); + Class type = producer.get(); + cache.put(key, CacheValue.of(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 diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DBObjectAccessor.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DBObjectAccessor.java index dd28c43be..8e16a1554 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DBObjectAccessor.java +++ b/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 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 parts = Arrays.asList(fieldName.split("\\.")).iterator(); - Map source = this.dbObject.toMap(); + Map source = this.dbObject; Object result = null; while (source != null && parts.hasNext()) { @@ -108,14 +117,14 @@ class DBObjectAccessor { } @SuppressWarnings("unchecked") - private Map getAsMap(Object source) { + private Map getAsMap(Object source) { if (source instanceof BasicDBObject) { - return ((DBObject) source).toMap(); + return (BasicDBObject) source; } if (source instanceof Map) { - return (Map) source; + return (Map) source; } return null; diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java index c921597ab..a82334700 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/MappingMongoConverter.java @@ -783,7 +783,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; } @@ -795,7 +795,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App return Enum.valueOf((Class) 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) { diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java index 283e98772..83a7c3de9 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/QueryMapper.java @@ -397,13 +397,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); + } } /** diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/CachingMongoPersistentProperty.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/CachingMongoPersistentProperty.java index c91a5de5e..e436a972c 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/CachingMongoPersistentProperty.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/mapping/CachingMongoPersistentProperty.java @@ -1,5 +1,5 @@ /* - * Copyright 2011-2013 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. @@ -30,6 +30,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}. @@ -84,4 +86,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; + } }