AbstractBeanFactory#markBeanAsCreated performance issue due to lock contention [SPR-9780]

[spring-projects-issues]

Nicko Cadell opened SPR-9780 and commented

We are calling getBean frequently (perhaps too often), but we have noticed that we get some lock contention where the bean name is added to the alreadyCreated set. This is a synchronized set created in the AbstractBeanFactory.

The call stack is typically something like this:

at java.util.Collections$SynchronizedCollection.add(Collections.java:1577)
at org.springframework.beans.factory.support.AbstractBeanFactory.markBeanAsCreated(AbstractBeanFactory.java:1363)
at org.springframework.beans.factory.support.AbstractBeanFactory.doGetBean(AbstractBeanFactory.java:271)
at org.springframework.beans.factory.support.AbstractBeanFactory.getBean(AbstractBeanFactory.java:190)
at org.springframework.beans.factory.support.CglibSubclassingInstantiationStrategy$CglibSubclassCreator$LookupOverrideMethodInterceptor.intercept(CglibSubclassingInstantiationStrategy.java:160)

When the markBeanAsCreated method is typically called the beanName already exists in the alreadyCreated set.

A couple of different options come to mind:

The first option would be to change the locking on the alreadyCreated set from a mutex to a reader/writer lock. Change the markBeanAsCreated method to acquire a read lock, then check if the set already contains the bean name, only if it does not contain it, upgrade the read lock to a write lock and add the bean name to the set. This could be slightly more work when a bean is created for the first time but allows for greater concurrency once the beans have been created.

The second option is that the factory is actually a DefaultListableBeanFactory and the configurationFrozen flag is set to TRUE. This means that the isBeanEligibleForMetadataCaching method is overridden to always return TRUE. I believe that the isBeanEligibleForMetadataCaching method in AbstractBeanFactory is the only place that cares about the contents of the alreadyCreated set. So it seems like a waste of time to populate the alreadyCreated set if the configurationFrozen flag is set to TRUE. So the DefaultListableBeanFactory should override the markBeanAsCreated method and do nothing if the configurationFrozen flag is set to TRUE.

---

Affects: 3.1.1

Issue Links:

• Non-singleton beans performance issue [SPR-9819] #14452 Non-singleton beans performance issue
• Further locking optimizations for the retrieval of non-singleton beans [SPR-12250] #16864 Further locking optimizations for the retrieval of non-singleton beans

[spring-projects-issues]

Juergen Hoeller commented

As of 3.2 GA, markBeanAsCreated uses a ConcurrentHashMap, hopefully addressing the lock contention reported here.

Juergen

[spring-projects-issues]

Bernhard Frauendienst commented

We're still experiencing lock contention with the ConcurrentHashMap solution under moderately high load, especially in combination with request scoped beans (which cause a lot of calls to getBean).

Increasing the concurrency level of the map would probably not help very much in our case, since only a very small set of keys is requested at high concurrency.

A possible solution would be to check for existence of the key before adding it, since reading does not cause the segment to be locked. This would make the marking non-atomic, but as far as I can tell this would be acceptable.

[spring-projects-issues]

Juergen Hoeller commented

I suppose the use of putIfAbsent instead of put / add would help? We'd have to keep alreadyCreated declared as a ConcurrentMap then, not using Collections.newSetFromMap as we do in the 4.x generation, but that's a minor inconvenience only.

FYI, we have issue #16864 for further performance improvements for the retrieval of non-singleton beans. I'll deal with your optimization there for 4.1.2 / 4.0.8 / 3.2.12.

Juergen