From 6cd67412ccd0a3df419205cc0281bbed59260bab Mon Sep 17 00:00:00 2001 From: Brian Clozel Date: Wed, 8 Mar 2023 16:23:32 +0100 Subject: [PATCH] Avoid lock contention in CaffeineCacheManager Prior to this commit, using a dynamic `CaffeineCacheManager` would rely on `ConcurrentHashMap#computeIfAbsent` for retrieving and creating cache instances as needed. It turns out that using this method concurrently can cause lock contention even when all known cache instances are instantiated. This commit avoids using this method if the cache instance already exists and avoid storing `null` entries in the map. This change reduces lock contention and the overall HashMap size in the non-dynamic case. Fixes gh-30066 --- .../cache/caffeine/CaffeineCacheManager.java | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/spring-context-support/src/main/java/org/springframework/cache/caffeine/CaffeineCacheManager.java b/spring-context-support/src/main/java/org/springframework/cache/caffeine/CaffeineCacheManager.java index d5b77cbc1bb..a09c12c9947 100644 --- a/spring-context-support/src/main/java/org/springframework/cache/caffeine/CaffeineCacheManager.java +++ b/spring-context-support/src/main/java/org/springframework/cache/caffeine/CaffeineCacheManager.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2020 the original author or authors. + * Copyright 2002-2023 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. @@ -51,6 +51,7 @@ import org.springframework.util.ObjectUtils; * @author Juergen Hoeller * @author Stephane Nicoll * @author Sam Brannen + * @author Brian Clozel * @since 4.3 * @see CaffeineCache */ @@ -188,8 +189,13 @@ public class CaffeineCacheManager implements CacheManager { @Override @Nullable public Cache getCache(String name) { - return this.cacheMap.computeIfAbsent(name, cacheName -> - this.dynamic ? createCaffeineCache(cacheName) : null); + if (this.dynamic) { + Cache cache = this.cacheMap.get(name); + return (cache != null) ? cache : this.cacheMap.computeIfAbsent(name, this::createCaffeineCache); + } + else { + return this.cacheMap.get(name); + } }