Avoid lock congestion in ConcurrentReferenceHashMap by chschu · Pull Request #36293 · spring-projects/spring-framework

After updating from Spring Framework v6.2.11 to v6.2.15, we observed heavy lock congestion even under moderate load. The following stacktrace shows the culprit:

"https-jsse-nio-31030-exec-13" - Thread t@166
   java.lang.Thread.State: WAITING
	at java.base@25.0.1/jdk.internal.misc.Unsafe.park(Native Method)
	- parking to wait for <3441435a> (a java.util.concurrent.locks.ReentrantLock$NonfairSync)
	at java.base@25.0.1/java.util.concurrent.locks.LockSupport.park(LockSupport.java:223)
	at java.base@25.0.1/java.util.concurrent.locks.AbstractQueuedSynchronizer.acquire(AbstractQueuedSynchronizer.java:790)
	at java.base@25.0.1/java.util.concurrent.locks.AbstractQueuedSynchronizer.acquire(AbstractQueuedSynchronizer.java:1030)
	at java.base@25.0.1/java.util.concurrent.locks.ReentrantLock$Sync.lock(ReentrantLock.java:154)
	at java.base@25.0.1/java.util.concurrent.locks.ReentrantLock.lock(ReentrantLock.java:323)
	at org.springframework.util.ConcurrentReferenceHashMap$Segment.doTask(ConcurrentReferenceHashMap.java:675)
	at org.springframework.util.ConcurrentReferenceHashMap.doTask(ConcurrentReferenceHashMap.java:570)
	at org.springframework.util.ConcurrentReferenceHashMap.computeIfAbsent(ConcurrentReferenceHashMap.java:389)
	at org.springframework.core.annotation.AnnotationTypeMappings.forAnnotationType(AnnotationTypeMappings.java:230)
	at org.springframework.core.annotation.AnnotationTypeMappings.forAnnotationType(AnnotationTypeMappings.java:206)
	at org.springframework.core.annotation.TypeMappedAnnotations$MergedAnnotationFinder.process(TypeMappedAnnotations.java:426)
	at org.springframework.core.annotation.TypeMappedAnnotations$MergedAnnotationFinder.doWithAnnotations(TypeMappedAnnotations.java:406)
	at org.springframework.core.annotation.TypeMappedAnnotations$MergedAnnotationFinder.doWithAnnotations(TypeMappedAnnotations.java:372)
	at org.springframework.core.annotation.AnnotationsScanner.processMethodAnnotations(AnnotationsScanner.java:395)
	at org.springframework.core.annotation.AnnotationsScanner.processMethodHierarchy(AnnotationsScanner.java:282)
	at org.springframework.core.annotation.AnnotationsScanner.processMethod(AnnotationsScanner.java:247)
	at org.springframework.core.annotation.AnnotationsScanner.process(AnnotationsScanner.java:95)
	at org.springframework.core.annotation.AnnotationsScanner.scan(AnnotationsScanner.java:82)
	at org.springframework.core.annotation.TypeMappedAnnotations.scan(TypeMappedAnnotations.java:248)
	at org.springframework.core.annotation.TypeMappedAnnotations.get(TypeMappedAnnotations.java:155)
	at org.springframework.core.annotation.AnnotatedElementUtils.findMergedAnnotation(AnnotatedElementUtils.java:648)
	at org.springframework.core.annotation.AnnotatedMethod.getMethodAnnotation(AnnotatedMethod.java:155)
	at org.springframework.core.annotation.AnnotatedMethod$AnnotatedMethodParameter.getMethodAnnotation(AnnotatedMethod.java:286)
	at org.springframework.web.method.annotation.ModelFactory.getNameForReturnValue(ModelFactory.java:271)
	at org.springframework.web.method.annotation.ModelFactory.invokeModelAttributeMethods(ModelFactory.java:153)
	at org.springframework.web.method.annotation.ModelFactory.initModel(ModelFactory.java:111)
	at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.invokeHandlerMethod(RequestMappingHandlerAdapter.java:975)
	at org.springframework.web.servlet.mvc.method.annotation.RequestMappingHandlerAdapter.handleInternal(RequestMappingHandlerAdapter.java:896)
	at org.springframework.web.servlet.mvc.method.AbstractHandlerMethodAdapter.handle(AbstractHandlerMethodAdapter.java:87)
	at org.springframework.web.servlet.DispatcherServlet.doDispatch(DispatcherServlet.java:1089)
	at org.springframework.web.servlet.DispatcherServlet.doService(DispatcherServlet.java:979)
	at org.springframework.web.servlet.FrameworkServlet.processRequest(FrameworkServlet.java:1014)
	at org.springframework.web.servlet.FrameworkServlet.doGet(FrameworkServlet.java:903)
	at jakarta.servlet.http.HttpServlet.service(HttpServlet.java:564)
	<redacted>

Most of the time, the call to ConcurrentReferenceHashMap.computeIfAbsent(...) is read-only and doesn't need the lock.

The implementations of computeIfAbsent(...) and computeIfPresent(...) have been added in v7.0.0 with commit 0552cdb and backported to v6.2.13 with commit 12dd758.

Please note that this issue is different from the one described in gh-35944, because it doesn't involve a call to AnnotationTypeMappings$Cache.get(...).

This PR adds lock-free checks to these methods to avoid locking in the predominant read-only case. It has been verified to solve the lock congestion issue in our load tests.

The modification drastically improves the performace of the read-only case, but adds some overhead (an entry lookup) in the modifying case. This tradeoff might not be feasible for all usages of ConcurrentReferenceHashMap. A similar issue has been identified (and improved, considering the inherent tradeoff) almost a decade ago in the JDK: https://bugs.openjdk.java.net/browse/JDK-8161372