jpountz commented on code in PR #13306: URL: https://github.com/apache/lucene/pull/13306#discussion_r1592821514
########## lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java: ########## @@ -96,14 +98,15 @@ public class LRUQueryCache implements QueryCache, Accountable { // mostRecentlyUsedQueries. This is why write operations are performed under a lock private final Set<Query> mostRecentlyUsedQueries; private final Map<IndexReader.CacheKey, LeafCache> cache; - private final ReentrantLock lock; + private final ReentrantReadWriteLock.ReadLock readLock; + private final ReentrantReadWriteLock.WriteLock writeLock; private final float skipCacheFactor; // these variables are volatile so that we do not need to sync reads // but increments need to be performed under the lock private volatile long ramBytesUsed; - private volatile long hitCount; - private volatile long missCount; + private volatile LongAdder hitCount; + private volatile LongAdder missCount; Review Comment: These fields could be made final and non volative now. ########## lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java: ########## @@ -911,15 +919,15 @@ public BulkScorer bulkScorer(LeafReaderContext context) throws IOException { } // If the lock is already busy, prefer using the uncached version than waiting - if (lock.tryLock() == false) { + if (writeLock.tryLock() == false) { Review Comment: Why are we using the write lock here? It looks like we're only reading so we could use the read lock? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org --------------------------------------------------------------------- To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org For additional commands, e-mail: issues-h...@lucene.apache.org