Merge branch cbeams/SPR-6870

* SPR-6870: Cache by-type lookups in DefaultListableBeanFactory Polish

Author: cbeams

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

@@ -21,13 +21,14 @@
 import java.io.ObjectInputStream;
 import java.io.ObjectStreamException;
 import java.io.Serializable;
+
 import java.lang.annotation.Annotation;
 import java.lang.ref.Reference;
 import java.lang.ref.WeakReference;
-import java.lang.reflect.ParameterizedType;
-import java.lang.reflect.Type;
+
 import java.security.AccessController;
 import java.security.PrivilegedAction;
+
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collection;
@@ -38,6 +39,7 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
+
 import javax.inject.Provider;
 
 import org.springframework.beans.BeansException;
@@ -90,6 +92,7 @@
  * @author Juergen Hoeller
  * @author Sam Brannen
  * @author Costin Leau
+ * @author Chris Beams
  * @since 16 April 2001
  * @see StaticListableBeanFactory
  * @see PropertiesBeanDefinitionReader
@@ -98,7 +101,7 @@
 public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFactory
 implements ConfigurableListableBeanFactory, BeanDefinitionRegistry, Serializable {
 
-private static Class javaxInjectProviderClass = null;
+private static Class<?> javaxInjectProviderClass = null;
 
 static {
 ClassLoader cl = DefaultListableBeanFactory.class.getClassLoader();
@@ -128,11 +131,17 @@ public class DefaultListableBeanFactory extends AbstractAutowireCapableBeanFacto
 private AutowireCandidateResolver autowireCandidateResolver = new SimpleAutowireCandidateResolver();
 
 /** Map from dependency type to corresponding autowired value */
-private final Map<Class, Object> resolvableDependencies = new HashMap<Class, Object>();
+private final Map<Class<?>, Object> resolvableDependencies = new HashMap<Class<?>, Object>();
 
 /** Map of bean definition objects, keyed by bean name */
 private final Map<String, BeanDefinition> beanDefinitionMap = new ConcurrentHashMap<String, BeanDefinition>();
 
+/** Map of singleton bean names keyed by bean class */
+private final Map<Class<?>, String[]> singletonBeanNamesByType = new ConcurrentHashMap<Class<?>, String[]>();
+
+/** Map of non-singleton bean names keyed by bean class */
+private final Map<Class<?>, String[]> nonSingletonBeanNamesByType = new ConcurrentHashMap<Class<?>, String[]>();
+
 /** List of bean definition names, in registration order */
 private final List<String> beanDefinitionNames = new ArrayList<String>();
 
@@ -294,11 +303,26 @@ public String[] getBeanDefinitionNames() {
 }
 }
 
-public String[] getBeanNamesForType(Class type) {
+public String[] getBeanNamesForType(Class<?> type) {
 return getBeanNamesForType(type, true, true);
 }
 
-public String[] getBeanNamesForType(Class type, boolean includeNonSingletons, boolean allowEagerInit) {
+public String[] getBeanNamesForType(Class<?> type, boolean includeNonSingletons, boolean allowEagerInit) {
+if (type == null || !allowEagerInit) {
+return this.doGetBeanNamesForType(type, includeNonSingletons, allowEagerInit);
+}
+Map<Class<?>, String[]> cache = includeNonSingletons ?
+this.nonSingletonBeanNamesByType : this.singletonBeanNamesByType;
+String[] resolvedBeanNames = cache.get(type);
+if (resolvedBeanNames != null) {
+return resolvedBeanNames;
+}
+resolvedBeanNames = this.doGetBeanNamesForType(type, includeNonSingletons, allowEagerInit);
+cache.put(type, resolvedBeanNames);
+return resolvedBeanNames;
+}
+
+private String[] doGetBeanNamesForType(Class<?> type, boolean includeNonSingletons, boolean allowEagerInit) {
 List<String> result = new ArrayList<String>();
 
 // Check all bean definitions.
@@ -441,7 +465,7 @@ public Map<String, Object> getBeansWithAnnotation(Class<? extends Annotation> an
  */
 public <A extends Annotation> A findAnnotationOnBean(String beanName, Class<A> annotationType) {
 A ann = null;
-Class beanType = getType(beanName);
+Class<?> beanType = getType(beanName);
 if (beanType != null) {
 ann = AnnotationUtils.findAnnotation(beanType, annotationType);
 }
@@ -564,18 +588,18 @@ public void preInstantiateSingletons() throws BeansException {
 RootBeanDefinition bd = getMergedLocalBeanDefinition(beanName);
 if (!bd.isAbstract() && bd.isSingleton() && !bd.isLazyInit()) {
 if (isFactoryBean(beanName)) {
-final FactoryBean factory = (FactoryBean) getBean(FACTORY_BEAN_PREFIX + beanName);
+final FactoryBean<?> factory = (FactoryBean<?>) getBean(FACTORY_BEAN_PREFIX + beanName);
 boolean isEagerInit;
 if (System.getSecurityManager() != null && factory instanceof SmartFactoryBean) {
 isEagerInit = AccessController.doPrivileged(new PrivilegedAction<Boolean>() {
 public Boolean run() {
-return ((SmartFactoryBean) factory).isEagerInit();
+return ((SmartFactoryBean<?>) factory).isEagerInit();
 }
 }, getAccessControlContext());
 }
 else {
 isEagerInit = (factory instanceof SmartFactoryBean &&
-((SmartFactoryBean) factory).isEagerInit());
+((SmartFactoryBean<?>) factory).isEagerInit());
 }
 if (isEagerInit) {
 getBean(beanName);
@@ -669,6 +693,10 @@ protected void resetBeanDefinition(String beanName) {
 destroySingleton(beanName);
 }
 
+// Remove any assumptions about by-type mappings
+this.singletonBeanNamesByType.clear();
+this.nonSingletonBeanNamesByType.clear();
+
 // Reset all bean definitions that have the given bean as parent (recursively).
 for (String bdName : this.beanDefinitionNames) {
 if (!beanName.equals(bdName)) {
@@ -723,7 +751,7 @@ protected Object doResolveDependency(DependencyDescriptor descriptor, Class<?> t
 }
 
 if (type.isArray()) {
-Class componentType = type.getComponentType();
+Class<?> componentType = type.getComponentType();
 Map<String, Object> matchingBeans = findAutowireCandidates(beanName, componentType, descriptor);
 if (matchingBeans.isEmpty()) {
 if (descriptor.isRequired()) {
@@ -738,7 +766,7 @@ protected Object doResolveDependency(DependencyDescriptor descriptor, Class<?> t
 return converter.convertIfNecessary(matchingBeans.values(), type);
 }
 else if (Collection.class.isAssignableFrom(type) && type.isInterface()) {
-Class elementType = descriptor.getCollectionType();
+Class<?> elementType = descriptor.getCollectionType();
 if (elementType == null) {
 if (descriptor.isRequired()) {
 throw new FatalBeanException("No element type declared for collection [" + type.getName() + "]");
@@ -759,15 +787,15 @@ else if (Collection.class.isAssignableFrom(type) && type.isInterface()) {
 return converter.convertIfNecessary(matchingBeans.values(), type);
 }
 else if (Map.class.isAssignableFrom(type) && type.isInterface()) {
-Class keyType = descriptor.getMapKeyType();
+Class<?> keyType = descriptor.getMapKeyType();
 if (keyType == null || !String.class.isAssignableFrom(keyType)) {
 if (descriptor.isRequired()) {
 throw new FatalBeanException("Key type [" + keyType + "] of map [" + type.getName() +
 "] must be assignable to [java.lang.String]");
 }
 return null;
 }
-Class valueType = descriptor.getMapValueType();
+Class<?> valueType = descriptor.getMapValueType();
 if (valueType == null) {
 if (descriptor.isRequired()) {
 throw new FatalBeanException("No value type declared for map [" + type.getName() + "]");
@@ -828,12 +856,12 @@ else if (Map.class.isAssignableFrom(type) && type.isInterface()) {
  * @see #autowireConstructor
  */
 protected Map<String, Object> findAutowireCandidates(
-String beanName, Class requiredType, DependencyDescriptor descriptor) {
+String beanName, Class<?> requiredType, DependencyDescriptor descriptor) {
 
 String[] candidateNames = BeanFactoryUtils.beanNamesForTypeIncludingAncestors(
 this, requiredType, true, descriptor.isEager());
 Map<String, Object> result = new LinkedHashMap<String, Object>(candidateNames.length);
-for (Class autowiringType : this.resolvableDependencies.keySet()) {
+for (Class<?> autowiringType : this.resolvableDependencies.keySet()) {
 if (autowiringType.isAssignableFrom(requiredType)) {
 Object autowiringValue = this.resolvableDependencies.get(autowiringType);
 autowiringValue = AutowireUtils.resolveAutowiringValue(autowiringValue, requiredType);
@@ -918,7 +946,7 @@ protected boolean matchesBeanName(String beanName, String candidateName) {
  * Raise a NoSuchBeanDefinitionException for an unresolvable dependency.
  */
 private void raiseNoSuchBeanDefinitionException(
-Class type, String dependencyDescription, DependencyDescriptor descriptor)
+Class<?> type, String dependencyDescription, DependencyDescriptor descriptor)
 throws NoSuchBeanDefinitionException {
 
 throw new NoSuchBeanDefinitionException(type, dependencyDescription,
@@ -967,6 +995,7 @@ protected Object writeReplace() throws ObjectStreamException {
  * Minimal id reference to the factory.
  * Resolved to the actual factory instance on deserialization.
  */
+@SuppressWarnings("serial")
 private static class SerializedBeanFactoryReference implements Serializable {
 
 private final String id;
@@ -976,7 +1005,7 @@ public SerializedBeanFactoryReference(String id) {
 }
 
 private Object readResolve() {
-Reference ref = serializableFactories.get(this.id);
+Reference<?> ref = serializableFactories.get(this.id);
 if (ref == null) {
 throw new IllegalStateException(
 "Cannot deserialize BeanFactory with id " + this.id + ": no factory registered for this id");
@@ -994,7 +1023,8 @@ private Object readResolve() {
 /**
  * Serializable ObjectFactory for lazy resolution of a dependency.
  */
-private class DependencyObjectFactory implements ObjectFactory, Serializable {
+@SuppressWarnings("serial")
+private class DependencyObjectFactory implements ObjectFactory<Object>, Serializable {
 
 private final DependencyDescriptor descriptor;
 
@@ -1015,7 +1045,8 @@ public Object getObject() throws BeansException {
 /**
  * Serializable ObjectFactory for lazy resolution of a dependency.
  */
-private class DependencyProvider extends DependencyObjectFactory implements Provider {
+@SuppressWarnings("serial")
+private class DependencyProvider extends DependencyObjectFactory implements Provider<Object> {
 
 public DependencyProvider(DependencyDescriptor descriptor, String beanName) {
 super(descriptor, beanName);

spring-beans/src/test/java/org/springframework/beans/factory/DefaultListableBeanFactoryTests.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2011 the original author or authors.
+ * Copyright 2002-2012 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.
@@ -16,26 +16,18 @@
 
 package org.springframework.beans.factory;
 
-import static org.hamcrest.CoreMatchers.is;
-import static org.junit.Assert.assertEquals;
-import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
-import static org.junit.Assert.assertNotSame;
-import static org.junit.Assert.assertNull;
-import static org.junit.Assert.assertSame;
-import static org.junit.Assert.assertThat;
-import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-import static org.springframework.beans.factory.support.BeanDefinitionBuilder.rootBeanDefinition;
-
 import java.lang.reflect.Field;
+
 import java.net.MalformedURLException;
+
 import java.security.AccessControlContext;
 import java.security.AccessController;
 import java.security.Principal;
 import java.security.PrivilegedAction;
+
 import java.text.NumberFormat;
 import java.text.ParseException;
+
 import java.util.Arrays;
 import java.util.Iterator;
 import java.util.List;
@@ -48,8 +40,10 @@
 
 import org.apache.commons.logging.Log;
 import org.apache.commons.logging.LogFactory;
+
 import org.junit.Ignore;
 import org.junit.Test;
+
 import org.springframework.beans.BeansException;
 import org.springframework.beans.MutablePropertyValues;
 import org.springframework.beans.NotWritablePropertyException;
@@ -92,6 +86,10 @@
 import test.beans.NestedTestBean;
 import test.beans.TestBean;
 
+import static org.hamcrest.CoreMatchers.*;
+
+import static org.junit.Assert.*;
+
 /**
  * Tests properties population and autowire behavior.
  *
@@ -2159,13 +2157,41 @@ public Object run() {
 @Test
 public void testContainsBeanReturnsTrueEvenForAbstractBeanDefinition() {
 DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
-bf.registerBeanDefinition("abs",
-rootBeanDefinition(TestBean.class).setAbstract(true).getBeanDefinition());
+bf.registerBeanDefinition("abs", BeanDefinitionBuilder
+.rootBeanDefinition(TestBean.class).setAbstract(true).getBeanDefinition());
 assertThat(bf.containsBean("abs"), is(true));
 assertThat(bf.containsBean("bogus"), is(false));
 }
 
 
+static class A { }
+static class B { }
+
+/**
+ * Test that by-type bean lookup caching is working effectively by searching for a
+ * bean of type B 10K times within a container having 1K additional beans of type A.
+ * Prior to by-type caching, each bean lookup would traverse the entire container
+ * (all 1001 beans), performing expensive assignability checks, etc. Now these
+ * operations are necessary only once, providing a dramatic performance improvement.
+ * On load-free modern hardware (e.g. an 8-core MPB), this method should complete well
+ * under the 1000 ms timeout, usually ~= 300ms. With caching removed and on the same
+ * hardware the method will take ~13000 ms. See SPR-6870.
+ */
+@Test(timeout=1000)
+public void testByTypeLookupIsFastEnough() {
+DefaultListableBeanFactory bf = new DefaultListableBeanFactory();
+
+for (int i=0; i<1000; i++) {
+bf.registerBeanDefinition("a"+i, new RootBeanDefinition(A.class));
+}
+bf.registerBeanDefinition("b", new RootBeanDefinition(B.class));
+
+for (int i=0; i<10000; i++) {
+bf.getBean(B.class);
+}
+}
+
+
 public static class NoDependencies {
 
 private NoDependencies() {