From 95edcb81b532accf30d35496468977ed87e529a5 Mon Sep 17 00:00:00 2001 From: Phillip Webb Date: Wed, 17 Jul 2019 13:45:09 +0100 Subject: [PATCH] Retain merged bean definition caches when possible Update the logic in `AbstractBeanFactory` so that caches from merged bean definitions remain whenever possible. Prior to this commit, all merged bean definitions would be completely removed after bean post processing in case a processor changed the bean type. It's fairly unlikely these days that the bean type will actually change, so instead we now compare a subset of the old cached properties against the newly created definition. Only if key properties have changed do we now discard the older cached values. Closes gh-23336 --- .../AbstractAutowireCapableBeanFactory.java | 7 ++-- .../factory/support/AbstractBeanFactory.java | 36 ++++++++++++++++--- .../factory/support/RootBeanDefinition.java | 3 ++ 3 files changed, 38 insertions(+), 8 deletions(-) diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java index 0c314a5b94..a84f675f0e 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java @@ -114,6 +114,7 @@ import org.springframework.util.StringUtils; * @author Costin Leau * @author Chris Beams * @author Sam Brannen + * @author Phillip Webb * @since 13.02.2004 * @see RootBeanDefinition * @see DefaultListableBeanFactory @@ -646,16 +647,16 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac @Nullable protected Class predictBeanType(String beanName, RootBeanDefinition mbd, Class... typesToMatch) { Class targetType = determineTargetType(beanName, mbd, typesToMatch); - // Apply SmartInstantiationAwareBeanPostProcessors to predict the // eventual type after a before-instantiation shortcut. if (targetType != null && !mbd.isSynthetic() && hasInstantiationAwareBeanPostProcessors()) { + boolean matchingOnlyFactoryBean = typesToMatch.length == 1 && typesToMatch[0] == FactoryBean.class; for (BeanPostProcessor bp : getBeanPostProcessors()) { if (bp instanceof SmartInstantiationAwareBeanPostProcessor) { SmartInstantiationAwareBeanPostProcessor ibp = (SmartInstantiationAwareBeanPostProcessor) bp; Class predicted = ibp.predictBeanType(targetType, beanName); - if (predicted != null && (typesToMatch.length != 1 || FactoryBean.class != typesToMatch[0] || - FactoryBean.class.isAssignableFrom(predicted))) { + if (predicted != null && + (!matchingOnlyFactoryBean || FactoryBean.class.isAssignableFrom(predicted))) { return predicted; } } diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java index e6fcceb9e4..73f0f95c01 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanFactory.java @@ -103,6 +103,7 @@ import org.springframework.util.StringValueResolver; * @author Juergen Hoeller * @author Costin Leau * @author Chris Beams + * @author Phillip Webb * @since 15 April 2001 * @see #getBeanDefinition * @see #createBean @@ -1215,7 +1216,7 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp protected RootBeanDefinition getMergedLocalBeanDefinition(String beanName) throws BeansException { // Quick check on the concurrent map first, with minimal locking. RootBeanDefinition mbd = this.mergedBeanDefinitions.get(beanName); - if (mbd != null) { + if (mbd != null && !mbd.stale) { return mbd; } return getMergedBeanDefinition(beanName, getBeanDefinition(beanName)); @@ -1251,13 +1252,16 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp synchronized (this.mergedBeanDefinitions) { RootBeanDefinition mbd = null; + RootBeanDefinition previous = null; // Check with full lock now in order to enforce the same merged instance. if (containingBd == null) { mbd = this.mergedBeanDefinitions.get(beanName); } - if (mbd == null) { + if (mbd == null || mbd.stale) { + previous = mbd; + mbd = null; if (bd.getParentName() == null) { // Use copy of given root bean definition. if (bd instanceof RootBeanDefinition) { @@ -1315,11 +1319,26 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp this.mergedBeanDefinitions.put(beanName, mbd); } } - + if (previous != null) { + copyRelevantMergedBeanDefinitionCaches(previous, mbd); + } return mbd; } } + private void copyRelevantMergedBeanDefinitionCaches(RootBeanDefinition previous, + RootBeanDefinition mbd) { + if (ObjectUtils.nullSafeEquals(mbd.getBeanClassName(), previous.getBeanClassName()) && + ObjectUtils.nullSafeEquals(mbd.getFactoryBeanName(), previous.getFactoryBeanName()) && + ObjectUtils.nullSafeEquals(mbd.getFactoryMethodName(), previous.getFactoryMethodName()) && + (mbd.targetType == null || mbd.targetType.equals(previous.targetType))) { + mbd.targetType = previous.targetType; + mbd.resolvedTargetType = previous.resolvedTargetType; + mbd.factoryMethodReturnType = previous.factoryMethodReturnType; + mbd.factoryMethodToIntrospect = previous.factoryMethodToIntrospect; + } + } + /** * Check the given merged bean definition, * potentially throwing validation exceptions. @@ -1342,7 +1361,10 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp * @param beanName the bean name to clear the merged definition for */ protected void clearMergedBeanDefinition(String beanName) { - this.mergedBeanDefinitions.remove(beanName); + RootBeanDefinition bd = this.mergedBeanDefinitions.get(beanName); + if (bd != null) { + bd.stale = true; + } } /** @@ -1354,7 +1376,11 @@ public abstract class AbstractBeanFactory extends FactoryBeanRegistrySupport imp * @since 4.2 */ public void clearMetadataCache() { - this.mergedBeanDefinitions.keySet().removeIf(bean -> !isBeanEligibleForMetadataCaching(bean)); + this.mergedBeanDefinitions.forEach((beanName, bd) -> { + if (!isBeanEligibleForMetadataCaching(beanName)) { + bd.stale = true; + } + }); } /** diff --git a/spring-beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java b/spring-beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java index 19fdb8314f..f393829e74 100644 --- a/spring-beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java +++ b/spring-beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java @@ -60,6 +60,9 @@ public class RootBeanDefinition extends AbstractBeanDefinition { @Nullable private AnnotatedElement qualifiedElement; + /** Determines if the definition needs to be re-merged. */ + volatile boolean stale; + boolean allowCaching = true; boolean isFactoryMethodUnique = false;