Polishing.

Use double checked locking to be as close to the previous implementation but still benefit from the fast read operation. Original Pull Request: #3186
spring-projects · Oct 31, 2024 · 1b79eeb · 1b79eeb
1 parent adc8e3f
commit 1b79eeb
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 28 deletions.
diff --git a/src/main/java/org/springframework/data/convert/CustomConversions.java b/src/main/java/org/springframework/data/convert/CustomConversions.java
@@ -497,7 +497,7 @@ private Class<?> getCustomTarget(Class<?> sourceType, @Nullable Class<?> targetT
 	 */
 	static class ConversionTargetsCache {
 
-		private Map<Class<?>, TargetTypes> customReadTargetTypes = new HashMap<>();
+		private volatile Map<Class<?>, TargetTypes> customReadTargetTypes = new HashMap<>();
 
 		/**
 		 * Get or compute a target type given its {@code sourceType}. Returns a cached {@link Optional} if the value
@@ -531,10 +531,19 @@ public Class<?> computeIfAbsent(Class<?> sourceType, Class<?> targetType,
 
 			if (targetTypes == null) {
 
-				Map<Class<?>, TargetTypes> customReadTargetTypes = new HashMap<>(this.customReadTargetTypes);
-				targetTypes = new TargetTypes(sourceType);
-				customReadTargetTypes.put(sourceType, targetTypes);
-				this.customReadTargetTypes = customReadTargetTypes;
+				synchronized (this) {
+
+					TargetTypes customReadTarget = customReadTargetTypes.get(sourceType);
+					if (customReadTarget != null) {
+						targetTypes = customReadTarget;
+					} else {
+
+						Map<Class<?>, TargetTypes> customReadTargetTypes = new HashMap<>(this.customReadTargetTypes);
+						targetTypes = new TargetTypes(sourceType);
+						customReadTargetTypes.put(sourceType, targetTypes);
+						this.customReadTargetTypes = customReadTargetTypes;
+					}
+				}
 			}
 
 			return targetTypes.computeIfAbsent(targetType, mappingFunction);
@@ -554,7 +563,7 @@ interface AbsentTargetTypeMarker {}
 	static class TargetTypes {
 
 		private final Class<?> sourceType;
-		private Map<Class<?>, Class<?>> conversionTargets = new HashMap<>();
+		private volatile Map<Class<?>, Class<?>> conversionTargets = new HashMap<>();
 
 		TargetTypes(Class<?> sourceType) {
 			this.sourceType = sourceType;
@@ -576,11 +585,19 @@ public Class<?> computeIfAbsent(Class<?> targetType, Function<ConvertiblePair, C
 
 			if (optionalTarget == null) {
 
-				optionalTarget = mappingFunction.apply(new ConvertiblePair(sourceType, targetType));
+				synchronized (this) {
+
+					Class<?> conversionTarget = conversionTargets.get(targetType);
+					if (conversionTarget != null) {
+						optionalTarget = conversionTarget;
+					} else {
 
-				Map<Class<?>, Class<?>> conversionTargets = new HashMap<>(this.conversionTargets);
-				conversionTargets.put(targetType, optionalTarget == null ? Void.class : optionalTarget);
-				this.conversionTargets = conversionTargets;
+						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;

diff --git a/src/main/java/org/springframework/data/convert/SimplePropertyValueConversions.java b/src/main/java/org/springframework/data/convert/SimplePropertyValueConversions.java
@@ -53,7 +53,7 @@ public class SimplePropertyValueConversions implements PropertyValueConversions,
 
 	private @Nullable ValueConverterRegistry<?> valueConverterRegistry;
 
-	private Map<PersistentProperty<?>, PropertyValueConverter<?, ?, ?>> converterCache = new HashMap<>();
+	private volatile Map<PersistentProperty<?>, PropertyValueConverter<?, ?, ?>> converterCache = new HashMap<>();
 
 	@SuppressWarnings("rawtypes")
 	enum NoOpConverter implements PropertyValueConverter {
@@ -171,17 +171,21 @@ private <DV, SV, P extends PersistentProperty<P>, D extends ValueConversionConte
 
 		if (converter == null) {
 
-			converter = requireConverterFactory().getConverter(property);
+			synchronized (this) {
 
-			Map<PersistentProperty<?>, PropertyValueConverter<?, ?, ?>> converterCache = new HashMap<>(this.converterCache);
+				PropertyValueConverter<?, ?, ?> fromCache = converterCache.get(property);
+				if (fromCache != null) {
+					converter = fromCache;
+				} else {
 
-			if (converter == null) {
-				converterCache.put(property, NoOpConverter.INSTANCE);
-			} else {
-				converterCache.put(property, converter);
-			}
+					converter = requireConverterFactory().getConverter(property);
 
-			this.converterCache = converterCache;
+					Map<PersistentProperty<?>, PropertyValueConverter<?, ?, ?>> converterCache = new HashMap<>(
+							this.converterCache);
+					converterCache.put(property, converter != null ? converter : NoOpConverter.INSTANCE);
+					this.converterCache = converterCache;
+				}
+			}
 		}
 
 		if (converter == NoOpConverter.INSTANCE) {

diff --git a/src/main/java/org/springframework/data/mapping/InstanceCreatorMetadataSupport.java b/src/main/java/org/springframework/data/mapping/InstanceCreatorMetadataSupport.java
@@ -35,7 +35,7 @@ class InstanceCreatorMetadataSupport<T, P extends PersistentProperty<P>> impleme
 
 	private final Executable executable;
 	private final List<Parameter<Object, P>> parameters;
-	private Map<PersistentProperty<?>, Boolean> isPropertyParameterCache = new HashMap<>();
+	private volatile Map<PersistentProperty<?>, Boolean> isPropertyParameterCache = new HashMap<>();
 
 	/**
 	 * Creates a new {@link InstanceCreatorMetadataSupport} from the given {@link Executable} and {@link Parameter}s.
@@ -90,17 +90,25 @@ public boolean isCreatorParameter(PersistentProperty<?> property) {
 
 		Boolean cached = isPropertyParameterCache.get(property);
 
-		if (cached != null) {
-			return cached;
-		}
+		if (cached == null) {
+
+			synchronized (this) {
+
+				Boolean fromCache = isPropertyParameterCache.get(property);
+				if (fromCache != null) {
+					cached = fromCache;
+				} else {
 
-		boolean result = doGetIsCreatorParameter(property);
+					cached = doGetIsCreatorParameter(property);
 
-		Map<PersistentProperty<?>, Boolean> isPropertyParameterCache = new HashMap<>(this.isPropertyParameterCache);
-		isPropertyParameterCache.put(property, result);
-		this.isPropertyParameterCache = isPropertyParameterCache;
+					Map<PersistentProperty<?>, Boolean> isPropertyParameterCache = new HashMap<>(this.isPropertyParameterCache);
+					isPropertyParameterCache.put(property, cached);
+					this.isPropertyParameterCache = isPropertyParameterCache;
+				}
+			}
+		}
 
-		return result;
+		return cached;
 	}
 
 	@Override