benwtrent commented on code in PR #13306:
URL: https://github.com/apache/lucene/pull/13306#discussion_r1574674149
##########
lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java:
##########
@@ -177,7 +180,6 @@ public boolean test(LeafReaderContext context) {
* @lucene.experimental
*/
protected void onHit(Object readerCoreKey, Query query) {
- assert lock.isHeldByCurrentThread();
Review Comment:
It seems to me that the accuracy `hitCount` and `missCount` is now broken
with this structure.
The volatile parameters can now be updated by more than one read-thread at a
time, thus adding race conditions to their increments.
I do not think we want to add this level of uncertainty to cache statistics.
What happens to performance if you adjust `missCount` and `hitCount` to be
`LongAdder` objects? This way the statistics are only locked when read, but
still allows multiple lockless thread updates?
--
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
