Optimize ConfigurationClassPostProcessor#enhanceConfigurationClasses method to shorten startup time

[yourbatman]

The Version of the Spring Framework I used: 5.2.8

ConfigurationClassPostProcessor#enhanceConfigurationClasses method determines the @configuration class in full mode:

if beanFactory.containsSingleton(beanName) is true.The output info log tells me that the Bean cannot be enhanced，But it still puts it inside 'configBeanDefs'.And then execute enhancer.enhance(configClass, this.beanClassLoader); to enhance it.

Although,enhanced instances do not end up in the IoC container（Because it already exists in the container）.But it's semantically inconsistent.It also creates unnecessary overhead in terms of performance.Slow application startup.

For example：

1、Define a @configuration class AppConfig:

@Configuration
public class AppConfig {
    
    @Bean
    public BeanDefinitionRegistryPostProcessor postProcessor() {
        return new BeanDefinitionRegistryPostProcessor() {
            @Override
            public void postProcessBeanDefinitionRegistry(BeanDefinitionRegistry registry) throws BeansException {
            }

            @Override
            public void postProcessBeanFactory(ConfigurableListableBeanFactory beanFactory) throws BeansException {
            }
        };
    }

}

2、debug run Spring container.Screenshot as follows:

Console log：

The log says that AppConfig will not be enhanced.But the enhanced logic is actually executed,and there are contradictions.

Solution：I think the following changes can be made to the code to avoid unnecessary performance losses and speed up startup

public void enhanceConfigurationClasses(ConfigurableListableBeanFactory beanFactory) {
    ...
			if (ConfigurationClassUtils.CONFIGURATION_CLASS_FULL.equals(configClassAttr)) {
				if (!(beanDef instanceof AbstractBeanDefinition)) {
					throw new BeanDefinitionStoreException("Cannot enhance @Configuration bean definition '" +
							beanName + "' since it is not stored in an AbstractBeanDefinition subclass");
				}
				else if (logger.isInfoEnabled() && beanFactory.containsSingleton(beanName)) {
					logger.info("Cannot enhance @Configuration bean definition '" + beanName +
							"' since its singleton instance has been created too early. The typical cause " +
							"is a non-static @Bean method with a BeanDefinitionRegistryPostProcessor " +
							"return type: Consider declaring such methods as 'static'.");
				} 
                                else {
                                        configBeanDefs.put(beanName, (AbstractBeanDefinition) beanDef);
                                }
				
			}
    ...
}

Just make configBeanDefs.put(beanName, (AbstractBeanDefinition) beanDef); at the independent else branch.

[mdeinum]

Which would still fail if info logging would be disabled because it is part of the expression.

[yourbatman]

Which would still fail if info logging would be disabled because it is part of the expression.

What I mean is that if you print the Info log, you should not put it into configBeanDefs to perform the enhanced logic, because that makes no sense.

[mdeinum]

I know what you meant, but adding an else isn't going to solve it. When warn` is enabled and the bean is already in the bean factory it now still would be enhanced. The whole point should be that the bean shouldn't be enhanced when it is already existing as a singleton, regardless of info being enabled or not.

[yourbatman]

I know what you meant, but adding an else isn't going to solve it. When warn` is enabled and the bean is already in the bean factory it now still would be enhanced. The whole point should be that the bean shouldn't be enhanced when it is already existing as a singleton, regardless of info being enabled or not.

I take your point:“the bean is already in the bean factory it now still would be enhanced”.But for this judgment beanFactory.containsSingleton(beanName) does this mean that the singleton Bean already exists? If so, it should not be put into 'configBeanDefs' any more.

[bclozel]

I've tested the following change and ran the entire Spring Framework test suite successfully.

--- a/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassPostProcessor.java
+++ b/spring-context/src/main/java/org/springframework/context/annotation/ConfigurationClassPostProcessor.java
@@ -507,14 +507,18 @@ public class ConfigurationClassPostProcessor implements BeanDefinitionRegistryPo
                                        throw new BeanDefinitionStoreException("Cannot enhance @Configuration bean definition '" +
                                                        beanName + "' since it is not stored in an AbstractBeanDefinition subclass");
                                }
-                               else if (logger.isWarnEnabled() && beanFactory.containsSingleton(beanName)) {
-                                       logger.warn("Cannot enhance @Configuration bean definition '" + beanName +
-                                                       "' since its singleton instance has been created too early. The typical cause " +
-                                                       "is a non-static @Bean method with a BeanDefinitionRegistryPostProcessor " +
-                                                       "return type: Consider declaring such methods as 'static' and/or mark the " +
-                                                       "containing configuration class as 'proxyBeanMethods=false'.");
+                               else if (beanFactory.containsSingleton(beanName)) {
+                                       if (logger.isWarnEnabled()) {
+                                               logger.warn("Cannot enhance @Configuration bean definition '" + beanName +
+                                                               "' since its singleton instance has been created too early. The typical cause " +
+                                                               "is a non-static @Bean method with a BeanDefinitionRegistryPostProcessor " +
+                                                               "return type: Consider declaring such methods as 'static' and/or mark the " +
+                                                               "containing configuration class as 'proxyBeanMethods=false'.");
+                                       }
+                               }
+                               else {
+                                       configBeanDefs.put(beanName, abd);
                                }
-                               configBeanDefs.put(beanName, abd);
                        }
                }
                if (configBeanDefs.isEmpty()) {

Should we consider this change in the 6.2 timeline @jhoeller?

[brucelwl]

Similar issues : #22990