From 0a053cfccb4ab961f66a57f3fdaec5dd908f7fbe 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. See gh-30066 Fixes gh-30085 --- .../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); + } }