From b346a7bb7d2b85cb8698b2dae4db3fa4fd291506 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Tue, 9 Jun 2026 16:46:10 +0000 Subject: [PATCH 1/6] Replace ConcurrentLRUCache with Caffeine Cache This change removes the custom ConcurrentLRUCache implementation and replaces its usages in IndexSchema and TemplateUpdateProcessorFactory with Caffeine's Cache. Additionally: - The org.apache.solr.common.util.Cache interface and MapBackedCache have been removed. - JavaBinCodec.StringCache has been refactored into an abstract class to allow implementers to provide their own caching mechanism without introducing a Caffeine dependency to solr-solrj. - ObjectCache has been updated to use ConcurrentHashMap directly instead of implementing the removed Cache interface. - Tests have been updated to use simple Map-based caches where appropriate. Co-authored-by: dsmiley <377295+dsmiley@users.noreply.github.com> --- .../org/apache/solr/schema/IndexSchema.java | 10 +- .../TemplateUpdateProcessorFactory.java | 17 +- .../apache/solr/util/ConcurrentLRUCache.java | 974 ------------------ .../org/apache/solr/common/util/Cache.java | 44 - .../apache/solr/common/util/JavaBinCodec.java | 16 +- .../solr/common/util/MapBackedCache.java | 60 -- .../apache/solr/common/util/ObjectCache.java | 25 +- .../solr/common/util/TestJavaBinCodec.java | 77 +- 8 files changed, 80 insertions(+), 1143 deletions(-) delete mode 100644 solr/core/src/java/org/apache/solr/util/ConcurrentLRUCache.java delete mode 100644 solr/solrj/src/java/org/apache/solr/common/util/Cache.java delete mode 100644 solr/solrj/src/java/org/apache/solr/common/util/MapBackedCache.java diff --git a/solr/core/src/java/org/apache/solr/schema/IndexSchema.java b/solr/core/src/java/org/apache/solr/schema/IndexSchema.java index 3571dd2a07d8..425814962683 100644 --- a/solr/core/src/java/org/apache/solr/schema/IndexSchema.java +++ b/solr/core/src/java/org/apache/solr/schema/IndexSchema.java @@ -55,6 +55,8 @@ import org.apache.lucene.search.similarities.Similarity; import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.ResourceLoaderAware; +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; import org.apache.lucene.util.Version; import org.apache.solr.analysis.TokenizerChain; import org.apache.solr.common.ConfigNode; @@ -68,7 +70,6 @@ import org.apache.solr.common.params.MapSolrParams; import org.apache.solr.common.params.ModifiableSolrParams; import org.apache.solr.common.params.SolrParams; -import org.apache.solr.common.util.Cache; import org.apache.solr.common.util.NamedList; import org.apache.solr.common.util.Pair; import org.apache.solr.common.util.SimpleOrderedMap; @@ -80,7 +81,6 @@ import org.apache.solr.response.SolrQueryResponse; import org.apache.solr.search.similarities.SchemaSimilarityFactory; import org.apache.solr.uninverting.UninvertingReader; -import org.apache.solr.util.ConcurrentLRUCache; import org.apache.solr.util.PayloadUtils; import org.apache.solr.util.plugin.SolrCoreAware; import org.slf4j.Logger; @@ -145,8 +145,8 @@ public DynamicField[] getDynamicFields() { private static final Set FIELDTYPE_KEYS = Set.of("fieldtype", "fieldType"); private static final Set FIELD_KEYS = Set.of("dynamicField", "field"); - protected Cache dynamicFieldCache = - new ConcurrentLRUCache<>(10000, 8000, 9000, 100, false, false, null); + protected final Cache dynamicFieldCache = + Caffeine.newBuilder().maximumSize(10000).build(); private Analyzer indexAnalyzer; private Analyzer queryAnalyzer; @@ -1390,7 +1390,7 @@ public boolean isDynamicField(String fieldName) { public SchemaField getFieldOrNull(String fieldName) { SchemaField f = fields.get(fieldName); if (f != null) return f; - f = dynamicFieldCache.get(fieldName); + f = dynamicFieldCache.getIfPresent(fieldName); if (f != null) return f; for (DynamicField df : dynamicFields) { diff --git a/solr/core/src/java/org/apache/solr/update/processor/TemplateUpdateProcessorFactory.java b/solr/core/src/java/org/apache/solr/update/processor/TemplateUpdateProcessorFactory.java index f37e4a4c419e..c2c3f8b2d44a 100644 --- a/solr/core/src/java/org/apache/solr/update/processor/TemplateUpdateProcessorFactory.java +++ b/solr/core/src/java/org/apache/solr/update/processor/TemplateUpdateProcessorFactory.java @@ -21,13 +21,13 @@ import java.util.List; import java.util.function.Function; import java.util.regex.Matcher; +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; import java.util.regex.Pattern; import org.apache.solr.common.SolrInputDocument; -import org.apache.solr.common.util.Cache; import org.apache.solr.request.SolrQueryRequest; import org.apache.solr.response.SolrQueryResponse; import org.apache.solr.update.AddUpdateCommand; -import org.apache.solr.util.ConcurrentLRUCache; /** * Adds new fields to documents based on a template pattern specified via Template.field request @@ -40,8 +40,7 @@ * @since 6.3.0 */ public class TemplateUpdateProcessorFactory extends SimpleUpdateProcessorFactory { - private Cache templateCache = - new ConcurrentLRUCache<>(1000, 800, 900, 10, false, false, null); + private Cache templateCache = Caffeine.newBuilder().maximumSize(1000).build(); public static final String NAME = "template"; @Override @@ -77,9 +76,8 @@ protected String getMyName() { return NAME; } - public static Resolved getResolved( - String template, Cache cache, Pattern pattern) { - Resolved r = cache == null ? null : cache.get(template); + public static Resolved getResolved(String template, Cache cache, Pattern pattern) { + Resolved r = cache == null ? null : cache.getIfPresent(template); if (r == null) { r = new Resolved(); Matcher m = pattern.matcher(template); @@ -105,10 +103,7 @@ public static List getVariables( } public static String replaceTokens( - String template, - Cache cache, - Function fun, - Pattern pattern) { + String template, Cache cache, Function fun, Pattern pattern) { if (template == null) { return null; } diff --git a/solr/core/src/java/org/apache/solr/util/ConcurrentLRUCache.java b/solr/core/src/java/org/apache/solr/util/ConcurrentLRUCache.java deleted file mode 100644 index a32b61580009..000000000000 --- a/solr/core/src/java/org/apache/solr/util/ConcurrentLRUCache.java +++ /dev/null @@ -1,974 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.solr.util; - -import static org.apache.lucene.util.RamUsageEstimator.HASHTABLE_RAM_BYTES_PER_ENTRY; -import static org.apache.lucene.util.RamUsageEstimator.QUERY_DEFAULT_RAM_BYTES_USED; - -import java.lang.ref.WeakReference; -import java.lang.reflect.Array; -import java.util.ArrayList; -import java.util.Arrays; -import java.util.Collection; -import java.util.Collections; -import java.util.Iterator; -import java.util.LinkedHashMap; -import java.util.List; -import java.util.Map; -import java.util.TreeSet; -import java.util.concurrent.ConcurrentHashMap; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicBoolean; -import java.util.concurrent.atomic.AtomicLong; -import java.util.concurrent.atomic.LongAdder; -import java.util.concurrent.locks.ReentrantLock; -import java.util.function.Function; -import org.apache.lucene.util.Accountable; -import org.apache.lucene.util.PriorityQueue; -import org.apache.lucene.util.RamUsageEstimator; -import org.apache.solr.common.util.Cache; -import org.apache.solr.common.util.TimeSource; - -/** - * A LRU cache implementation based upon ConcurrentHashMap and other techniques to reduce contention - * and synchronization overhead to utilize multiple CPU cores more effectively. - * - *

Note that the implementation does not follow a true LRU (least-recently-used) eviction - * strategy. Instead it strives to remove least recently used items but when the initial cleanup - * does not remove enough items to reach the 'acceptableWaterMark' limit, it can remove more items - * forcefully regardless of access order. - * - * @since solr 1.4 - */ -public class ConcurrentLRUCache implements Cache, Accountable { - - private static final long BASE_RAM_BYTES_USED = - RamUsageEstimator.shallowSizeOfInstance(ConcurrentLRUCache.class) - + new Stats().ramBytesUsed() - + RamUsageEstimator.primitiveSizes.get(long.class) - + RamUsageEstimator.shallowSizeOfInstance(ConcurrentHashMap.class); - - private final ConcurrentHashMap> map; - private int upperWaterMark, lowerWaterMark; - private final ReentrantLock markAndSweepLock = new ReentrantLock(true); - private boolean isCleaning = false; // not volatile... piggybacked on other volatile vars - private boolean newThreadForCleanup; - private volatile boolean islive = true; - private final Stats stats = new Stats(); - private int acceptableWaterMark; - private long oldestEntry = 0; // not volatile, only accessed in the cleaning method - private final TimeSource timeSource = TimeSource.NANO_TIME; - private final AtomicLong oldestEntryNs = new AtomicLong(0); - private long maxIdleTimeNs; - private final EvictionListener evictionListener; - private CleanupThread cleanupThread; - private boolean runCleanupThread; - - private long ramLowerWatermark, ramUpperWatermark; - private final LongAdder ramBytes = new LongAdder(); - - public ConcurrentLRUCache( - long ramLowerWatermark, - long ramUpperWatermark, - boolean runCleanupThread, - EvictionListener evictionListener) { - this(ramLowerWatermark, ramUpperWatermark, runCleanupThread, evictionListener, -1); - } - - public ConcurrentLRUCache( - long ramLowerWatermark, - long ramUpperWatermark, - boolean runCleanupThread, - EvictionListener evictionListener, - int maxIdleTimeSec) { - this.ramLowerWatermark = ramLowerWatermark; - this.ramUpperWatermark = ramUpperWatermark; - - this.evictionListener = evictionListener; - this.map = new ConcurrentHashMap<>(); - this.newThreadForCleanup = false; - - this.acceptableWaterMark = -1; - this.lowerWaterMark = Integer.MIN_VALUE; - this.upperWaterMark = Integer.MAX_VALUE; - - setMaxIdleTime(maxIdleTimeSec); - setRunCleanupThread(runCleanupThread); - } - - public ConcurrentLRUCache( - int upperWaterMark, - final int lowerWaterMark, - int acceptableWaterMark, - int initialSize, - boolean runCleanupThread, - boolean runNewThreadForCleanup, - EvictionListener evictionListener) { - this( - upperWaterMark, - lowerWaterMark, - acceptableWaterMark, - initialSize, - runCleanupThread, - runNewThreadForCleanup, - evictionListener, - -1); - } - - public ConcurrentLRUCache( - int upperWaterMark, - final int lowerWaterMark, - int acceptableWaterMark, - int initialSize, - boolean runCleanupThread, - boolean runNewThreadForCleanup, - EvictionListener evictionListener, - int maxIdleTimeSec) { - if (upperWaterMark < 1) throw new IllegalArgumentException("upperWaterMark must be > 0"); - if (lowerWaterMark >= upperWaterMark) - throw new IllegalArgumentException("lowerWaterMark must be < upperWaterMark"); - map = new ConcurrentHashMap<>(initialSize); - newThreadForCleanup = runNewThreadForCleanup; - this.upperWaterMark = upperWaterMark; - this.lowerWaterMark = lowerWaterMark; - this.acceptableWaterMark = acceptableWaterMark; - this.evictionListener = evictionListener; - this.ramLowerWatermark = Long.MIN_VALUE; - this.ramUpperWatermark = Long.MAX_VALUE; - setMaxIdleTime(maxIdleTimeSec); - setRunCleanupThread(runCleanupThread); - } - - public ConcurrentLRUCache(int size, int lowerWaterMark) { - this( - size, - lowerWaterMark, - (int) Math.floor((lowerWaterMark + size) / 2.0), - (int) Math.ceil(0.75 * size), - false, - false, - null, - -1); - } - - public void setAlive(boolean live) { - islive = live; - } - - public void setUpperWaterMark(int upperWaterMark) { - if (upperWaterMark < 1) throw new IllegalArgumentException("upperWaterMark must be >= 1"); - this.upperWaterMark = upperWaterMark; - } - - public void setLowerWaterMark(int lowerWaterMark) { - this.lowerWaterMark = lowerWaterMark; - } - - public void setAcceptableWaterMark(int acceptableWaterMark) { - this.acceptableWaterMark = acceptableWaterMark; - } - - public void setRamUpperWatermark(long ramUpperWatermark) { - if (ramUpperWatermark < 1) { - throw new IllegalArgumentException("ramUpperWaterMark must be >= 1"); - } - this.ramUpperWatermark = ramUpperWatermark; - } - - public void setRamLowerWatermark(long ramLowerWatermark) { - if (ramLowerWatermark < 0) { - throw new IllegalArgumentException("ramLowerWaterMark must be >= 0"); - } - this.ramLowerWatermark = ramLowerWatermark; - } - - public void setMaxIdleTime(int maxIdleTime) { - long oldMaxIdleTimeNs = maxIdleTimeNs; - maxIdleTimeNs = - maxIdleTime > 0 - ? TimeUnit.NANOSECONDS.convert(maxIdleTime, TimeUnit.SECONDS) - : Long.MAX_VALUE; - if (cleanupThread != null && maxIdleTimeNs < oldMaxIdleTimeNs) { - cleanupThread.wakeThread(); - } - } - - public synchronized void setRunCleanupThread(boolean runCleanupThread) { - this.runCleanupThread = runCleanupThread; - if (this.runCleanupThread) { - if (cleanupThread == null) { - cleanupThread = new CleanupThread(this); - cleanupThread.start(); - } - } else { - if (cleanupThread != null) { - cleanupThread.stopThread(); - cleanupThread = null; - } - } - } - - @Override - public V get(K key) { - CacheEntry e = map.get(key); - if (e == null) { - if (islive) stats.missCounter.increment(); - return null; - } - if (islive) e.lastAccessed = stats.accessCounter.incrementAndGet(); - return e.value; - } - - @Override - public V remove(K key) { - CacheEntry cacheEntry = map.remove(key); - if (cacheEntry != null) { - stats.size.decrement(); - ramBytes.add(-cacheEntry.ramBytesUsed() - HASHTABLE_RAM_BYTES_PER_ENTRY); - return cacheEntry.value; - } - return null; - } - - @Override - public V computeIfAbsent(K key, Function mappingFunction) { - // prescreen access first - V val = get(key); - if (val != null) { - return val; - } - AtomicBoolean newEntry = new AtomicBoolean(); - CacheEntry entry = - map.computeIfAbsent( - key, - k -> { - V value = mappingFunction.apply(key); - // preserve the semantics of computeIfAbsent - if (value == null) { - return null; - } - CacheEntry e = - new CacheEntry<>( - key, - value, - timeSource.getEpochTimeNs(), - stats.accessCounter.incrementAndGet()); - oldestEntryNs.updateAndGet(x -> x > e.createTime || x == 0 ? e.createTime : x); - stats.size.increment(); - ramBytes.add( - e.ramBytesUsed() + HASHTABLE_RAM_BYTES_PER_ENTRY); // added key + value + entry - if (islive) { - stats.putCounter.increment(); - } else { - stats.nonLivePutCounter.increment(); - } - newEntry.set(true); - return e; - }); - if (newEntry.get()) { - maybeMarkAndSweep(); - } else { - if (islive && entry != null) { - entry.lastAccessed = stats.accessCounter.incrementAndGet(); - } - } - return entry != null ? entry.value : null; - } - - @Override - public V put(K key, V val) { - if (val == null) return null; - CacheEntry e = - new CacheEntry<>( - key, val, timeSource.getEpochTimeNs(), stats.accessCounter.incrementAndGet()); - return putCacheEntry(e); - } - - /** - * Visible for testing to create synthetic cache entries. - * - * @lucene.internal - */ - public V putCacheEntry(CacheEntry e) { - // initialize oldestEntryNs - oldestEntryNs.updateAndGet(x -> x > e.createTime || x == 0 ? e.createTime : x); - CacheEntry oldCacheEntry = map.put(e.key, e); - if (oldCacheEntry == null) { - stats.size.increment(); - ramBytes.add(e.ramBytesUsed() + HASHTABLE_RAM_BYTES_PER_ENTRY); // added key + value + entry - } else { - ramBytes.add(-oldCacheEntry.ramBytesUsed()); - ramBytes.add(e.ramBytesUsed()); - } - if (islive) { - stats.putCounter.increment(); - } else { - stats.nonLivePutCounter.increment(); - } - maybeMarkAndSweep(); - return oldCacheEntry == null ? null : oldCacheEntry.value; - } - - private void maybeMarkAndSweep() { - // Check if we need to clear out old entries from the cache. - // isCleaning variable is checked instead of markAndSweepLock.isLocked() - // for performance because every put invocation will check until - // the size is back to an acceptable level. - // - // There is a race between the check and the call to markAndSweep, but - // it's unimportant because markAndSweep actually acquires the lock or returns if it can't. - // - // Thread safety note: isCleaning read is piggybacked (comes after) other volatile reads - // in this method. - long idleCutoff = timeSource.getEpochTimeNs() - maxIdleTimeNs; - int currentSize = stats.size.intValue(); - if ((currentSize > upperWaterMark - || ramBytes.sum() > ramUpperWatermark - || oldestEntryNs.get() < idleCutoff) - && !isCleaning) { - if (newThreadForCleanup) { - new Thread(this::markAndSweep, "CacheCleanupThread").start(); - } else if (cleanupThread != null) { - cleanupThread.wakeThread(); - } else { - markAndSweep(); - } - } - } - - /** - * Removes items from the cache to bring the size down to an acceptable value. - * - *

Visible for unit testing. - * - * @lucene.internal - */ - public void markAndSweep() { - // if we want to keep at least 1000 entries, then timestamps of - // current through current-1000 are guaranteed not to be the oldest (but that does - // not mean there are 1000 entries in that group... it's actually anywhere between - // 1 and 1000). - // Also, if we want to remove 500 entries, then - // oldestEntry through oldestEntry+500 are guaranteed to be - // removed (however many there are there). - - if (!markAndSweepLock.tryLock()) return; - try { - if (maxIdleTimeNs != Long.MAX_VALUE) { - long idleCutoff = timeSource.getEpochTimeNs() - maxIdleTimeNs; - if (oldestEntryNs.get() < idleCutoff) { - markAndSweepByIdleTime(); - } - } - if (upperWaterMark < size()) { - markAndSweepByCacheSize(); - } else if (ramUpperWatermark < ramBytesUsed()) { - markAndSweepByRamSize(); - } else if (upperWaterMark == Integer.MAX_VALUE && ramUpperWatermark == Long.MAX_VALUE) { - // should never happen - throw new AssertionError( - "ConcurrentLRUCache initialized with neither size limits nor ram limits"); - } - } finally { - isCleaning = false; // set before markAndSweep.unlock() for visibility - markAndSweepLock.unlock(); - } - } - - /* - Must be called after acquiring markAndSweepLock - */ - private void markAndSweepByIdleTime() { - assert markAndSweepLock.isHeldByCurrentThread() : "markAndSweepLock held by another thread"; - Iterator>> iterator = map.entrySet().iterator(); - long idleCutoff = timeSource.getEpochTimeNs() - maxIdleTimeNs; - long currentOldestEntry = Long.MAX_VALUE; - while (iterator.hasNext()) { - Map.Entry> entry = iterator.next(); - if (entry.getValue().createTime < idleCutoff) { - iterator.remove(); - stats.evictionIdleCounter.increment(); - postRemoveEntry(entry.getValue()); - } else { - if (entry.getValue().createTime < currentOldestEntry) { - currentOldestEntry = entry.getValue().createTime; - } - } - } - if (currentOldestEntry != Long.MAX_VALUE) { - oldestEntryNs.set(currentOldestEntry); - } - } - - /* - Must be called after acquiring markAndSweepLock - */ - private void markAndSweepByRamSize() { - assert markAndSweepLock.isHeldByCurrentThread() : "markAndSweepLock held by another thread"; - List> entriesInAccessOrder = new ArrayList<>(map.size()); - map.forEach( - (o, kvCacheEntry) -> { - // important because we want to avoid volatile read during comparisons - kvCacheEntry.lastAccessedCopy = kvCacheEntry.lastAccessed; - entriesInAccessOrder.add(kvCacheEntry); - }); - - Collections.sort(entriesInAccessOrder); // newer access is smaller, older access is bigger - - // iterate in oldest to newest order - for (int i = entriesInAccessOrder.size() - 1; i >= 0; i--) { - CacheEntry kvCacheEntry = entriesInAccessOrder.get(i); - evictEntry(kvCacheEntry.key); - if (ramBytes.sum() <= ramLowerWatermark) { - break; // we are done! - } - } - } - - /* - * Removes items from the cache to bring the size down - * to an acceptable value ('acceptableWaterMark'). - *

- * It is done in two stages. In the first stage, least recently used items are evicted. - * If, after the first stage, the cache size is still greater than 'acceptableSize' - * config parameter, the second stage takes over. - *

- *

The second stage is more intensive and tries to bring down the cache size - * to the 'lowerWaterMark' config parameter.

- * Must be called after acquiring markAndSweepLock - */ - private void markAndSweepByCacheSize() { - assert markAndSweepLock.isHeldByCurrentThread() : "markAndSweepLock held by another thread"; - long oldestEntry = this.oldestEntry; - isCleaning = true; - this.oldestEntry = oldestEntry; // volatile write to make isCleaning visible - - long timeCurrent = stats.accessCounter.longValue(); - int sz = stats.size.intValue(); - - int numRemoved = 0; - int numKept = 0; - long newestEntry = timeCurrent; - long newNewestEntry = -1; - long newOldestEntry = Long.MAX_VALUE; - - int wantToKeep = lowerWaterMark; - int wantToRemove = sz - lowerWaterMark; - - @SuppressWarnings("unchecked") - CacheEntry[] eset = (CacheEntry[]) Array.newInstance(CacheEntry.class, sz); - int eSize = 0; - - // System.out.println("newestEntry="+newestEntry + " oldestEntry="+oldestEntry); - // System.out.println("items removed:" + numRemoved + " numKept=" + numKept + " esetSz="+ eSize - // + " sz-numRemoved=" + (sz-numRemoved)); - - for (CacheEntry ce : map.values()) { - // set lastAccessedCopy to avoid more volatile reads - ce.lastAccessedCopy = ce.lastAccessed; - long thisEntry = ce.lastAccessedCopy; - - // since the wantToKeep group is likely to be bigger than wantToRemove, check it first - if (thisEntry > newestEntry - wantToKeep) { - // this entry is guaranteed not to be in the bottom - // group, so do nothing. - numKept++; - newOldestEntry = Math.min(thisEntry, newOldestEntry); - } else if (thisEntry < oldestEntry + wantToRemove) { // entry in bottom group? - // this entry is guaranteed to be in the bottom group - // so immediately remove it from the map. - evictEntry(ce.key); - numRemoved++; - } else { - // This entry *could* be in the bottom group. - // Collect these entries to avoid another full pass... this is wasted - // effort if enough entries are normally removed in this first pass. - // An alternate impl could make a full second pass. - if (eSize < eset.length - 1) { - eset[eSize++] = ce; - newNewestEntry = Math.max(thisEntry, newNewestEntry); - newOldestEntry = Math.min(thisEntry, newOldestEntry); - } - } - } - - // System.out.println("items removed:" + numRemoved + " numKept=" + numKept + " esetSz="+ eSize - // + " sz-numRemoved=" + (sz-numRemoved)); - // TODO: allow this to be customized in the constructor? - int numPasses = 1; // maximum number of linear passes over the data - - // if we didn't remove enough entries, then make more passes - // over the values we collected, with updated min and max values. - while (sz - numRemoved > acceptableWaterMark && --numPasses >= 0) { - - oldestEntry = newOldestEntry == Long.MAX_VALUE ? oldestEntry : newOldestEntry; - newOldestEntry = Long.MAX_VALUE; - newestEntry = newNewestEntry; - newNewestEntry = -1; - wantToKeep = lowerWaterMark - numKept; - wantToRemove = sz - lowerWaterMark - numRemoved; - - // iterate backward to make it easy to remove items. - for (int i = eSize - 1; i >= 0; i--) { - CacheEntry ce = eset[i]; - long thisEntry = ce.lastAccessedCopy; - - if (thisEntry > newestEntry - wantToKeep) { - // this entry is guaranteed not to be in the bottom - // group, so do nothing but remove it from the eset. - numKept++; - // remove the entry by moving the last element to its position - eset[i] = eset[eSize - 1]; - eSize--; - - newOldestEntry = Math.min(thisEntry, newOldestEntry); - - } else if (thisEntry < oldestEntry + wantToRemove) { // entry in bottom group? - - // this entry is guaranteed to be in the bottom group - // so immediately remove it from the map. - evictEntry(ce.key); - numRemoved++; - - // remove the entry by moving the last element to its position - eset[i] = eset[eSize - 1]; - eSize--; - } else { - // This entry *could* be in the bottom group, so keep it in the eset, - // and update the stats. - newNewestEntry = Math.max(thisEntry, newNewestEntry); - newOldestEntry = Math.min(thisEntry, newOldestEntry); - } - } - // System.out.println("items removed:" + numRemoved + " numKept=" + numKept + " esetSz="+ - // eSize + " sz-numRemoved=" + (sz-numRemoved)); - } - - // if we still didn't remove enough entries, then make another pass while - // inserting into a priority queue - if (sz - numRemoved > acceptableWaterMark) { - - oldestEntry = newOldestEntry == Long.MAX_VALUE ? oldestEntry : newOldestEntry; - newOldestEntry = Long.MAX_VALUE; - newestEntry = newNewestEntry; - newNewestEntry = -1; - wantToKeep = lowerWaterMark - numKept; - wantToRemove = sz - lowerWaterMark - numRemoved; - - PQueue queue = new PQueue<>(wantToRemove); - - for (int i = eSize - 1; i >= 0; i--) { - CacheEntry ce = eset[i]; - long thisEntry = ce.lastAccessedCopy; - - if (thisEntry > newestEntry - wantToKeep) { - // this entry is guaranteed not to be in the bottom - // group, so do nothing but remove it from the eset. - numKept++; - // removal not necessary on last pass. - // eset[i] = eset[eSize-1]; - // eSize--; - - newOldestEntry = Math.min(thisEntry, newOldestEntry); - - } else if (thisEntry < oldestEntry + wantToRemove) { // entry in bottom group? - // this entry is guaranteed to be in the bottom group - // so immediately remove it. - evictEntry(ce.key); - numRemoved++; - - // removal not necessary on last pass. - // eset[i] = eset[eSize-1]; - // eSize--; - } else { - // This entry *could* be in the bottom group. - // add it to the priority queue - - // everything in the priority queue will be removed, so keep track of - // the lowest value that ever comes back out of the queue. - - // first reduce the size of the priority queue to account for - // the number of items we have already removed while executing - // this loop so far. - queue.myMaxSize = sz - lowerWaterMark - numRemoved; - while (queue.size() > queue.myMaxSize && queue.size() > 0) { - CacheEntry otherEntry = queue.pop(); - newOldestEntry = Math.min(otherEntry.lastAccessedCopy, newOldestEntry); - } - if (queue.myMaxSize <= 0) break; - - CacheEntry o = queue.myInsertWithOverflow(ce); - if (o != null) { - newOldestEntry = Math.min(o.lastAccessedCopy, newOldestEntry); - } - } - } - - // Now delete everything in the priority queue. - // avoid using pop() since order doesn't matter anymore - for (CacheEntry ce : queue.getValues()) { - if (ce == null) continue; - evictEntry(ce.key); - numRemoved++; - } - - // System.out.println("items removed:" + numRemoved + " numKept=" + numKept + " - // initialQueueSize="+ wantToRemove + " finalQueueSize=" + queue.size() + " sz-numRemoved=" + - // (sz-numRemoved)); - } - - oldestEntry = newOldestEntry == Long.MAX_VALUE ? oldestEntry : newOldestEntry; - this.oldestEntry = oldestEntry; - } - - private static class PQueue extends PriorityQueue> { - int myMaxSize; - final Object[] heap; - - PQueue(int maxSz) { - super(maxSz); - heap = getHeapArray(); - myMaxSize = maxSz; - } - - @SuppressWarnings("unchecked") - Iterable> getValues() { - return (Iterable) Collections.unmodifiableCollection(Arrays.asList(heap)); - } - - @Override - protected boolean lessThan(CacheEntry a, CacheEntry b) { - // reverse the parameter order so that the queue keeps the oldest items - return b.lastAccessedCopy < a.lastAccessedCopy; - } - - // necessary because maxSize is private in base class - @SuppressWarnings("unchecked") - public CacheEntry myInsertWithOverflow(CacheEntry element) { - if (size() < myMaxSize) { - add(element); - return null; - } else if (size() > 0 && !lessThan(element, (CacheEntry) heap[1])) { - CacheEntry ret = (CacheEntry) heap[1]; - heap[1] = element; - updateTop(); - return ret; - } else { - return element; - } - } - } - - private void evictEntry(K key) { - CacheEntry o = map.remove(key); - postRemoveEntry(o); - } - - private void postRemoveEntry(CacheEntry o) { - if (o == null) return; - ramBytes.add(-(o.ramBytesUsed() + HASHTABLE_RAM_BYTES_PER_ENTRY)); - stats.size.decrement(); - stats.evictionCounter.increment(); - if (evictionListener != null) evictionListener.evictedEntry(o.key, o.value); - } - - /** - * Returns 'n' number of oldest accessed entries present in this cache. - * - *

This uses a TreeSet to collect the 'n' oldest items ordered by ascending last access time - * and returns a LinkedHashMap containing 'n' or less than 'n' entries. - * - * @param n the number of oldest items needed - * @return a LinkedHashMap containing 'n' or less than 'n' entries - */ - public Map getOldestAccessedItems(int n) { - Map result = new LinkedHashMap<>(); - if (n <= 0) return result; - TreeSet> tree = new TreeSet<>(); - markAndSweepLock.lock(); - try { - for (Map.Entry> entry : map.entrySet()) { - CacheEntry ce = entry.getValue(); - ce.lastAccessedCopy = ce.lastAccessed; - if (tree.size() < n) { - tree.add(ce); - } else { - if (ce.lastAccessedCopy < tree.first().lastAccessedCopy) { - tree.remove(tree.first()); - tree.add(ce); - } - } - } - } finally { - markAndSweepLock.unlock(); - } - for (CacheEntry e : tree) { - result.put(e.key, e.value); - } - return result; - } - - public Map getLatestAccessedItems(int n) { - Map result = new LinkedHashMap<>(); - if (n <= 0) return result; - TreeSet> tree = new TreeSet<>(); - // we need to grab the lock since we are changing lastAccessedCopy - markAndSweepLock.lock(); - try { - for (Map.Entry> entry : map.entrySet()) { - CacheEntry ce = entry.getValue(); - ce.lastAccessedCopy = ce.lastAccessed; - if (tree.size() < n) { - tree.add(ce); - } else { - if (ce.lastAccessedCopy > tree.last().lastAccessedCopy) { - tree.remove(tree.last()); - tree.add(ce); - } - } - } - } finally { - markAndSweepLock.unlock(); - } - for (CacheEntry e : tree) { - result.put(e.key, e.value); - } - return result; - } - - public int size() { - return stats.size.intValue(); - } - - @Override - public void clear() { - map.clear(); - ramBytes.reset(); - } - - public Map> getMap() { - return map; - } - - public static class CacheEntry implements Comparable>, Accountable { - public static long BASE_RAM_BYTES_USED = RamUsageEstimator.shallowSizeOf(CacheEntry.class); - - final K key; - final V value; - final long createTime; - final long ramBytesUsed; // cache - volatile long lastAccessed = 0; - long lastAccessedCopy = 0; - - public CacheEntry(K key, V value, long createTime, long lastAccessed) { - this.key = key; - this.value = value; - this.createTime = createTime; - this.lastAccessed = lastAccessed; - this.ramBytesUsed = - BASE_RAM_BYTES_USED - + RamUsageEstimator.sizeOfObject(key, QUERY_DEFAULT_RAM_BYTES_USED) - + RamUsageEstimator.sizeOfObject(value, QUERY_DEFAULT_RAM_BYTES_USED); - } - - public void setLastAccessed(long lastAccessed) { - this.lastAccessed = lastAccessed; - } - - @Override - public int compareTo(CacheEntry that) { - if (this.lastAccessedCopy == that.lastAccessedCopy) return 0; - return this.lastAccessedCopy < that.lastAccessedCopy ? 1 : -1; - } - - @Override - public int hashCode() { - return value.hashCode(); - } - - @Override - public boolean equals(Object obj) { - return value.equals(obj); - } - - @Override - public String toString() { - return "key: " + key + " value: " + value + " lastAccessed:" + lastAccessed; - } - - @Override - public long ramBytesUsed() { - return ramBytesUsed; - } - - @Override - public Collection getChildResources() { - return List.of(); - } - } - - private boolean isDestroyed = false; - - public void destroy() { - try { - if (cleanupThread != null) { - cleanupThread.stopThread(); - } - } finally { - isDestroyed = true; - } - } - - public Stats getStats() { - return stats; - } - - public static class Stats implements Accountable { - private static final long RAM_BYTES_USED = - // accounts for field refs - RamUsageEstimator.shallowSizeOfInstance(Stats.class) - + - // LongAdder - 6 - * (RamUsageEstimator.NUM_BYTES_ARRAY_HEADER - + RamUsageEstimator.primitiveSizes.get(long.class) - + 2L - * (RamUsageEstimator.NUM_BYTES_OBJECT_REF - + RamUsageEstimator.primitiveSizes.get(long.class))) - + - // AtomicLong - RamUsageEstimator.primitiveSizes.get(long.class); - - private final AtomicLong accessCounter = new AtomicLong(0); - private final LongAdder putCounter = new LongAdder(); - private final LongAdder nonLivePutCounter = new LongAdder(); - private final LongAdder missCounter = new LongAdder(); - private final LongAdder size = new LongAdder(); - private LongAdder evictionCounter = new LongAdder(); - private LongAdder evictionIdleCounter = new LongAdder(); - - public long getCumulativeLookups() { - return (accessCounter.longValue() - putCounter.longValue() - nonLivePutCounter.longValue()) - + missCounter.longValue(); - } - - public long getCumulativeHits() { - return accessCounter.longValue() - putCounter.longValue() - nonLivePutCounter.longValue(); - } - - public long getCumulativePuts() { - return putCounter.longValue(); - } - - public long getCumulativeEvictions() { - return evictionCounter.longValue(); - } - - public long getCumulativeIdleEvictions() { - return evictionIdleCounter.longValue(); - } - - public int getCurrentSize() { - return size.intValue(); - } - - public long getCumulativeNonLivePuts() { - return nonLivePutCounter.longValue(); - } - - public long getCumulativeMisses() { - return missCounter.longValue(); - } - - public void add(Stats other) { - accessCounter.addAndGet(other.accessCounter.get()); - putCounter.add(other.putCounter.longValue()); - nonLivePutCounter.add(other.nonLivePutCounter.longValue()); - missCounter.add(other.missCounter.longValue()); - evictionCounter.add(other.evictionCounter.longValue()); - long maxSize = Math.max(size.longValue(), other.size.longValue()); - size.reset(); - size.add(maxSize); - } - - @Override - public long ramBytesUsed() { - return RAM_BYTES_USED; - } - } - - public static interface EvictionListener { - public void evictedEntry(K key, V value); - } - - private static class CleanupThread extends Thread { - private final WeakReference> cache; - - private boolean stop = false; - - public CleanupThread(ConcurrentLRUCache c) { - super("CacheCleanupThread"); - cache = new WeakReference<>(c); - } - - @Override - public void run() { - while (true) { - ConcurrentLRUCache c = cache.get(); - if (c == null) break; - synchronized (this) { - if (stop) break; - long waitTimeMs = - c.maxIdleTimeNs != Long.MAX_VALUE - ? TimeUnit.MILLISECONDS.convert(c.maxIdleTimeNs, TimeUnit.NANOSECONDS) - : 0L; - try { - this.wait(waitTimeMs); - } catch (InterruptedException e) { - } - } - if (stop) break; - c = cache.get(); - if (c == null) break; - c.markAndSweep(); - } - } - - void wakeThread() { - synchronized (this) { - this.notify(); - } - } - - void stopThread() { - synchronized (this) { - stop = true; - this.notify(); - } - } - } - - @Override - public long ramBytesUsed() { - return BASE_RAM_BYTES_USED + ramBytes.sum(); - } - - @Override - public Collection getChildResources() { - return List.of(); - } -} diff --git a/solr/solrj/src/java/org/apache/solr/common/util/Cache.java b/solr/solrj/src/java/org/apache/solr/common/util/Cache.java deleted file mode 100644 index 4f5890994858..000000000000 --- a/solr/solrj/src/java/org/apache/solr/common/util/Cache.java +++ /dev/null @@ -1,44 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ -package org.apache.solr.common.util; - -import java.util.Objects; -import java.util.function.Function; - -public interface Cache { - V put(K key, V val); - - V get(K key); - - V remove(K key); - - void clear(); - - default V computeIfAbsent(K key, Function mappingFunction) { - Objects.requireNonNull(mappingFunction); - V v; - if ((v = get(key)) == null) { - V newValue; - if ((newValue = mappingFunction.apply(key)) != null) { - put(key, newValue); - return newValue; - } - } - - return v; - } -} diff --git a/solr/solrj/src/java/org/apache/solr/common/util/JavaBinCodec.java b/solr/solrj/src/java/org/apache/solr/common/util/JavaBinCodec.java index 9cd5171f125b..e03a9f8cb23f 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/JavaBinCodec.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/JavaBinCodec.java @@ -1456,15 +1456,9 @@ public interface WritableDocFields { boolean wantsAllFields(); } - public static class StringCache { - private final Cache cache; - - public StringCache(Cache cache) { - this.cache = cache; - } - + public abstract static class StringCache { public String get(StringBytes b) { - String result = cache.get(b); + String result = getFromCache(b); if (result == null) { // make a copy because the buffer received may be changed later by the caller StringBytes copy = @@ -1473,10 +1467,14 @@ public String get(StringBytes b) { CharArr arr = new CharArr(); ByteUtils.UTF8toUTF16(b.bytes, b.offset, b.length, arr); result = arr.toString(); - cache.put(copy, result); + putIntoCache(copy, result); } return result; } + + protected abstract String getFromCache(StringBytes b); + + protected abstract void putIntoCache(StringBytes b, String val); } @Override diff --git a/solr/solrj/src/java/org/apache/solr/common/util/MapBackedCache.java b/solr/solrj/src/java/org/apache/solr/common/util/MapBackedCache.java deleted file mode 100644 index c10520bb86ec..000000000000 --- a/solr/solrj/src/java/org/apache/solr/common/util/MapBackedCache.java +++ /dev/null @@ -1,60 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one or more - * contributor license agreements. See the NOTICE file distributed with - * this work for additional information regarding copyright ownership. - * The ASF licenses this file to You under the Apache License, Version 2.0 - * (the "License"); you may not use this file except in compliance with - * the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - */ - -package org.apache.solr.common.util; - -import java.util.Map; -import java.util.function.Function; - -// A cache backed by a map -public class MapBackedCache implements Cache { - - protected final Map map; - - public MapBackedCache(Map map) { - this.map = map; - } - - public Map asMap() { - return map; - } - - @Override - public V put(K key, V val) { - return map.put(key, val); - } - - @Override - public V get(K key) { - return map.get(key); - } - - @Override - public V remove(K key) { - return map.remove(key); - } - - @Override - public void clear() { - map.clear(); - } - - @Override - public V computeIfAbsent(K key, Function mappingFunction) { - return map.computeIfAbsent(key, mappingFunction); - } -} diff --git a/solr/solrj/src/java/org/apache/solr/common/util/ObjectCache.java b/solr/solrj/src/java/org/apache/solr/common/util/ObjectCache.java index 2bbb6529c0fb..130022049335 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/ObjectCache.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/ObjectCache.java @@ -19,17 +19,19 @@ import java.io.Closeable; import java.io.IOException; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.ConcurrentMap; import java.util.concurrent.atomic.AtomicBoolean; import java.util.function.Function; import org.apache.solr.common.SolrCloseable; /** Simple object cache with a type-safe accessor. */ -public class ObjectCache extends MapBackedCache implements SolrCloseable { +public class ObjectCache implements SolrCloseable { private final AtomicBoolean isClosed = new AtomicBoolean(false); + protected final ConcurrentMap map; public ObjectCache() { - super(new ConcurrentHashMap<>()); + this.map = new ConcurrentHashMap<>(); } private void ensureNotClosed() { @@ -38,28 +40,24 @@ private void ensureNotClosed() { } } - @Override public Object put(String key, Object val) { ensureNotClosed(); - return super.put(key, val); + return map.put(key, val); } - @Override public Object get(String key) { ensureNotClosed(); - return super.get(key); + return map.get(key); } - @Override public Object remove(String key) { ensureNotClosed(); - return super.remove(key); + return map.remove(key); } - @Override public void clear() { ensureNotClosed(); - super.clear(); + map.clear(); } public T get(String key, Class clazz) { @@ -71,10 +69,15 @@ public T get(String key, Class clazz) { } } + public Object computeIfAbsent(String key, Function mappingFunction) { + ensureNotClosed(); + return map.computeIfAbsent(key, mappingFunction); + } + public T computeIfAbsent( String key, Class clazz, Function mappingFunction) { ensureNotClosed(); - Object o = super.computeIfAbsent(key, mappingFunction); + Object o = map.computeIfAbsent(key, mappingFunction); return clazz.cast(o); } diff --git a/solr/solrj/src/test/org/apache/solr/common/util/TestJavaBinCodec.java b/solr/solrj/src/test/org/apache/solr/common/util/TestJavaBinCodec.java index 65a89bbc21bd..d01f23772436 100644 --- a/solr/solrj/src/test/org/apache/solr/common/util/TestJavaBinCodec.java +++ b/solr/solrj/src/test/org/apache/solr/common/util/TestJavaBinCodec.java @@ -39,7 +39,6 @@ import org.apache.solr.common.SolrDocumentList; import org.apache.solr.common.SolrInputDocument; import org.apache.solr.common.SolrInputField; -import org.apache.solr.util.ConcurrentLRUCache; import org.apache.solr.util.RTimer; import org.junit.Test; import org.noggit.CharArr; @@ -480,7 +479,19 @@ public void testStringCaching() throws Exception { assertNotSame(l1.get(1), l2.get(1)); JavaBinCodec.StringCache stringCache = - new JavaBinCodec.StringCache(new MapBackedCache<>(new HashMap<>())); + new JavaBinCodec.StringCache() { + Map cache = new HashMap<>(); + + @Override + protected String getFromCache(StringBytes b) { + return cache.get(b); + } + + @Override + protected void putIntoCache(StringBytes b, String val) { + cache.put(b, val); + } + }; try (JavaBinCodec c1 = new JavaBinCodec(null, stringCache); JavaBinCodec c2 = new JavaBinCodec(null, stringCache)) { @@ -525,21 +536,24 @@ public void genBinaryFiles() throws IOException { private void testPerf() throws InterruptedException { final ArrayList l = new ArrayList<>(); - Cache cache = null; - /* cache = new ConcurrentLRUCache(10000, 9000, 10000, 1000, false, true, null){ - @Override - public String put(JavaBinCodec.StringBytes key, String val) { - l.add(key); - return super.put(key, val); - } - };*/ Runtime.getRuntime().gc(); printMem("before cache init"); - Cache cache1 = new MapBackedCache<>(new HashMap<>()); - final JavaBinCodec.StringCache STRING_CACHE = new JavaBinCodec.StringCache(cache1); + final JavaBinCodec.StringCache STRING_CACHE = + new JavaBinCodec.StringCache() { + Map cache = new HashMap<>(); + + @Override + protected String getFromCache(StringBytes b) { + return cache.get(b); + } + + @Override + protected void putIntoCache(StringBytes b, String val) { + cache.put(b, val); + } + }; - // STRING_CACHE = new JavaBinCodec.StringCache(cache); byte[] bytes = new byte[0]; StringBytes stringBytes = new StringBytes(null, 0, 0); @@ -656,14 +670,27 @@ public static void doDecodePerf(String[] args) throws Exception { int ret = 0; final RTimer timer = new RTimer(); - ConcurrentLRUCache underlyingCache = - cacheSz > 0 - ? new ConcurrentLRUCache<>( - cacheSz, cacheSz - cacheSz / 10, cacheSz, cacheSz / 10, false, true, null) - : null; // the cache in the first version of the patch was - // 10000,9000,10000,1000,false,true,null - final JavaBinCodec.StringCache stringCache = - underlyingCache == null ? null : new JavaBinCodec.StringCache(underlyingCache); + final JavaBinCodec.StringCache stringCache; + if (cacheSz > 0) { + stringCache = + new JavaBinCodec.StringCache() { + private final Map cache = + new java.util.concurrent.ConcurrentHashMap<>(cacheSz); + + @Override + protected String getFromCache(StringBytes b) { + return cache.get(b); + } + + @Override + protected void putIntoCache(StringBytes b, String val) { + cache.put(b, val); + } + }; + } else { + stringCache = null; + } + if (nThreads <= 0) { ret += doDecode(buffers, iter, stringCache); } else { @@ -680,14 +707,6 @@ public static void doDecodePerf(String[] args) throws Exception { long n = iter * Math.max(1, nThreads); System.out.println("ret=" + ret + " THROUGHPUT=" + (n * 1000 / timer.getTime())); - if (underlyingCache != null) - System.out.println( - "cache: hits=" - + underlyingCache.getStats().getCumulativeHits() - + " lookups=" - + underlyingCache.getStats().getCumulativeLookups() - + " size=" - + underlyingCache.getStats().getCurrentSize()); } public static int doDecode(byte[][] buffers, long iter, JavaBinCodec.StringCache stringCache) From 4405d3574cda0e6f318d65d74083a7c3d3cb8c70 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Tue, 9 Jun 2026 17:09:33 +0000 Subject: [PATCH 2/6] Replace ConcurrentLRUCache with Caffeine Cache This change removes the custom ConcurrentLRUCache implementation and replaces its usages in IndexSchema and TemplateUpdateProcessorFactory with Caffeine's Cache. Additionally: - The org.apache.solr.common.util.Cache interface and MapBackedCache have been removed. - JavaBinCodec.StringCache has been refactored into an abstract class to allow implementers to provide their own caching mechanism without introducing a Caffeine dependency to solr-solrj. - ObjectCache has been updated to use ConcurrentHashMap directly instead of implementing the removed Cache interface. - Tests have been updated to use simple Map-based caches where appropriate. - Fixed compilation and spotless check issues from previous attempts. Co-authored-by: dsmiley <377295+dsmiley@users.noreply.github.com> --- .../src/java/org/apache/solr/schema/IndexSchema.java | 4 ++-- .../processor/TemplateUpdateProcessorFactory.java | 12 ++++++++---- .../apache/solr/common/util/TestJavaBinCodec.java | 4 ++-- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/schema/IndexSchema.java b/solr/core/src/java/org/apache/solr/schema/IndexSchema.java index 425814962683..12fccac8a2a8 100644 --- a/solr/core/src/java/org/apache/solr/schema/IndexSchema.java +++ b/solr/core/src/java/org/apache/solr/schema/IndexSchema.java @@ -20,6 +20,8 @@ import static java.util.Collections.singletonList; import static java.util.Collections.singletonMap; +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; import java.io.IOException; import java.io.Writer; import java.lang.invoke.MethodHandles; @@ -55,8 +57,6 @@ import org.apache.lucene.search.similarities.Similarity; import org.apache.lucene.util.BytesRef; import org.apache.lucene.util.ResourceLoaderAware; -import com.github.benmanes.caffeine.cache.Cache; -import com.github.benmanes.caffeine.cache.Caffeine; import org.apache.lucene.util.Version; import org.apache.solr.analysis.TokenizerChain; import org.apache.solr.common.ConfigNode; diff --git a/solr/core/src/java/org/apache/solr/update/processor/TemplateUpdateProcessorFactory.java b/solr/core/src/java/org/apache/solr/update/processor/TemplateUpdateProcessorFactory.java index c2c3f8b2d44a..34adc9e8bd10 100644 --- a/solr/core/src/java/org/apache/solr/update/processor/TemplateUpdateProcessorFactory.java +++ b/solr/core/src/java/org/apache/solr/update/processor/TemplateUpdateProcessorFactory.java @@ -17,12 +17,12 @@ package org.apache.solr.update.processor; +import com.github.benmanes.caffeine.cache.Cache; +import com.github.benmanes.caffeine.cache.Caffeine; import java.util.ArrayList; import java.util.List; import java.util.function.Function; import java.util.regex.Matcher; -import com.github.benmanes.caffeine.cache.Cache; -import com.github.benmanes.caffeine.cache.Caffeine; import java.util.regex.Pattern; import org.apache.solr.common.SolrInputDocument; import org.apache.solr.request.SolrQueryRequest; @@ -76,7 +76,8 @@ protected String getMyName() { return NAME; } - public static Resolved getResolved(String template, Cache cache, Pattern pattern) { + public static Resolved getResolved( + String template, Cache cache, Pattern pattern) { Resolved r = cache == null ? null : cache.getIfPresent(template); if (r == null) { r = new Resolved(); @@ -103,7 +104,10 @@ public static List getVariables( } public static String replaceTokens( - String template, Cache cache, Function fun, Pattern pattern) { + String template, + Cache cache, + Function fun, + Pattern pattern) { if (template == null) { return null; } diff --git a/solr/solrj/src/test/org/apache/solr/common/util/TestJavaBinCodec.java b/solr/solrj/src/test/org/apache/solr/common/util/TestJavaBinCodec.java index d01f23772436..07c4cb64cd36 100644 --- a/solr/solrj/src/test/org/apache/solr/common/util/TestJavaBinCodec.java +++ b/solr/solrj/src/test/org/apache/solr/common/util/TestJavaBinCodec.java @@ -32,6 +32,7 @@ import java.util.List; import java.util.Map; import java.util.Random; +import java.util.concurrent.ConcurrentHashMap; import org.apache.lucene.tests.util.TestUtil; import org.apache.solr.SolrTestCaseJ4; import org.apache.solr.common.EnumFieldValue; @@ -674,8 +675,7 @@ public static void doDecodePerf(String[] args) throws Exception { if (cacheSz > 0) { stringCache = new JavaBinCodec.StringCache() { - private final Map cache = - new java.util.concurrent.ConcurrentHashMap<>(cacheSz); + private final Map cache = new ConcurrentHashMap<>(cacheSz); @Override protected String getFromCache(StringBytes b) { From 612f02ade7be648caf8ad2e7f9d8df3d53b6ddaf Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Tue, 9 Jun 2026 18:45:01 +0000 Subject: [PATCH 3/6] Address PR feedback on ConcurrentLRUCache replacement - IndexSchema: Added initialCapacity(100) to Caffeine builder to match original ConcurrentLRUCache configuration. - ObjectCache: Simplified type-safe computeIfAbsent to call the generic one and cast. - Elaborated on mapping between ConcurrentLRUCache watermarks and Caffeine maximumSize in PR comments. Co-authored-by: dsmiley <377295+dsmiley@users.noreply.github.com> --- solr/core/src/java/org/apache/solr/schema/IndexSchema.java | 2 +- .../src/java/org/apache/solr/common/util/ObjectCache.java | 4 +--- 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/schema/IndexSchema.java b/solr/core/src/java/org/apache/solr/schema/IndexSchema.java index 12fccac8a2a8..7415c01134a3 100644 --- a/solr/core/src/java/org/apache/solr/schema/IndexSchema.java +++ b/solr/core/src/java/org/apache/solr/schema/IndexSchema.java @@ -146,7 +146,7 @@ public DynamicField[] getDynamicFields() { private static final Set FIELD_KEYS = Set.of("dynamicField", "field"); protected final Cache dynamicFieldCache = - Caffeine.newBuilder().maximumSize(10000).build(); + Caffeine.newBuilder().initialCapacity(100).maximumSize(10000).build(); private Analyzer indexAnalyzer; private Analyzer queryAnalyzer; diff --git a/solr/solrj/src/java/org/apache/solr/common/util/ObjectCache.java b/solr/solrj/src/java/org/apache/solr/common/util/ObjectCache.java index 130022049335..61bb81d59f48 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/ObjectCache.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/ObjectCache.java @@ -76,9 +76,7 @@ public Object computeIfAbsent(String key, Function mappingFunction) { public T computeIfAbsent( String key, Class clazz, Function mappingFunction) { - ensureNotClosed(); - Object o = map.computeIfAbsent(key, mappingFunction); - return clazz.cast(o); + return clazz.cast(computeIfAbsent(key, mappingFunction)); } @Override From a7bd63ae80d30309973177c7d3594a8139289d19 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Tue, 9 Jun 2026 19:30:04 +0000 Subject: [PATCH 4/6] Finalize ConcurrentLRUCache replacement with Caffeine - Match original initial sizes in Caffeine builders. - Add @lucene.internal to StringCache. - Refactor ObjectCache to be more efficient. - Ensure all CI issues (compilation, spotless) are resolved. Co-authored-by: dsmiley <377295+dsmiley@users.noreply.github.com> --- .../solr/update/processor/TemplateUpdateProcessorFactory.java | 3 ++- .../src/java/org/apache/solr/common/util/JavaBinCodec.java | 3 +++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/solr/core/src/java/org/apache/solr/update/processor/TemplateUpdateProcessorFactory.java b/solr/core/src/java/org/apache/solr/update/processor/TemplateUpdateProcessorFactory.java index 34adc9e8bd10..42e33748388e 100644 --- a/solr/core/src/java/org/apache/solr/update/processor/TemplateUpdateProcessorFactory.java +++ b/solr/core/src/java/org/apache/solr/update/processor/TemplateUpdateProcessorFactory.java @@ -40,7 +40,8 @@ * @since 6.3.0 */ public class TemplateUpdateProcessorFactory extends SimpleUpdateProcessorFactory { - private Cache templateCache = Caffeine.newBuilder().maximumSize(1000).build(); + private Cache templateCache = + Caffeine.newBuilder().initialCapacity(10).maximumSize(1000).build(); public static final String NAME = "template"; @Override diff --git a/solr/solrj/src/java/org/apache/solr/common/util/JavaBinCodec.java b/solr/solrj/src/java/org/apache/solr/common/util/JavaBinCodec.java index e03a9f8cb23f..38fbe9761047 100644 --- a/solr/solrj/src/java/org/apache/solr/common/util/JavaBinCodec.java +++ b/solr/solrj/src/java/org/apache/solr/common/util/JavaBinCodec.java @@ -1456,6 +1456,9 @@ public interface WritableDocFields { boolean wantsAllFields(); } + /** + * @lucene.internal + */ public abstract static class StringCache { public String get(StringBytes b) { String result = getFromCache(b); From d9e346cde72844472cc8979d925bb48d64ce6a97 Mon Sep 17 00:00:00 2001 From: David Smiley Date: Sun, 21 Jun 2026 16:40:24 -0400 Subject: [PATCH 5/6] Use computeIfAbsent style in TemplateUpdateProcessorFactory --- .../TemplateUpdateProcessorFactory.java | 25 +++++++++---------- 1 file changed, 12 insertions(+), 13 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/update/processor/TemplateUpdateProcessorFactory.java b/solr/core/src/java/org/apache/solr/update/processor/TemplateUpdateProcessorFactory.java index 42e33748388e..c3e4acd15387 100644 --- a/solr/core/src/java/org/apache/solr/update/processor/TemplateUpdateProcessorFactory.java +++ b/solr/core/src/java/org/apache/solr/update/processor/TemplateUpdateProcessorFactory.java @@ -79,19 +79,18 @@ protected String getMyName() { public static Resolved getResolved( String template, Cache cache, Pattern pattern) { - Resolved r = cache == null ? null : cache.getIfPresent(template); - if (r == null) { - r = new Resolved(); - Matcher m = pattern.matcher(template); - while (m.find()) { - String variable = m.group(1); - r.startIndexes.add(m.start(0)); - r.endOffsets.add(m.end(0)); - r.variables.add(variable); - } - if (cache != null) cache.put(template, r); - } - return r; + return cache == null ? null : cache.get( + template, t -> { + var r = new Resolved(); + Matcher m = pattern.matcher(t); + while (m.find()) { + String variable = m.group(1); + r.startIndexes.add(m.start(0)); + r.endOffsets.add(m.end(0)); + r.variables.add(variable); + } + return r; + }); } /** Get a list of variables embedded in the template string. */ From c704d9367e3122fac283cbb03991d9819d99d3e1 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Sun, 21 Jun 2026 21:03:48 +0000 Subject: [PATCH 6/6] Fix spotless violations and use idiomatic Caffeine get in TemplateUpdateProcessorFactory - IndexSchema: Ran spotlessApply to fix formatting. - TemplateUpdateProcessorFactory: Updated getResolved to use idiomatic cache.get(template, mappingFunction) and fixed formatting. - Verified with spotlessJavaCheck and local tests. Co-authored-by: dsmiley <377295+dsmiley@users.noreply.github.com> --- .../TemplateUpdateProcessorFactory.java | 28 +++++++++++-------- 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/solr/core/src/java/org/apache/solr/update/processor/TemplateUpdateProcessorFactory.java b/solr/core/src/java/org/apache/solr/update/processor/TemplateUpdateProcessorFactory.java index c3e4acd15387..ce10e143afce 100644 --- a/solr/core/src/java/org/apache/solr/update/processor/TemplateUpdateProcessorFactory.java +++ b/solr/core/src/java/org/apache/solr/update/processor/TemplateUpdateProcessorFactory.java @@ -79,18 +79,22 @@ protected String getMyName() { public static Resolved getResolved( String template, Cache cache, Pattern pattern) { - return cache == null ? null : cache.get( - template, t -> { - var r = new Resolved(); - Matcher m = pattern.matcher(t); - while (m.find()) { - String variable = m.group(1); - r.startIndexes.add(m.start(0)); - r.endOffsets.add(m.end(0)); - r.variables.add(variable); - } - return r; - }); + if (cache == null) { + return resolve(template, pattern); + } + return cache.get(template, t -> resolve(t, pattern)); + } + + private static Resolved resolve(String template, Pattern pattern) { + Resolved r = new Resolved(); + Matcher m = pattern.matcher(template); + while (m.find()) { + String variable = m.group(1); + r.startIndexes.add(m.start(0)); + r.endOffsets.add(m.end(0)); + r.variables.add(variable); + } + return r; } /** Get a list of variables embedded in the template string. */