jpountz commented on code in PR #14412:
URL: https://github.com/apache/lucene/pull/14412#discussion_r2017616847
##########
lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java:
##########
@@ -99,7 +100,7 @@ public class LRUQueryCache implements QueryCache,
Accountable {
private final Map<IndexReader.CacheKey, LeafCache> cache;
private final ReentrantReadWriteLock.ReadLock readLock;
private final ReentrantReadWriteLock.WriteLock writeLock;
- private final float skipCacheFactor;
+ private final AtomicReference<Float> skipCacheFactor;
Review Comment:
Making it volatile should be enough?
##########
lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java:
##########
@@ -122,12 +123,30 @@ public LRUQueryCache(
long maxRamBytesUsed,
Predicate<LeafReaderContext> leavesToCache,
float skipCacheFactor) {
+ this(maxSize, maxRamBytesUsed, leavesToCache, new
AtomicReference<>(skipCacheFactor));
+ }
+
+ /**
+ * Additionally, allows the ability to pass skipCacheFactor in form of
AtomicReference where the
+ * caller can dynamically update(in a thread safe way) its value by calling
skipCacheFactor.set()
+ * on their end.
+ */
+ public LRUQueryCache(
+ int maxSize,
+ long maxRamBytesUsed,
+ Predicate<LeafReaderContext> leavesToCache,
+ AtomicReference<Float> skipCacheFactor) {
Review Comment:
Let's avoid exposing the implementation detail of how the skip cache factor
is stored internally, ie. skipCacheFactor should be a regular `float` here?
##########
lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java:
##########
@@ -142,6 +161,10 @@ public LRUQueryCache(
missCount = new LongAdder();
}
+ AtomicReference<Float> getSkipCacheFactor() {
+ return skipCacheFactor;
+ }
Review Comment:
Can you add a regular getter/setter instead?
##########
lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java:
##########
@@ -269,8 +292,8 @@ boolean requiresEviction() {
}
CacheAndCount get(Query key, IndexReader.CacheHelper cacheHelper) {
- assert key instanceof BoostQuery == false;
- assert key instanceof ConstantScoreQuery == false;
+ assert !(key instanceof BoostQuery);
+ assert !(key instanceof ConstantScoreQuery);
Review Comment:
Please undo these changes, these `== false` are on purpose (you'll find many
of them across the code base) to be more readable.
--
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
