Skip to content

Commit

Permalink
Revisit internal caching arrangements.
Browse files Browse the repository at this point in the history
Closes: #3185
Original Pull Request: #3186
  • Loading branch information
mp911de authored and christophstrobl committed Oct 31, 2024
1 parent e0e881f commit adc8e3f
Show file tree
Hide file tree
Showing 5 changed files with 97 additions and 33 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,23 +16,14 @@
package org.springframework.data.convert;

import java.lang.annotation.Annotation;
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.Map;
import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import java.util.*;
import java.util.function.Function;
import java.util.function.Predicate;
import java.util.stream.Collectors;

import org.apache.commons.logging.Log;
import org.apache.commons.logging.LogFactory;

import org.springframework.core.GenericTypeResolver;
import org.springframework.core.annotation.AnnotationUtils;
import org.springframework.core.convert.converter.Converter;
Expand Down Expand Up @@ -119,7 +110,7 @@ public class CustomConversions {
private final Function<ConvertiblePair, Class<?>> getRawWriteTarget = convertiblePair -> getCustomTarget(
convertiblePair.getSourceType(), null, writingPairs);

@Nullable private final PropertyValueConversions propertyValueConversions;
private final @Nullable PropertyValueConversions propertyValueConversions;

/**
* @param converterConfiguration the {@link ConverterConfiguration} to apply.
Expand Down Expand Up @@ -506,7 +497,7 @@ private Class<?> getCustomTarget(Class<?> sourceType, @Nullable Class<?> targetT
*/
static class ConversionTargetsCache {

private final Map<Class<?>, TargetTypes> customReadTargetTypes = new ConcurrentHashMap<>();
private Map<Class<?>, TargetTypes> customReadTargetTypes = new HashMap<>();

/**
* Get or compute a target type given its {@code sourceType}. Returns a cached {@link Optional} if the value
Expand Down Expand Up @@ -536,7 +527,15 @@ public Class<?> computeIfAbsent(Class<?> sourceType, Function<ConvertiblePair, C
public Class<?> computeIfAbsent(Class<?> sourceType, Class<?> targetType,
Function<ConvertiblePair, Class<?>> mappingFunction) {

TargetTypes targetTypes = customReadTargetTypes.computeIfAbsent(sourceType, TargetTypes::new);
TargetTypes targetTypes = customReadTargetTypes.get(sourceType);

if (targetTypes == null) {

Map<Class<?>, TargetTypes> customReadTargetTypes = new HashMap<>(this.customReadTargetTypes);
targetTypes = new TargetTypes(sourceType);
customReadTargetTypes.put(sourceType, targetTypes);
this.customReadTargetTypes = customReadTargetTypes;
}

return targetTypes.computeIfAbsent(targetType, mappingFunction);
}
Expand All @@ -555,7 +554,7 @@ interface AbsentTargetTypeMarker {}
static class TargetTypes {

private final Class<?> sourceType;
private final Map<Class<?>, Class<?>> conversionTargets = new ConcurrentHashMap<>();
private Map<Class<?>, Class<?>> conversionTargets = new HashMap<>();

TargetTypes(Class<?> sourceType) {
this.sourceType = sourceType;
Expand All @@ -576,17 +575,21 @@ public Class<?> computeIfAbsent(Class<?> targetType, Function<ConvertiblePair, C
Class<?> optionalTarget = conversionTargets.get(targetType);

if (optionalTarget == null) {

optionalTarget = mappingFunction.apply(new ConvertiblePair(sourceType, targetType));

Map<Class<?>, Class<?>> conversionTargets = new HashMap<>(this.conversionTargets);
conversionTargets.put(targetType, optionalTarget == null ? Void.class : optionalTarget);
this.conversionTargets = conversionTargets;
}

return Void.class.equals(optionalTarget) ? null : optionalTarget;
}
}

/**
* Value class tying together a {@link ConverterRegistration} and its {@link ConverterOrigin origin} to allow fine
* grained registration based on store supported types.
* Value class tying together a {@link ConverterRegistration} and its {@link ConverterOrigin origin} to allow
* fine-grained registration based on store supported types.
*
* @since 2.3
* @author Christoph Strobl
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@
package org.springframework.data.convert;

import java.util.ArrayList;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.Optional;

import org.springframework.beans.factory.InitializingBean;
Expand Down Expand Up @@ -51,6 +53,23 @@ public class SimplePropertyValueConversions implements PropertyValueConversions,

private @Nullable ValueConverterRegistry<?> valueConverterRegistry;

private Map<PersistentProperty<?>, PropertyValueConverter<?, ?, ?>> converterCache = new HashMap<>();

@SuppressWarnings("rawtypes")
enum NoOpConverter implements PropertyValueConverter {
INSTANCE;

@Override
public Object read(Object value, ValueConversionContext context) {
return null;
}

@Override
public Object write(Object value, ValueConversionContext context) {
return null;
}
}

/**
* Set the {@link PropertyValueConverterFactory} responsible for creating the actual {@link PropertyValueConverter}.
*
Expand Down Expand Up @@ -129,20 +148,49 @@ public void setConverterCacheEnabled(boolean converterCacheEnabled) {
*/
@Override
public boolean hasValueConverter(PersistentProperty<?> property) {
return requireConverterFactory().getConverter(property) != null;
return doGetConverter(property) != null;
}

@Override
public <DV, SV, P extends PersistentProperty<P>, D extends ValueConversionContext<P>> PropertyValueConverter<DV, SV, D> getValueConverter(
P property) {

PropertyValueConverter<DV, SV, D> converter = requireConverterFactory().getConverter(property);
PropertyValueConverter<DV, SV, D> converter = doGetConverter(property);

Assert.notNull(converter, String.format("No PropertyValueConverter registered for %s", property));

return converter;
}

@SuppressWarnings("unchecked")
@Nullable
private <DV, SV, P extends PersistentProperty<P>, D extends ValueConversionContext<P>> PropertyValueConverter<DV, SV, D> doGetConverter(
PersistentProperty<?> property) {

PropertyValueConverter<?, ?, ?> converter = converterCache.get(property);

if (converter == null) {

converter = requireConverterFactory().getConverter(property);

Map<PersistentProperty<?>, PropertyValueConverter<?, ?, ?>> converterCache = new HashMap<>(this.converterCache);

if (converter == null) {
converterCache.put(property, NoOpConverter.INSTANCE);
} else {
converterCache.put(property, converter);
}

this.converterCache = converterCache;
}

if (converter == NoOpConverter.INSTANCE) {
return null;
}

return (PropertyValueConverter<DV, SV, D>) converter;
}

/**
* May be called just once to initialize the underlying factory with its values.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@

import java.lang.reflect.Executable;
import java.util.Arrays;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
import java.util.concurrent.ConcurrentHashMap;

import org.springframework.util.Assert;

Expand All @@ -35,7 +35,7 @@ class InstanceCreatorMetadataSupport<T, P extends PersistentProperty<P>> impleme

private final Executable executable;
private final List<Parameter<Object, P>> parameters;
private final Map<PersistentProperty<?>, Boolean> isPropertyParameterCache = new ConcurrentHashMap<>();
private Map<PersistentProperty<?>, Boolean> isPropertyParameterCache = new HashMap<>();

/**
* Creates a new {@link InstanceCreatorMetadataSupport} from the given {@link Executable} and {@link Parameter}s.
Expand Down Expand Up @@ -96,7 +96,9 @@ public boolean isCreatorParameter(PersistentProperty<?> property) {

boolean result = doGetIsCreatorParameter(property);

Map<PersistentProperty<?>, Boolean> isPropertyParameterCache = new HashMap<>(this.isPropertyParameterCache);
isPropertyParameterCache.put(property, result);
this.isPropertyParameterCache = isPropertyParameterCache;

return result;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,19 @@
*
* @author Oliver Gierke
* @author Christoph Strobl
* @author Mark Paluch
* @deprecated since 3.0 to go package protected at some point. Refer to {@link TypeInformation} only.
*/
@Deprecated(since = "3.0", forRemoval = true)
@SuppressWarnings({ "rawtypes", "unchecked" })
public class ClassTypeInformation<S> extends TypeDiscoverer<S> {

private static final ConcurrentLruCache<ResolvableType, ClassTypeInformation<?>> cache = new ConcurrentLruCache<>(64,
private static final ConcurrentLruCache<ResolvableType, ClassTypeInformation<?>> cache = new ConcurrentLruCache<>(128,
ClassTypeInformation::new);

private static final ConcurrentLruCache<Class<?>, ResolvableType> resolvableTypeCache = new ConcurrentLruCache<>(128,
ResolvableType::forClass);

public static final ClassTypeInformation<Collection> COLLECTION;
public static final ClassTypeInformation<List> LIST;
public static final ClassTypeInformation<Set> SET;
Expand All @@ -48,11 +52,11 @@ public class ClassTypeInformation<S> extends TypeDiscoverer<S> {

static {

OBJECT = (ClassTypeInformation<Object>) cache.get(ResolvableType.forClass(Object.class));
COLLECTION = (ClassTypeInformation<Collection>) cache.get(ResolvableType.forClass(Collection.class));
LIST = (ClassTypeInformation<List>) cache.get(ResolvableType.forClass(List.class));
SET = (ClassTypeInformation<Set>) cache.get(ResolvableType.forClass(Set.class));
MAP = (ClassTypeInformation<Map>) cache.get(ResolvableType.forClass(Map.class));
OBJECT = from(Object.class);
COLLECTION = from(Collection.class);
LIST = from(List.class);
SET = from(Set.class);
MAP = from(Map.class);
}

private final Class<S> type;
Expand All @@ -70,7 +74,7 @@ public class ClassTypeInformation<S> extends TypeDiscoverer<S> {
*/
@Deprecated
public static <S> ClassTypeInformation<S> from(Class<S> type) {
return from(ResolvableType.forClass(type));
return from(resolvableTypeCache.get(type));
}

static <S> ClassTypeInformation<S> from(ResolvableType type) {
Expand Down
17 changes: 12 additions & 5 deletions src/main/java/org/springframework/data/util/TypeDiscoverer.java
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,16 @@ class TypeDiscoverer<S> implements TypeInformation<S> {

private final ResolvableType resolvableType;
private final Map<String, Optional<TypeInformation<?>>> fields = new ConcurrentHashMap<>();
private final Lazy<Boolean> isMap = Lazy.of(() -> CustomCollections.isMap(getType()));
private final Lazy<Boolean> isCollectionLike = Lazy.of(() -> {

Class<S> type = getType();

return type.isArray() //
|| Iterable.class.equals(type) //
|| Collection.class.isAssignableFrom(type) //
|| Streamable.class.isAssignableFrom(type) || CustomCollections.isCollection(type);
});
private final Lazy<TypeInformation<?>> componentType;
private final Lazy<TypeInformation<?>> valueType;
private final Map<Constructor<?>, List<TypeInformation<?>>> constructorParameters = new ConcurrentHashMap<>();
Expand Down Expand Up @@ -125,10 +135,7 @@ public boolean isCollectionLike() {

Class<S> type = getType();

return type.isArray() //
|| Iterable.class.equals(type) //
|| Collection.class.isAssignableFrom(type) //
|| Streamable.class.isAssignableFrom(type) || CustomCollections.isCollection(type);
return isCollectionLike.get();
}

@Nullable
Expand Down Expand Up @@ -169,7 +176,7 @@ protected TypeInformation<?> doGetComponentType() {

@Override
public boolean isMap() {
return CustomCollections.isMap(getType());
return isMap.get();
}

@Nullable
Expand Down

0 comments on commit adc8e3f

Please sign in to comment.