From 34ce87b80c0303828a8475f09cee07035c5eed5c Mon Sep 17 00:00:00 2001 From: Oliver Gierke Date: Wed, 15 Aug 2018 16:06:42 +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 | 26 ++++-- .../core/convert/MappingMongoConverter.java | 62 ++++++++----- .../data/mongodb/core/convert/ObjectPath.java | 91 ++++++++++--------- .../CachingMongoPersistentProperty.java | 34 ++++++- 4 files changed, 136 insertions(+), 77 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 91b45af7e..8f0a646a2 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 @@ -15,6 +15,8 @@ */ package org.springframework.data.mongodb.core.convert; +import lombok.Getter; + import java.util.Arrays; import java.util.Iterator; import java.util.Map; @@ -40,7 +42,7 @@ import com.mongodb.DBObject; */ class DocumentAccessor { - private final Bson document; + private final @Getter Bson document; /** * Creates a new {@link DocumentAccessor} for the given {@link Document}. @@ -137,15 +139,21 @@ 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 130fe6b66..485d9594d 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 @@ -15,17 +15,8 @@ */ 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.LinkedHashMap; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.Map.Entry; -import java.util.Optional; -import java.util.Set; import org.bson.Document; import org.bson.conversions.Bson; @@ -42,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; @@ -250,11 +242,11 @@ 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, DefaultSpELExpressionEvaluator evaluator, ObjectPath path) { + DocumentAccessor source, DefaultSpELExpressionEvaluator evaluator, ObjectPath path) { MongoDbPropertyValueProvider provider = new MongoDbPropertyValueProvider(source, evaluator, path); PersistentEntityParameterValueProvider parameterProvider = new PersistentEntityParameterValueProvider<>( @@ -267,8 +259,14 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App private S read(final MongoPersistentEntity entity, final Document bson, final ObjectPath path) { DefaultSpELExpressionEvaluator evaluator = new DefaultSpELExpressionEvaluator(bson, spELContext); + DocumentAccessor documentAccessor = new DocumentAccessor(bson); + + PreferredConstructor constructor = entity.getPersistenceConstructor(); + + ParameterValueProvider provider = constructor != null && constructor.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); @@ -276,7 +274,6 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App conversionService); MongoPersistentProperty idProperty = entity.getIdProperty(); - DocumentAccessor documentAccessor = new DocumentAccessor(bson); // make sure id property is set before all other properties Object idValue = null; @@ -292,9 +289,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App MongoDbPropertyValueProvider valueProvider = new MongoDbPropertyValueProvider(documentAccessor, evaluator, currentPath); - DbRefResolverCallback callback = new DefaultDbRefResolverCallback(bson, currentPath, evaluator, - MappingMongoConverter.this); - readProperties(entity, accessor, idProperty, documentAccessor, valueProvider, callback); + readProperties(entity, accessor, idProperty, documentAccessor, valueProvider, currentPath, evaluator); return instance; } @@ -310,10 +305,16 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App private void readProperties(MongoPersistentEntity entity, PersistentPropertyAccessor accessor, @Nullable MongoPersistentProperty idProperty, DocumentAccessor documentAccessor, - MongoDbPropertyValueProvider valueProvider, DbRefResolverCallback callback) { + MongoDbPropertyValueProvider valueProvider, ObjectPath currentPath, SpELExpressionEvaluator evaluator) { + + DbRefResolverCallback callback = null; for (MongoPersistentProperty prop : entity) { + if (callback == null) { + callback = getDbRefResolverCallback(documentAccessor, currentPath, evaluator); + } + if (prop.isAssociation() && !entity.isConstructorArgument(prop)) { readAssociation(prop.getRequiredAssociation(), accessor, documentAccessor, dbRefProxyHandler, callback); continue; @@ -336,6 +337,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) { @@ -672,11 +680,13 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App * @param sink the {@link Collection} to write to. * @return */ - private List writeCollectionInternal(Collection source, @Nullable TypeInformation type, Collection sink) { + @SuppressWarnings("unchecked") + private List writeCollectionInternal(Collection source, @Nullable TypeInformation type, + Collection sink) { TypeInformation componentType = null; - List collection = sink instanceof List ? (List) sink : new ArrayList<>(sink); + List collection = sink instanceof List ? (List) sink : new ArrayList<>(sink); if (type != null) { componentType = type.getComponentType(); @@ -881,7 +891,7 @@ public class MappingMongoConverter extends AbstractMongoConverter implements App */ @Nullable @SuppressWarnings({ "rawtypes", "unchecked" }) - private Object getPotentiallyConvertedSimpleRead(@Nullable Object value, @Nullable Class target) { + private Object getPotentiallyConvertedSimpleRead(@Nullable Object value, @Nullable Class target) { if (value == null || target == null || ClassUtils.isAssignableValue(target, value)) { return value; @@ -1541,4 +1551,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 35f64d2ad..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 @@ -1,5 +1,5 @@ /* - * Copyright 2014-2017 the original author or authors. + * Copyright 2014-2018 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. @@ -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 110037158..a5b0ef6a7 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-2017 the original author or authors. + * Copyright 2011-2018 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. @@ -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 @@ -67,9 +69,11 @@ public class CachingMongoPersistentProperty extends BasicMongoPersistentProperty */ @Override public boolean isAssociation() { + if (this.isAssociation == null) { this.isAssociation = super.isAssociation(); } + return this.isAssociation; } @@ -114,4 +118,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; + } }