From d8adbc83a24ad957c015559edf754d3a8c0dd17b Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 6 Aug 2018 09:50:37 +0200 Subject: [PATCH 1/3] DATACMNS-1364 - Prepare issue branch. --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index 4f26a47983..f13f9e523e 100644 --- a/pom.xml +++ b/pom.xml @@ -5,7 +5,7 @@ org.springframework.data spring-data-commons - 2.1.0.BUILD-SNAPSHOT + 2.1.0.DATACMNS-1364-SNAPSHOT Spring Data Core From 40ddf67229f8cac42e9d785844b6e7cd66f3036f Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 6 Aug 2018 09:53:22 +0200 Subject: [PATCH 2/3] DATACMNS-1364 - Store persistent properties in ConcurrentHashMap. We now use ConcurrentHashMap to store persistent properties of a PersistentEntity and to prevent eviction caused by GC activity. Previously, we used ConcurrentReferenceHashMap defaulting to soft references. Soft references can be cleared at the discretion of the GC in response to memory demand. So a default ConcurrentReferenceHashMap is memory-sensitive and acts like a cache with memory-based eviction rules. Persistent properties are not subject to be cached but elements of a PersistentEntity and cannot be recovered once cleared. --- .../data/mapping/model/BasicPersistentEntity.java | 3 ++- .../mapping/model/BasicPersistentEntityUnitTests.java | 11 +++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java b/src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java index 3a8589fccf..d4be4a34ad 100644 --- a/src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java +++ b/src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java @@ -30,6 +30,7 @@ import java.util.Optional; import java.util.Set; import java.util.TreeSet; +import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; import org.springframework.core.annotation.AnnotatedElementUtils; @@ -124,7 +125,7 @@ public BasicPersistentEntity(TypeInformation information, @Nullable Comparato this.constructor = PreferredConstructorDiscoverer.discover(this); this.associations = comparator == null ? new HashSet<>() : new TreeSet<>(new AssociationComparator<>(comparator)); - this.propertyCache = new ConcurrentReferenceHashMap<>(); + this.propertyCache = new ConcurrentHashMap<>(); this.annotationCache = new ConcurrentReferenceHashMap<>(); this.propertyAnnotationCache = CollectionUtils.toMultiValueMap(new ConcurrentReferenceHashMap<>()); this.propertyAccessorFactory = BeanWrapperPropertyAccessorFactory.INSTANCE; diff --git a/src/test/java/org/springframework/data/mapping/model/BasicPersistentEntityUnitTests.java b/src/test/java/org/springframework/data/mapping/model/BasicPersistentEntityUnitTests.java index 632b765c1e..b04434d8f4 100755 --- a/src/test/java/org/springframework/data/mapping/model/BasicPersistentEntityUnitTests.java +++ b/src/test/java/org/springframework/data/mapping/model/BasicPersistentEntityUnitTests.java @@ -134,24 +134,27 @@ public void considersComparatorForPropertyOrder() { assertThat(entity.getPersistentProperty("ssn")).isEqualTo(iterator.next()); } - @Test // DATACMNS-186 + @Test // DATACMNS-18, DATACMNS-1364 public void addingAndIdPropertySetsIdPropertyInternally() { MutablePersistentEntity entity = createEntity(Person.class); assertThat(entity.getIdProperty()).isNull(); + when(property.getName()).thenReturn("id"); when(property.isIdProperty()).thenReturn(true); entity.addPersistentProperty(property); assertThat(entity.getIdProperty()).isEqualTo(property); } - @Test // DATACMNS-186 + @Test // DATACMNS-186, DATACMNS-1364 public void rejectsIdPropertyIfAlreadySet() { MutablePersistentEntity entity = createEntity(Person.class); + when(property.getName()).thenReturn("id"); when(property.isIdProperty()).thenReturn(true); when(anotherProperty.isIdProperty()).thenReturn(true); + when(anotherProperty.getName()).thenReturn("another"); entity.addPersistentProperty(property); exception.expect(MappingException.class); @@ -406,7 +409,7 @@ static class PersistableEntity implements Persistable { private final Long id = 42L; - /* + /* * (non-Javadoc) * @see org.springframework.data.domain.Persistable#getId() */ @@ -415,7 +418,7 @@ public Long getId() { return 4711L; } - /* + /* * (non-Javadoc) * @see org.springframework.data.domain.Persistable#isNew() */ From 53fbf674121803ca1c0375c699728336b9f95729 Mon Sep 17 00:00:00 2001 From: Mark Paluch Date: Mon, 6 Aug 2018 09:56:05 +0200 Subject: [PATCH 3/3] DATACMNS-1364 - Polishing. Use weak references in annotation and property annotation cache to retain references until the last GC root is cleared. Remove trailing whitespaces. Reformat. --- .../mapping/model/BasicPersistentEntity.java | 41 +++++-------------- 1 file changed, 11 insertions(+), 30 deletions(-) diff --git a/src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java b/src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java index d4be4a34ad..81364ecbb2 100644 --- a/src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java +++ b/src/main/java/org/springframework/data/mapping/model/BasicPersistentEntity.java @@ -20,16 +20,7 @@ import java.io.Serializable; import java.lang.annotation.Annotation; -import java.util.ArrayList; -import java.util.Collections; -import java.util.Comparator; -import java.util.HashSet; -import java.util.Iterator; -import java.util.List; -import java.util.Map; -import java.util.Optional; -import java.util.Set; -import java.util.TreeSet; +import java.util.*; import java.util.concurrent.ConcurrentHashMap; import java.util.stream.Collectors; @@ -37,19 +28,7 @@ import org.springframework.data.annotation.Immutable; import org.springframework.data.annotation.TypeAlias; import org.springframework.data.domain.Persistable; -import org.springframework.data.mapping.Alias; -import org.springframework.data.mapping.Association; -import org.springframework.data.mapping.AssociationHandler; -import org.springframework.data.mapping.IdentifierAccessor; -import org.springframework.data.mapping.MappingException; -import org.springframework.data.mapping.PersistentEntity; -import org.springframework.data.mapping.PersistentProperty; -import org.springframework.data.mapping.PersistentPropertyAccessor; -import org.springframework.data.mapping.PreferredConstructor; -import org.springframework.data.mapping.PropertyHandler; -import org.springframework.data.mapping.SimpleAssociationHandler; -import org.springframework.data.mapping.SimplePropertyHandler; -import org.springframework.data.mapping.TargetAwareIdentifierAccessor; +import org.springframework.data.mapping.*; import org.springframework.data.spel.EvaluationContextProvider; import org.springframework.data.support.IsNewStrategy; import org.springframework.data.support.PersistableIsNewStrategy; @@ -60,6 +39,7 @@ import org.springframework.util.Assert; import org.springframework.util.CollectionUtils; import org.springframework.util.ConcurrentReferenceHashMap; +import org.springframework.util.ConcurrentReferenceHashMap.ReferenceType; import org.springframework.util.MultiValueMap; import org.springframework.util.StringUtils; @@ -126,8 +106,9 @@ public BasicPersistentEntity(TypeInformation information, @Nullable Comparato this.associations = comparator == null ? new HashSet<>() : new TreeSet<>(new AssociationComparator<>(comparator)); this.propertyCache = new ConcurrentHashMap<>(); - this.annotationCache = new ConcurrentReferenceHashMap<>(); - this.propertyAnnotationCache = CollectionUtils.toMultiValueMap(new ConcurrentReferenceHashMap<>()); + this.annotationCache = new ConcurrentReferenceHashMap<>(16, ReferenceType.WEAK); + this.propertyAnnotationCache = CollectionUtils + .toMultiValueMap(new ConcurrentReferenceHashMap<>(16, ReferenceType.WEAK)); this.propertyAccessorFactory = BeanWrapperPropertyAccessorFactory.INSTANCE; this.typeAlias = Lazy.of(() -> getAliasFromAnnotation(getType())); this.isNewStrategy = Lazy.of(() -> Persistable.class.isAssignableFrom(information.getType()) // @@ -255,7 +236,7 @@ public void addPersistentProperty(P property) { } } - /* + /* * (non-Javadoc) * @see org.springframework.data.mapping.model.MutablePersistentEntity#setEvaluationContextProvider(org.springframework.data.spel.EvaluationContextProvider) */ @@ -488,7 +469,7 @@ public IdentifierAccessor getIdentifierAccessor(Object bean) { return hasIdProperty() ? new IdPropertyIdentifierAccessor(this, bean) : new AbsentIdentifierAccessor(bean); } - /* + /* * (non-Javadoc) * @see org.springframework.data.mapping.PersistentEntity#isNew(java.lang.Object) */ @@ -500,7 +481,7 @@ public boolean isNew(Object bean) { return isNewStrategy.get().isNew(bean); } - /* + /* * (non-Javadoc) * @see org.springframework.data.mapping.PersistentEntity#isImmutable() */ @@ -526,7 +507,7 @@ protected EvaluationContext getEvaluationContext(Object rootObject) { * Returns the default {@link IsNewStrategy} to be used. Will be a {@link PersistentEntityIsNewStrategy} by default. * Note, that this strategy only gets used if the entity doesn't implement {@link Persistable} as this indicates the * user wants to be in control over whether an entity is new or not. - * + * * @return * @since 2.1 */ @@ -536,7 +517,7 @@ protected IsNewStrategy getFallbackIsNewStrategy() { /** * Verifies the given bean type to no be {@literal null} and of the type of the current {@link PersistentEntity}. - * + * * @param bean must not be {@literal null}. */ private final void verifyBeanType(Object bean) {