From 54afa69ba5f082ebe6df807b156cf1684b9ddbe4 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Sat, 22 Mar 2025 20:51:03 -0700 Subject: [PATCH] Add `Binder` cache and use in `JavaBeanBinder` and `ValueObjectBinder` Introduce a general purpose cache in the `Binder` and make use of it in `JavaBeanBinder` and `ValueObjectBinder` to reuse potentially expensive operations. Closes gh-44861 --- .../boot/context/properties/bind/Binder.java | 7 +++ .../properties/bind/JavaBeanBinder.java | 57 ++++++++++++++----- .../properties/bind/ValueObjectBinder.java | 50 +++++++++++++--- 3 files changed, 92 insertions(+), 22 deletions(-) diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Binder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Binder.java index 995c8b17d2..3844dae084 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Binder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/Binder.java @@ -46,6 +46,7 @@ import org.springframework.core.convert.ConverterNotFoundException; import org.springframework.core.env.Environment; import org.springframework.format.support.DefaultFormattingConversionService; import org.springframework.util.Assert; +import org.springframework.util.ConcurrentReferenceHashMap; /** * A container object which Binds objects from one or more @@ -70,6 +71,8 @@ public class Binder { private final Map> dataObjectBinders; + private final Map cache = new ConcurrentReferenceHashMap<>(); + private ConfigurationPropertyCaching configurationPropertyCaching; /** @@ -635,6 +638,10 @@ public class Binder { return Binder.this.bindConverter; } + Map getCache() { + return Binder.this.cache; + } + @Override public Binder getBinder() { return Binder.this; diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/JavaBeanBinder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/JavaBeanBinder.java index 985a5fa995..af445bc2d9 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/JavaBeanBinder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/JavaBeanBinder.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2024 the original author or authors. + * Copyright 2012-2025 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. @@ -28,6 +28,7 @@ import java.util.LinkedHashMap; import java.util.LinkedHashSet; import java.util.Map; import java.util.Set; +import java.util.concurrent.ConcurrentHashMap; import java.util.function.BiConsumer; import java.util.function.Function; import java.util.function.Supplier; @@ -50,13 +51,16 @@ import org.springframework.core.ResolvableType; */ class JavaBeanBinder implements DataObjectBinder { + private static final String HAS_KNOWN_BINDABLE_PROPERTIES_CACHE = JavaBeanBinder.class.getName() + + ".HAS_KNOWN_BINDABLE_PROPERTIES_CACHE"; + static final JavaBeanBinder INSTANCE = new JavaBeanBinder(); @Override public T bind(ConfigurationPropertyName name, Bindable target, Context context, DataObjectPropertyBinder propertyBinder) { boolean hasKnownBindableProperties = target.getValue() != null && hasKnownBindableProperties(name, context); - Bean bean = Bean.get(target, hasKnownBindableProperties); + Bean bean = Bean.get(target, context, hasKnownBindableProperties); if (bean == null) { return null; } @@ -73,6 +77,16 @@ class JavaBeanBinder implements DataObjectBinder { } private boolean hasKnownBindableProperties(ConfigurationPropertyName name, Context context) { + Map cache = getHasKnownBindablePropertiesCache(context); + Boolean hasKnownBindableProperties = cache.get(name); + if (hasKnownBindableProperties == null) { + hasKnownBindableProperties = computeHasKnownBindableProperties(name, context); + cache.put(name, hasKnownBindableProperties); + } + return hasKnownBindableProperties; + } + + private boolean computeHasKnownBindableProperties(ConfigurationPropertyName name, Context context) { for (ConfigurationPropertySource source : context.getSources()) { if (source.containsDescendantOf(name) == ConfigurationPropertyState.PRESENT) { return true; @@ -81,6 +95,16 @@ class JavaBeanBinder implements DataObjectBinder { return false; } + @SuppressWarnings("unchecked") + private Map getHasKnownBindablePropertiesCache(Context context) { + Object cache = context.getCache().get(HAS_KNOWN_BINDABLE_PROPERTIES_CACHE); + if (cache == null) { + cache = new ConcurrentHashMap(); + context.getCache().put(HAS_KNOWN_BINDABLE_PROPERTIES_CACHE, cache); + } + return (Map) cache; + } + private boolean bind(DataObjectPropertyBinder propertyBinder, Bean bean, BeanSupplier beanSupplier, Context context) { boolean bound = false; @@ -236,8 +260,6 @@ class JavaBeanBinder implements DataObjectBinder { */ static class Bean extends BeanProperties { - private static Bean cached; - Bean(ResolvableType type, Class resolvedType) { super(type, resolvedType); } @@ -257,7 +279,7 @@ class JavaBeanBinder implements DataObjectBinder { } @SuppressWarnings("unchecked") - static Bean get(Bindable bindable, boolean canCallGetValue) { + static Bean get(Bindable bindable, Context context, boolean canCallGetValue) { ResolvableType type = bindable.getType(); Class resolvedType = type.resolve(Object.class); Supplier value = bindable.getValue(); @@ -269,14 +291,26 @@ class JavaBeanBinder implements DataObjectBinder { if (instance == null && !isInstantiable(resolvedType)) { return null; } - Bean bean = Bean.cached; - if (bean == null || !bean.isOfType(type, resolvedType)) { + Map> cache = getCache(context); + CacheKey cacheKey = new CacheKey(type, resolvedType); + Bean bean = cache.get(cacheKey); + if (bean == null) { bean = new Bean<>(type, resolvedType); - cached = bean; + cache.put(cacheKey, bean); } return (Bean) bean; } + @SuppressWarnings("unchecked") + private static Map> getCache(Context context) { + Map> cache = (Map>) context.getCache().get(Bean.class); + if (cache == null) { + cache = new ConcurrentHashMap<>(); + context.getCache().put(Bean.class, cache); + } + return cache; + } + private static boolean isInstantiable(Class type) { if (type.isInterface()) { return false; @@ -290,11 +324,8 @@ class JavaBeanBinder implements DataObjectBinder { } } - private boolean isOfType(ResolvableType type, Class resolvedType) { - if (getType().hasGenerics() || type.hasGenerics()) { - return getType().equals(type); - } - return getResolvedType() != null && getResolvedType().equals(resolvedType); + private record CacheKey(ResolvableType type, Class resolvedType) { + } } diff --git a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/ValueObjectBinder.java b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/ValueObjectBinder.java index d304bff3ba..9ae5dfece5 100644 --- a/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/ValueObjectBinder.java +++ b/spring-boot-project/spring-boot/src/main/java/org/springframework/boot/context/properties/bind/ValueObjectBinder.java @@ -1,5 +1,5 @@ /* - * Copyright 2012-2024 the original author or authors. + * Copyright 2012-2025 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. @@ -29,6 +29,7 @@ import java.util.EnumMap; import java.util.List; import java.util.Map; import java.util.Optional; +import java.util.concurrent.ConcurrentHashMap; import java.util.function.Consumer; import kotlin.reflect.KFunction; @@ -73,7 +74,7 @@ class ValueObjectBinder implements DataObjectBinder { @Override public T bind(ConfigurationPropertyName name, Bindable target, Binder.Context context, DataObjectPropertyBinder propertyBinder) { - ValueObject valueObject = ValueObject.get(target, this.constructorProvider, context, Discoverer.LENIENT); + ValueObject valueObject = ValueObject.get(target, context, this.constructorProvider, Discoverer.LENIENT); if (valueObject == null) { return null; } @@ -94,7 +95,7 @@ class ValueObjectBinder implements DataObjectBinder { @Override public T create(Bindable target, Binder.Context context) { - ValueObject valueObject = ValueObject.get(target, this.constructorProvider, context, Discoverer.LENIENT); + ValueObject valueObject = ValueObject.get(target, context, this.constructorProvider, Discoverer.LENIENT); if (valueObject == null) { return null; } @@ -109,7 +110,7 @@ class ValueObjectBinder implements DataObjectBinder { @Override public void onUnableToCreateInstance(Bindable target, Context context, RuntimeException exception) { try { - ValueObject.get(target, this.constructorProvider, context, Discoverer.STRICT); + ValueObject.get(target, context, this.constructorProvider, Discoverer.STRICT); } catch (Exception ex) { exception.addSuppressed(ex); @@ -191,6 +192,8 @@ class ValueObjectBinder implements DataObjectBinder { */ private abstract static class ValueObject { + private static final Object NONE = new Object(); + private final Constructor constructor; protected ValueObject(Constructor constructor) { @@ -204,24 +207,53 @@ class ValueObjectBinder implements DataObjectBinder { abstract List getConstructorParameters(); @SuppressWarnings("unchecked") - static ValueObject get(Bindable bindable, BindConstructorProvider constructorProvider, - Binder.Context context, ParameterNameDiscoverer parameterNameDiscoverer) { - Class type = (Class) bindable.getType().resolve(); - if (type == null || type.isEnum() || Modifier.isAbstract(type.getModifiers())) { + static ValueObject get(Bindable bindable, Binder.Context context, + BindConstructorProvider constructorProvider, ParameterNameDiscoverer parameterNameDiscoverer) { + Class resolvedType = (Class) bindable.getType().resolve(); + if (resolvedType == null || resolvedType.isEnum() || Modifier.isAbstract(resolvedType.getModifiers())) { return null; } + Map cache = getCache(context); + CacheKey cacheKey = new CacheKey(bindable, constructorProvider, parameterNameDiscoverer); + Object valueObject = cache.get(cacheKey); + if (valueObject == null) { + valueObject = get(bindable, context, constructorProvider, parameterNameDiscoverer, resolvedType); + cache.put(cacheKey, (valueObject != null) ? valueObject : NONE); + } + return (valueObject != NONE) ? (ValueObject) valueObject : null; + } + + @SuppressWarnings("unchecked") + private static ValueObject get(Bindable bindable, Binder.Context context, + BindConstructorProvider constructorProvider, ParameterNameDiscoverer parameterNameDiscoverer, + Class resolvedType) { Constructor bindConstructor = constructorProvider.getBindConstructor(bindable, context.isNestedConstructorBinding()); if (bindConstructor == null) { return null; } - if (KotlinDetector.isKotlinType(type)) { + if (KotlinDetector.isKotlinType(resolvedType)) { return KotlinValueObject.get((Constructor) bindConstructor, bindable.getType(), parameterNameDiscoverer); } return DefaultValueObject.get(bindConstructor, bindable.getType(), parameterNameDiscoverer); } + @SuppressWarnings("unchecked") + private static Map getCache(Context context) { + Map cache = (Map) context.getCache().get(ValueObject.class); + if (cache == null) { + cache = new ConcurrentHashMap<>(); + context.getCache().put(ValueObject.class, cache); + } + return cache; + } + + private record CacheKey(Bindable bindable, BindConstructorProvider constructorProvider, + ParameterNameDiscoverer parameterNameDiscoverer) { + + } + } /**