Skip to content

Unnecessarily conservative locking in registerDependentBean is a bottleneck [SPR-8814] #13456

@spring-projects-issues

Description

@spring-projects-issues

Dennis Homann opened SPR-8814 and commented

In a highly concurrent application with a high rate of prototype bean instantiations, injection of (singleton) bean references into the prototype beans becomes a bottleneck due to unnecessary synchonization in DefaultSingletonBeanRegistry#registerDependentBean.

registerDependentBean updates the two ConcurrentHashMaps dependentBeanMap and dependenciesForBeanMap. For both maps, the method ensures that it maps the singleton bean name to a LinkedHashSet (will be added, if not present) and that this set contains the dependent prototype bean name. A synchronized block guards both map manipulations, leading to contention and a severe performance bottleneck under high load.

Since both maps are already ConcurrentHashMaps, the map manipulation may be refactored to be non-blocking. The following suggestion for dependentBeanMap applies in the same way to dependenciesForBeanMap. It may also be applied to some other ConcurrentHashMap manipulations in DefaultSingletonBeanRegistry, however those are not as performance-critical.

// change type of field from Map to ConcurrentMap
private final ConcurrentMap<String, Set<String>> dependentBeanMap = new ConcurrentHashMap<String, Set<String>>();

public void registerDependentBean(String beanName, String dependentBeanName) {
  String canonicalName = canonicalName(beanName);
  getMappedSet(dependentBeanMap, canonicalName).add(dependentBeanName);
  getMappedSet(dependenciesForBeanMap, dependentBeanName).add(canonicalName);
}

private Set<String> getMappedSet(ConcurrentMap<String, Set<String>> map, String name) {
  // a simple putIfAbsent would be enough to be functionally correct,
  // but calling get first will avoid creation of the set in the common case that a mapping already exists

  Set<String> set = map.get(name);
  return set != null ? set : map.putIfAbsent(name, Collections.synchronizedSet(new LinkedHashSet<String>(8)));
}

After the change the set will be synchronized, but contention on the contained sets will be much lower than on a global map lock.


Affects: 3.0.5

Issue Links:

3 votes, 6 watchers

Metadata

Metadata

Assignees

Labels

in: coreIssues in core modules (aop, beans, core, context, expression)type: enhancementA general enhancement

Type

No type

Projects

No projects

Milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions