Consistent and efficient access to BeanDefinition argument values

Issue: SPR-16192

spring-beans/src/main/java/org/springframework/beans/MutablePropertyValues.java

@@ -318,7 +318,7 @@ public class MutablePropertyValues implements PropertyValues, Serializable {
 	 */
 	public void registerProcessedProperty(String propertyName) {
 		if (this.processedProperties == null) {
-			this.processedProperties = new HashSet<>();
+			this.processedProperties = new HashSet<>(4);
 		}
 		this.processedProperties.add(propertyName);
 	}

spring-beans/src/main/java/org/springframework/beans/factory/config/BeanDefinition.java

@@ -219,6 +219,14 @@ public interface BeanDefinition extends AttributeAccessor, BeanMetadataElement {
 	 */
 	ConstructorArgumentValues getConstructorArgumentValues();
 
+	/**
+	 * Return if there are constructor argument values defined for this bean.
+	 * @since 5.0.2
+	 */
+	default boolean hasConstructorArgumentValues() {
+		return !getConstructorArgumentValues().isEmpty();
+	}
+
 	/**
 	 * Return the property values to be applied to a new instance of the bean.
 	 * <p>The returned instance can be modified during bean factory post-processing.
@@ -226,6 +234,14 @@ public interface BeanDefinition extends AttributeAccessor, BeanMetadataElement {
 	 */
 	MutablePropertyValues getPropertyValues();
 
+	/**
+	 * Return if there are property values values defined for this bean.
+	 * @since 5.0.2
+	 */
+	default boolean hasPropertyValues() {
+		return !getPropertyValues().isEmpty();
+	}
+
 
 	// Read-only attributes
 

spring-beans/src/main/java/org/springframework/beans/factory/config/BeanDefinitionVisitor.java

@@ -81,10 +81,14 @@ public class BeanDefinitionVisitor {
 		visitFactoryBeanName(beanDefinition);
 		visitFactoryMethodName(beanDefinition);
 		visitScope(beanDefinition);
-		visitPropertyValues(beanDefinition.getPropertyValues());
-		ConstructorArgumentValues cas = beanDefinition.getConstructorArgumentValues();
-		visitIndexedArgumentValues(cas.getIndexedArgumentValues());
-		visitGenericArgumentValues(cas.getGenericArgumentValues());
+		if (beanDefinition.hasPropertyValues()) {
+			visitPropertyValues(beanDefinition.getPropertyValues());
+		}
+		if (beanDefinition.hasConstructorArgumentValues()) {
+			ConstructorArgumentValues cas = beanDefinition.getConstructorArgumentValues();
+			visitIndexedArgumentValues(cas.getIndexedArgumentValues());
+			visitGenericArgumentValues(cas.getGenericArgumentValues());
+		}
 	}
 
 	protected void visitParentName(BeanDefinition beanDefinition) {

spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractAutowireCapableBeanFactory.java

@@ -723,7 +723,8 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac
 		// Can't clearly figure out exact method due to type converting / autowiring!
 		Class<?> commonType = null;
 		Method uniqueCandidate = null;
-		int minNrOfArgs = mbd.getConstructorArgumentValues().getArgumentCount();
+		int minNrOfArgs =
+				(mbd.hasConstructorArgumentValues() ? mbd.getConstructorArgumentValues().getArgumentCount() : 0);
 		Method[] candidates = ReflectionUtils.getUniqueDeclaredMethods(factoryClass);
 		for (Method factoryMethod : candidates) {
 			if (Modifier.isStatic(factoryMethod.getModifiers()) == isStatic &&
@@ -1277,10 +1278,8 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac
 	 * @param bw BeanWrapper with bean instance
 	 */
 	protected void populateBean(String beanName, RootBeanDefinition mbd, @Nullable BeanWrapper bw) {
-		PropertyValues pvs = mbd.getPropertyValues();
-
 		if (bw == null) {
-			if (!pvs.isEmpty()) {
+			if (mbd.hasPropertyValues()) {
 				throw new BeanCreationException(
 						mbd.getResourceDescription(), beanName, "Cannot apply property values to null instance");
 			}
@@ -1311,6 +1310,8 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac
 			return;
 		}
 
+		PropertyValues pvs = (mbd.hasPropertyValues() ? mbd.getPropertyValues() : null);
+
 		if (mbd.getResolvedAutowireMode() == RootBeanDefinition.AUTOWIRE_BY_NAME ||
 				mbd.getResolvedAutowireMode() == RootBeanDefinition.AUTOWIRE_BY_TYPE) {
 			MutablePropertyValues newPvs = new MutablePropertyValues(pvs);
@@ -1332,6 +1333,9 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac
 		boolean needsDepCheck = (mbd.getDependencyCheck() != RootBeanDefinition.DEPENDENCY_CHECK_NONE);
 
 		if (hasInstAwareBpps || needsDepCheck) {
+			if (pvs == null) {
+				pvs = mbd.getPropertyValues();
+			}
 			PropertyDescriptor[] filteredPds = filterPropertyDescriptorsForDependencyCheck(bw, mbd.allowCaching);
 			if (hasInstAwareBpps) {
 				for (BeanPostProcessor bp : getBeanPostProcessors()) {
@@ -1349,7 +1353,9 @@ public abstract class AbstractAutowireCapableBeanFactory extends AbstractBeanFac
 			}
 		}
 
-		applyPropertyValues(beanName, mbd, bw, pvs);
+		if (pvs != null) {
+			applyPropertyValues(beanName, mbd, bw, pvs);
+		}
 	}
 
 	/**

spring-beans/src/main/java/org/springframework/beans/factory/support/AbstractBeanDefinition.java

@@ -173,11 +173,14 @@ public abstract class AbstractBeanDefinition extends BeanMetadataAttributeAccess
 	@Nullable
 	private String factoryMethodName;
 
+	@Nullable
 	private ConstructorArgumentValues constructorArgumentValues;
 
+	@Nullable
 	private MutablePropertyValues propertyValues;
 
-	private MethodOverrides methodOverrides = new MethodOverrides();
+	@Nullable
+	private MethodOverrides methodOverrides;
 
 	@Nullable
 	private String initMethodName;
@@ -212,8 +215,8 @@ public abstract class AbstractBeanDefinition extends BeanMetadataAttributeAccess
 	 * constructor argument values and property values.
 	 */
 	protected AbstractBeanDefinition(@Nullable ConstructorArgumentValues cargs, @Nullable MutablePropertyValues pvs) {
-		this.constructorArgumentValues = (cargs != null ? cargs : new ConstructorArgumentValues());
-		this.propertyValues = (pvs != null ? pvs : new MutablePropertyValues());
+		this.constructorArgumentValues = cargs;
+		this.propertyValues = pvs;
 	}
 
 	/**
@@ -229,8 +232,6 @@ public abstract class AbstractBeanDefinition extends BeanMetadataAttributeAccess
 		setLazyInit(original.isLazyInit());
 		setFactoryBeanName(original.getFactoryBeanName());
 		setFactoryMethodName(original.getFactoryMethodName());
-		this.constructorArgumentValues = new ConstructorArgumentValues(original.getConstructorArgumentValues());
-		this.propertyValues = new MutablePropertyValues(original.getPropertyValues());
 		setRole(original.getRole());
 		setSource(original.getSource());
 		copyAttributesFrom(original);
@@ -240,6 +241,15 @@ public abstract class AbstractBeanDefinition extends BeanMetadataAttributeAccess
 			if (originalAbd.hasBeanClass()) {
 				setBeanClass(originalAbd.getBeanClass());
 			}
+			if (originalAbd.hasConstructorArgumentValues()) {
+				setConstructorArgumentValues(new ConstructorArgumentValues(original.getConstructorArgumentValues()));
+			}
+			if (originalAbd.hasPropertyValues()) {
+				setPropertyValues(new MutablePropertyValues(original.getPropertyValues()));
+			}
+			if (originalAbd.hasMethodOverrides()) {
+				setMethodOverrides(new MethodOverrides(originalAbd.getMethodOverrides()));
+			}
 			setAutowireMode(originalAbd.getAutowireMode());
 			setDependencyCheck(originalAbd.getDependencyCheck());
 			setDependsOn(originalAbd.getDependsOn());
@@ -249,7 +259,6 @@ public abstract class AbstractBeanDefinition extends BeanMetadataAttributeAccess
 			setInstanceSupplier(originalAbd.getInstanceSupplier());
 			setNonPublicAccessAllowed(originalAbd.isNonPublicAccessAllowed());
 			setLenientConstructorResolution(originalAbd.isLenientConstructorResolution());
-			setMethodOverrides(new MethodOverrides(originalAbd.getMethodOverrides()));
 			setInitMethodName(originalAbd.getInitMethodName());
 			setEnforceInitMethod(originalAbd.isEnforceInitMethod());
 			setDestroyMethodName(originalAbd.getDestroyMethodName());
@@ -258,6 +267,8 @@ public abstract class AbstractBeanDefinition extends BeanMetadataAttributeAccess
 			setResource(originalAbd.getResource());
 		}
 		else {
+			setConstructorArgumentValues(new ConstructorArgumentValues(original.getConstructorArgumentValues()));
+			setPropertyValues(new MutablePropertyValues(original.getPropertyValues()));
 			setResourceDescription(original.getResourceDescription());
 		}
 	}
@@ -294,8 +305,6 @@ public abstract class AbstractBeanDefinition extends BeanMetadataAttributeAccess
 		if (StringUtils.hasLength(other.getFactoryMethodName())) {
 			setFactoryMethodName(other.getFactoryMethodName());
 		}
-		getConstructorArgumentValues().addArgumentValues(other.getConstructorArgumentValues());
-		getPropertyValues().addPropertyValues(other.getPropertyValues());
 		setRole(other.getRole());
 		setSource(other.getSource());
 		copyAttributesFrom(other);
@@ -305,6 +314,15 @@ public abstract class AbstractBeanDefinition extends BeanMetadataAttributeAccess
 			if (otherAbd.hasBeanClass()) {
 				setBeanClass(otherAbd.getBeanClass());
 			}
+			if (otherAbd.hasConstructorArgumentValues()) {
+				getConstructorArgumentValues().addArgumentValues(other.getConstructorArgumentValues());
+			}
+			if (otherAbd.hasPropertyValues()) {
+				getPropertyValues().addPropertyValues(other.getPropertyValues());
+			}
+			if (otherAbd.hasMethodOverrides()) {
+				getMethodOverrides().addOverrides(otherAbd.getMethodOverrides());
+			}
 			setAutowireMode(otherAbd.getAutowireMode());
 			setDependencyCheck(otherAbd.getDependencyCheck());
 			setDependsOn(otherAbd.getDependsOn());
@@ -314,7 +332,6 @@ public abstract class AbstractBeanDefinition extends BeanMetadataAttributeAccess
 			setInstanceSupplier(otherAbd.getInstanceSupplier());
 			setNonPublicAccessAllowed(otherAbd.isNonPublicAccessAllowed());
 			setLenientConstructorResolution(otherAbd.isLenientConstructorResolution());
-			getMethodOverrides().addOverrides(otherAbd.getMethodOverrides());
 			if (otherAbd.getInitMethodName() != null) {
 				setInitMethodName(otherAbd.getInitMethodName());
 				setEnforceInitMethod(otherAbd.isEnforceInitMethod());
@@ -327,6 +344,8 @@ public abstract class AbstractBeanDefinition extends BeanMetadataAttributeAccess
 			setResource(otherAbd.getResource());
 		}
 		else {
+			getConstructorArgumentValues().addArgumentValues(other.getConstructorArgumentValues());
+			getPropertyValues().addPropertyValues(other.getPropertyValues());
 			setResourceDescription(other.getResourceDescription());
 		}
 	}
@@ -778,9 +797,8 @@ public abstract class AbstractBeanDefinition extends BeanMetadataAttributeAccess
 	/**
 	 * Specify constructor argument values for this bean.
 	 */
-	public void setConstructorArgumentValues(@Nullable ConstructorArgumentValues constructorArgumentValues) {
-		this.constructorArgumentValues =
-				(constructorArgumentValues != null ? constructorArgumentValues : new ConstructorArgumentValues());
+	public void setConstructorArgumentValues(ConstructorArgumentValues constructorArgumentValues) {
+		this.constructorArgumentValues = constructorArgumentValues;
 	}
 
 	/**
@@ -788,6 +806,9 @@ public abstract class AbstractBeanDefinition extends BeanMetadataAttributeAccess
 	 */
 	@Override
 	public ConstructorArgumentValues getConstructorArgumentValues() {
+		if (this.constructorArgumentValues == null) {
+			this.constructorArgumentValues = new ConstructorArgumentValues();
+		}
 		return this.constructorArgumentValues;
 	}
 
@@ -795,14 +816,14 @@ public abstract class AbstractBeanDefinition extends BeanMetadataAttributeAccess
 	 * Return if there are constructor argument values defined for this bean.
 	 */
 	public boolean hasConstructorArgumentValues() {
-		return !this.constructorArgumentValues.isEmpty();
+		return (this.constructorArgumentValues != null && !this.constructorArgumentValues.isEmpty());
 	}
 
 	/**
 	 * Specify property values for this bean, if any.
 	 */
-	public void setPropertyValues(@Nullable MutablePropertyValues propertyValues) {
-		this.propertyValues = (propertyValues != null ? propertyValues : new MutablePropertyValues());
+	public void setPropertyValues(MutablePropertyValues propertyValues) {
+		this.propertyValues = propertyValues;
 	}
 
 	/**
@@ -810,14 +831,25 @@ public abstract class AbstractBeanDefinition extends BeanMetadataAttributeAccess
 	 */
 	@Override
 	public MutablePropertyValues getPropertyValues() {
+		if (this.propertyValues == null) {
+			this.propertyValues = new MutablePropertyValues();
+		}
 		return this.propertyValues;
 	}
 
+	/**
+	 * Return if there are property values values defined for this bean.
+	 * @since 5.0.2
+	 */
+	public boolean hasPropertyValues() {
+		return (this.propertyValues != null && !this.propertyValues.isEmpty());
+	}
+
 	/**
 	 * Specify method overrides for the bean, if any.
 	 */
-	public void setMethodOverrides(@Nullable MethodOverrides methodOverrides) {
-		this.methodOverrides = (methodOverrides != null ? methodOverrides : new MethodOverrides());
+	public void setMethodOverrides(MethodOverrides methodOverrides) {
+		this.methodOverrides = methodOverrides;
 	}
 
 	/**
@@ -826,9 +858,20 @@ public abstract class AbstractBeanDefinition extends BeanMetadataAttributeAccess
 	 * <p>Never returns {@code null}.
 	 */
 	public MethodOverrides getMethodOverrides() {
+		if (this.methodOverrides == null) {
+			this.methodOverrides = new MethodOverrides();
+		}
 		return this.methodOverrides;
 	}
 
+	/**
+	 * Return if there are method overrides defined for this bean.
+	 * @since 5.0.2
+	 */
+	public boolean hasMethodOverrides() {
+		return (this.methodOverrides != null && !this.methodOverrides.isEmpty());
+	}
+
 	/**
 	 * Set the name of the initializer method.
 	 * <p>The default is {@code null} in which case there is no initializer method.
@@ -1002,7 +1045,7 @@ public abstract class AbstractBeanDefinition extends BeanMetadataAttributeAccess
 	 * @throws BeanDefinitionValidationException in case of validation failure
 	 */
 	public void validate() throws BeanDefinitionValidationException {
-		if (!getMethodOverrides().isEmpty() && getFactoryMethodName() != null) {
+		if (hasMethodOverrides() && getFactoryMethodName() != null) {
 			throw new BeanDefinitionValidationException(
 					"Cannot combine static factory method with method overrides: " +
 					"the static factory method must create the instance");
@@ -1020,9 +1063,8 @@ public abstract class AbstractBeanDefinition extends BeanMetadataAttributeAccess
 	 */
 	public void prepareMethodOverrides() throws BeanDefinitionValidationException {
 		// Check that lookup methods exists.
-		MethodOverrides methodOverrides = getMethodOverrides();
-		if (!methodOverrides.isEmpty()) {
-			Set<MethodOverride> overrides = methodOverrides.getOverrides();
+		if (hasMethodOverrides()) {
+			Set<MethodOverride> overrides = getMethodOverrides().getOverrides();
 			synchronized (overrides) {
 				for (MethodOverride mo : overrides) {
 					prepareMethodOverride(mo);

spring-beans/src/main/java/org/springframework/beans/factory/support/ConstructorResolver.java

@@ -434,9 +434,14 @@ class ConstructorResolver {
 			else {
 				// We don't have arguments passed in programmatically, so we need to resolve the
 				// arguments specified in the constructor arguments held in the bean definition.
-				ConstructorArgumentValues cargs = mbd.getConstructorArgumentValues();
-				resolvedValues = new ConstructorArgumentValues();
-				minNrOfArgs = resolveConstructorArguments(beanName, mbd, bw, cargs, resolvedValues);
+				if (mbd.hasConstructorArgumentValues()) {
+					ConstructorArgumentValues cargs = mbd.getConstructorArgumentValues();
+					resolvedValues = new ConstructorArgumentValues();
+					minNrOfArgs = resolveConstructorArguments(beanName, mbd, bw, cargs, resolvedValues);
+				}
+				else {
+					minNrOfArgs = 0;
+				}
 			}
 
 			LinkedList<UnsatisfiedDependencyException> causes = null;
@@ -447,7 +452,14 @@ class ConstructorResolver {
 				if (paramTypes.length >= minNrOfArgs) {
 					ArgumentsHolder argsHolder;
 
-					if (resolvedValues != null) {
+					if (explicitArgs != null){
+						// Explicit arguments given -> arguments length must match exactly.
+						if (paramTypes.length != explicitArgs.length) {
+							continue;
+						}
+						argsHolder = new ArgumentsHolder(explicitArgs);
+					}
+					else {
 						// Resolved constructor arguments: type conversion and/or autowiring necessary.
 						try {
 							String[] paramNames = null;
@@ -472,14 +484,6 @@ class ConstructorResolver {
 						}
 					}
 
-					else {
-						// Explicit arguments given -> arguments length must match exactly.
-						if (paramTypes.length != explicitArgs.length) {
-							continue;
-						}
-						argsHolder = new ArgumentsHolder(explicitArgs);
-					}
-
 					int typeDiffWeight = (mbd.isLenientConstructorResolution() ?
 							argsHolder.getTypeDifferenceWeight(paramTypes) : argsHolder.getAssignabilityWeight(paramTypes));
 					// Choose this factory method if it represents the closest match.
@@ -522,7 +526,7 @@ class ConstructorResolver {
 						argTypes.add(arg != null ? arg.getClass().getSimpleName() : "null");
 					}
 				}
-				else {
+				else if (resolvedValues != null){
 					Set<ValueHolder> valueHolders = new LinkedHashSet<>(resolvedValues.getArgumentCount());
 					valueHolders.addAll(resolvedValues.getIndexedArgumentValues().values());
 					valueHolders.addAll(resolvedValues.getGenericArgumentValues());
@@ -645,7 +649,7 @@ class ConstructorResolver {
 	 * given the resolved constructor argument values.
 	 */
 	private ArgumentsHolder createArgumentArray(
-			String beanName, RootBeanDefinition mbd, ConstructorArgumentValues resolvedValues,
+			String beanName, RootBeanDefinition mbd, @Nullable ConstructorArgumentValues resolvedValues,
 			BeanWrapper bw, Class<?>[] paramTypes, @Nullable String[] paramNames, Executable executable,
 			boolean autowiring) throws UnsatisfiedDependencyException {
 
@@ -660,13 +664,15 @@ class ConstructorResolver {
 			Class<?> paramType = paramTypes[paramIndex];
 			String paramName = (paramNames != null ? paramNames[paramIndex] : "");
 			// Try to find matching constructor argument value, either indexed or generic.
-			ConstructorArgumentValues.ValueHolder valueHolder =
-					resolvedValues.getArgumentValue(paramIndex, paramType, paramName, usedValueHolders);
-			// If we couldn't find a direct match and are not supposed to autowire,
-			// let's try the next generic, untyped argument value as fallback:
-			// it could match after type conversion (for example, String -> int).
-			if (valueHolder == null && (!autowiring || paramTypes.length == resolvedValues.getArgumentCount())) {
-				valueHolder = resolvedValues.getGenericArgumentValue(null, null, usedValueHolders);
+			ConstructorArgumentValues.ValueHolder valueHolder = null;
+			if (resolvedValues != null) {
+				valueHolder = resolvedValues.getArgumentValue(paramIndex, paramType, paramName, usedValueHolders);
+				// If we couldn't find a direct match and are not supposed to autowire,
+				// let's try the next generic, untyped argument value as fallback:
+				// it could match after type conversion (for example, String -> int).
+				if (valueHolder == null && (!autowiring || paramTypes.length == resolvedValues.getArgumentCount())) {
+					valueHolder = resolvedValues.getGenericArgumentValue(null, null, usedValueHolders);
+				}
 			}
 			if (valueHolder != null) {
 				// We found a potential match - let's give it a try.

spring-beans/src/main/java/org/springframework/beans/factory/support/MethodOverrides.java

@@ -37,7 +37,7 @@ import org.springframework.lang.Nullable;
  */
 public class MethodOverrides {
 
-	private final Set<MethodOverride> overrides = Collections.synchronizedSet(new LinkedHashSet<>(0));
+	private final Set<MethodOverride> overrides = Collections.synchronizedSet(new LinkedHashSet<>(2));
 
 	private volatile boolean modified = false;
 

spring-beans/src/main/java/org/springframework/beans/factory/support/RootBeanDefinition.java

@@ -190,7 +190,9 @@ public class RootBeanDefinition extends AbstractBeanDefinition {
 	 * @param cargs the constructor argument values to apply
 	 * @param pvs the property values to apply
 	 */
-	public RootBeanDefinition(@Nullable Class<?> beanClass, ConstructorArgumentValues cargs, @Nullable MutablePropertyValues pvs) {
+	public RootBeanDefinition(@Nullable Class<?> beanClass, @Nullable ConstructorArgumentValues cargs,
+			@Nullable MutablePropertyValues pvs) {
+
 		super(cargs, pvs);
 		setBeanClass(beanClass);
 	}

spring-beans/src/main/java/org/springframework/beans/factory/support/SimpleInstantiationStrategy.java

@@ -60,7 +60,7 @@ public class SimpleInstantiationStrategy implements InstantiationStrategy {
 	@Override
 	public Object instantiate(RootBeanDefinition bd, @Nullable String beanName, BeanFactory owner) {
 		// Don't override the class with CGLIB if no overrides.
-		if (bd.getMethodOverrides().isEmpty()) {
+		if (!bd.hasMethodOverrides()) {
 			Constructor<?> constructorToUse;
 			synchronized (bd.constructorArgumentLock) {
 				constructorToUse = (Constructor<?>) bd.resolvedConstructorOrFactoryMethod;
@@ -72,8 +72,7 @@ public class SimpleInstantiationStrategy implements InstantiationStrategy {
 					try {
 						if (System.getSecurityManager() != null) {
 							constructorToUse = AccessController.doPrivileged(
-									(PrivilegedExceptionAction<Constructor<?>>) () ->
-											clazz.getDeclaredConstructor());
+									(PrivilegedExceptionAction<Constructor<?>>) () -> clazz.getDeclaredConstructor());
 						}
 						else {
 							constructorToUse =	clazz.getDeclaredConstructor();
@@ -107,7 +106,7 @@ public class SimpleInstantiationStrategy implements InstantiationStrategy {
 	public Object instantiate(RootBeanDefinition bd, @Nullable String beanName, BeanFactory owner,
 			final Constructor<?> ctor, @Nullable Object... args) {
 
-		if (bd.getMethodOverrides().isEmpty()) {
+		if (!bd.hasMethodOverrides()) {
 			if (System.getSecurityManager() != null) {
 				// use own privileged to change accessibility (when security is on)
 				AccessController.doPrivileged((PrivilegedAction<Object>) () -> {