From 799fa6c87e636db9aac1ae1ba2e10eb61226e4dd Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Thu, 9 Aug 2018 11:55:12 +0200 Subject: [PATCH] DATAMONGO-2046 - Performance improvements in mapping and conversion subsystem. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In MappingMongoConverter, we now avoid the creation of a ParameterValueProvider for parameter-less constructors. We also skip property population if entity can be constructed entirely through constructor creation. Replaced the lambda in MappingMongoConverter.readAndPopulateIdentifier(…) with direct call to ….readIdValue(…). Objectpath now uses decomposed ObjectPathItems to avoid array copying and creation. It now stores a reference to its parent and ObjectPathItem fields are now merged into ObjectPath, which reduces the number of created objects during reads. Extended CachingMongoPersistentProperty with DBRef caching. Turned key access in DocumentAccessor into an optimistic lookup. DbRefResolverCallbacks are now created lazily. Related tickets: DATACMNS-1366. Original pull request: #602. --- .../core/convert/DocumentAccessor.java | 23 +++-- .../core/convert/MappingMongoConverter.java | 76 ++++++++++++---- .../data/mongodb/core/convert/ObjectPath.java | 89 ++++++++++--------- .../CachingMongoPersistentProperty.java | 30 ++++++- 4 files changed, 148 insertions(+), 70 deletions(-) diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DocumentAccessor.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DocumentAccessor.java index d3376b377..f8008475e 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DocumentAccessor.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/DocumentAccessor.java @@ -137,7 +137,7 @@ class DocumentAccessor { /** * Returns the raw identifier for the given {@link MongoPersistentEntity} or the value of the default identifier * field. - * + * * @param entity must not be {@literal null}. * @return */ @@ -159,15 +159,20 @@ class DocumentAccessor { String fieldName = property.getFieldName(); + + if (this.document instanceof Document) { + + if (((Document) this.document).containsKey(fieldName)) { + return true; + } + } else if (this.document instanceof DBObject) { + if (((DBObject) this.document).containsField(fieldName)) { + return true; + } + } + if (!fieldName.contains(".")) { - - if (this.document instanceof Document) { - return ((Document) this.document).containsKey(fieldName); - } - - if (this.document instanceof DBObject) { - return ((DBObject) this.document).containsField(fieldName); - } + return false; } String[] parts = fieldName.split("\\."); 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 20a52e522..9b8ce5563 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 @@ -17,7 +17,6 @@ package org.springframework.data.mongodb.core.convert; import java.util.*; import java.util.Map.Entry; -import java.util.function.BiFunction; import org.bson.Document; import org.bson.conversions.Bson; @@ -34,6 +33,7 @@ import org.springframework.data.convert.TypeMapper; import org.springframework.data.mapping.Association; import org.springframework.data.mapping.MappingException; import org.springframework.data.mapping.PersistentPropertyAccessor; +import org.springframework.data.mapping.PreferredConstructor; import org.springframework.data.mapping.PreferredConstructor.Parameter; import org.springframework.data.mapping.context.MappingContext; import org.springframework.data.mapping.model.ConvertingPropertyAccessor; @@ -242,11 +242,12 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App throw new MappingException(String.format(INVALID_TYPE_TO_READ, target, typeToUse.getType())); } - return read((MongoPersistentEntity) mappingContext.getRequiredPersistentEntity(typeToUse), target, path); + return read((MongoPersistentEntity) entity, target, path); } private ParameterValueProvider getParameterProvider(MongoPersistentEntity entity, - Bson source, SpELExpressionEvaluator evaluator, ObjectPath path) { + DocumentAccessor source, SpELExpressionEvaluator evaluator, ObjectPath path) { + AssociationAwareMongoDbPropertyValueProvider provider = new AssociationAwareMongoDbPropertyValueProvider(source, evaluator, path); @@ -260,28 +261,40 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App private S read(final MongoPersistentEntity entity, final Document bson, final ObjectPath path) { SpELExpressionEvaluator evaluator = new DefaultSpELExpressionEvaluator(bson, spELContext); + DocumentAccessor documentAccessor = new DocumentAccessor(bson); + + PreferredConstructor persistenceConstructor = entity.getPersistenceConstructor(); + + ParameterValueProvider provider = persistenceConstructor != null + && persistenceConstructor.hasParameters() ? getParameterProvider(entity, documentAccessor, evaluator, path) + : NoOpParameterValueProvider.INSTANCE; - ParameterValueProvider provider = getParameterProvider(entity, bson, evaluator, path); EntityInstantiator instantiator = instantiators.getInstantiatorFor(entity); S instance = instantiator.createInstance(entity, provider); + if (entity.requiresPropertyPopulation()) { + return populateProperties(entity, documentAccessor, path, evaluator, instance); + } + + return instance; + } + + private S populateProperties(MongoPersistentEntity entity, DocumentAccessor documentAccessor, ObjectPath path, + SpELExpressionEvaluator evaluator, S instance) { + PersistentPropertyAccessor accessor = new ConvertingPropertyAccessor<>(entity.getPropertyAccessor(instance), conversionService); - DocumentAccessor documentAccessor = new DocumentAccessor(bson); - // Make sure id property is set before all other properties Object rawId = readAndPopulateIdentifier(accessor, documentAccessor, entity, - (property, id) -> readIdValue(path, evaluator, property, id)); + path, evaluator); ObjectPath currentPath = path.push(accessor.getBean(), entity, rawId); MongoDbPropertyValueProvider valueProvider = new MongoDbPropertyValueProvider(documentAccessor, evaluator, currentPath); - DbRefResolverCallback callback = new DefaultDbRefResolverCallback(bson, currentPath, evaluator, - MappingMongoConverter.this); - readProperties(entity, accessor, documentAccessor, valueProvider, callback); + readProperties(entity, accessor, documentAccessor, valueProvider, currentPath, evaluator); return accessor.getBean(); } @@ -293,12 +306,12 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App * @param accessor must not be {@literal null}. * @param document must not be {@literal null}. * @param entity must not be {@literal null}. - * @param callback the callback to actually resolve the value for the identifier property, must not be - * {@literal null}. + * @param path + * @param evaluator * @return */ private Object readAndPopulateIdentifier(PersistentPropertyAccessor accessor, DocumentAccessor document, - MongoPersistentEntity entity, BiFunction callback) { + MongoPersistentEntity entity, ObjectPath path, SpELExpressionEvaluator evaluator) { Object rawId = document.getRawId(entity); @@ -312,7 +325,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App return rawId; } - accessor.setProperty(idProperty, callback.apply(idProperty, rawId)); + accessor.setProperty(idProperty, readIdValue(path, evaluator, idProperty, rawId)); return rawId; } @@ -327,11 +340,19 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App } private void readProperties(MongoPersistentEntity entity, PersistentPropertyAccessor accessor, - DocumentAccessor documentAccessor, MongoDbPropertyValueProvider valueProvider, DbRefResolverCallback callback) { + DocumentAccessor documentAccessor, MongoDbPropertyValueProvider valueProvider, ObjectPath currentPath, + SpELExpressionEvaluator evaluator) { + + DbRefResolverCallback callback = null; for (MongoPersistentProperty prop : entity) { if (prop.isAssociation() && !entity.isConstructorArgument(prop)) { + + if (callback == null) { + callback = getDbRefResolverCallback(documentAccessor, currentPath, evaluator); + } + readAssociation(prop.getRequiredAssociation(), accessor, documentAccessor, dbRefProxyHandler, callback); continue; } @@ -347,6 +368,11 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App } if (prop.isAssociation()) { + + if (callback == null) { + callback = getDbRefResolverCallback(documentAccessor, currentPath, evaluator); + } + readAssociation(prop.getRequiredAssociation(), accessor, documentAccessor, dbRefProxyHandler, callback); continue; } @@ -355,6 +381,13 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App } } + private DbRefResolverCallback getDbRefResolverCallback(DocumentAccessor documentAccessor, ObjectPath currentPath, + SpELExpressionEvaluator evaluator) { + + return new DefaultDbRefResolverCallback(documentAccessor.getDocument(), currentPath, evaluator, + MappingMongoConverter.this); + } + private void readAssociation(Association association, PersistentPropertyAccessor accessor, DocumentAccessor documentAccessor, DbRefProxyHandler handler, DbRefResolverCallback callback) { @@ -1381,7 +1414,8 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App * @param evaluator must not be {@literal null}. * @param path must not be {@literal null}. */ - AssociationAwareMongoDbPropertyValueProvider(Bson source, SpELExpressionEvaluator evaluator, ObjectPath path) { + AssociationAwareMongoDbPropertyValueProvider(DocumentAccessor source, SpELExpressionEvaluator evaluator, + ObjectPath path) { super(source, evaluator, path); } @@ -1606,4 +1640,14 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App static class NestedDocument { } + + enum NoOpParameterValueProvider implements ParameterValueProvider { + + INSTANCE; + + @Override + public T getParameterValue(Parameter parameter) { + return null; + } + } } diff --git a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/ObjectPath.java b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/ObjectPath.java index 32604f8b1..ff4f44571 100644 --- a/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/ObjectPath.java +++ b/spring-data-mongodb/src/main/java/org/springframework/data/mongodb/core/convert/ObjectPath.java @@ -15,8 +15,6 @@ */ package org.springframework.data.mongodb.core.convert; -import lombok.Value; - import java.util.ArrayList; import java.util.List; @@ -45,26 +43,33 @@ class ObjectPath { static final ObjectPath ROOT = new ObjectPath(); - private final ObjectPathItem[] items; + private final @Nullable ObjectPath parent; + private final @Nullable Object object; + private final @Nullable Object idValue; + private final String collection; private ObjectPath() { - this.items = new ObjectPathItem[0]; + + this.parent = null; + this.object = null; + this.idValue = null; + this.collection = ""; } /** - * Creates a new {@link ObjectPath} from the given parent {@link ObjectPath} by adding the provided - * {@link ObjectPathItem} to it. + * Creates a new {@link ObjectPath} from the given parent {@link ObjectPath} and adding the provided path values. * * @param parent must not be {@literal null}. - * @param item + * @param collection + * @param idValue + * @param collection */ - private ObjectPath(ObjectPath parent, ObjectPath.ObjectPathItem item) { + private ObjectPath(ObjectPath parent, Object object, @Nullable Object idValue, String collection) { - ObjectPathItem[] items = new ObjectPathItem[parent.items.length + 1]; - System.arraycopy(parent.items, 0, items, 0, parent.items.length); - items[parent.items.length] = item; - - this.items = items; + this.parent = parent; + this.object = object; + this.idValue = idValue; + this.collection = collection; } /** @@ -80,8 +85,7 @@ class ObjectPath { Assert.notNull(object, "Object must not be null!"); Assert.notNull(entity, "MongoPersistentEntity must not be null!"); - ObjectPathItem item = new ObjectPathItem(object, id, entity.getCollection()); - return new ObjectPath(this, item); + return new ObjectPath(this, object, id, entity.getCollection()); } /** @@ -100,15 +104,15 @@ class ObjectPath { Assert.notNull(id, "Id must not be null!"); Assert.hasText(collection, "Collection name must not be null!"); - for (ObjectPathItem item : items) { + for (ObjectPath current = this; current != null; current = current.parent) { - Object object = item.getObject(); + Object object = current.getObject(); - if (object == null || item.getIdValue() == null) { + if (object == null || current.getIdValue() == null) { continue; } - if (collection.equals(item.getCollection()) && id.equals(item.getIdValue())) { + if (collection.equals(current.getCollection()) && id.equals(current.getIdValue())) { return object; } } @@ -133,15 +137,15 @@ class ObjectPath { Assert.hasText(collection, "Collection name must not be null!"); Assert.notNull(type, "Type must not be null!"); - for (ObjectPathItem item : items) { + for (ObjectPath current = this; current != null; current = current.parent) { - Object object = item.getObject(); + Object object = current.getObject(); - if (object == null || item.getIdValue() == null) { + if (object == null || current.getIdValue() == null) { continue; } - if (collection.equals(item.getCollection()) && id.equals(item.getIdValue()) + if (collection.equals(current.getCollection()) && id.equals(current.getIdValue()) && ClassUtils.isAssignable(type, object.getClass())) { return type.cast(object); } @@ -157,7 +161,21 @@ class ObjectPath { */ @Nullable Object getCurrentObject() { - return items.length == 0 ? null : items[items.length - 1].getObject(); + return getObject(); + } + + @Nullable + private Object getObject() { + return object; + } + + @Nullable + private Object getIdValue() { + return idValue; + } + + private String getCollection() { + return collection; } /* @@ -167,31 +185,16 @@ class ObjectPath { @Override public String toString() { - if (items.length == 0) { + if (parent == null) { return "[empty]"; } - List strings = new ArrayList<>(items.length); + List strings = new ArrayList<>(); - for (ObjectPathItem item : items) { - strings.add(ObjectUtils.nullSafeToString(item.object)); + for (ObjectPath current = this; current != null; current = current.parent) { + strings.add(ObjectUtils.nullSafeToString(current.getObject())); } return StringUtils.collectionToDelimitedString(strings, " -> "); } - - /** - * An item in an {@link ObjectPath}. - * - * @author Thomas Darimont - * @author Oliver Gierke - * @author Mark Paluch - */ - @Value - private static class ObjectPathItem { - - Object object; - @Nullable Object idValue; - String collection; - } } 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 4b82a9e81..8a64b59b5 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 @@ -24,11 +24,14 @@ import org.springframework.lang.Nullable; * {@link MongoPersistentProperty} caching access to {@link #isIdProperty()} and {@link #getFieldName()}. * * @author Oliver Gierke + * @author Mark Paluch */ public class CachingMongoPersistentProperty extends BasicMongoPersistentProperty { private @Nullable Boolean isIdProperty; private @Nullable Boolean isAssociation; + private @Nullable boolean dbRefResolved; + private @Nullable DBRef dbref; private @Nullable String fieldName; private @Nullable Boolean usePropertyAccess; private @Nullable Boolean isTransient; @@ -36,8 +39,7 @@ public class CachingMongoPersistentProperty extends BasicMongoPersistentProperty /** * Creates a new {@link CachingMongoPersistentProperty}. * - * @param field - * @param propertyDescriptor + * @param property * @param owner * @param simpleTypeHolder * @param fieldNamingStrategy @@ -114,4 +116,28 @@ public class CachingMongoPersistentProperty extends BasicMongoPersistentProperty return this.isTransient; } + + /* + * (non-Javadoc) + * @see org.springframework.data.mongodb.core.mapping.BasicMongoPersistentProperty#isDbReference() + */ + @Override + public boolean isDbReference() { + return getDBRef() != null; + } + + /* + * (non-Javadoc) + * @see org.springframework.data.mongodb.core.mapping.BasicMongoPersistentProperty#getDBRef() + */ + @Override + public DBRef getDBRef() { + + if (!dbRefResolved) { + this.dbref = super.getDBRef(); + this.dbRefResolved = true; + } + + return this.dbref; + } }