sgup432 commented on code in PR #15124:
URL: https://github.com/apache/lucene/pull/15124#discussion_r2305073808


##########
lucene/core/src/java/org/apache/lucene/search/LRUQueryCache.java:
##########
@@ -426,10 +425,8 @@ public void clear() {
   }
 
   private static long getRamBytesUsed(Query query) {
-    return LINKED_HASHTABLE_RAM_BYTES_PER_ENTRY
-        + (query instanceof Accountable accountableQuery
-            ? accountableQuery.ramBytesUsed()
-            : QUERY_DEFAULT_RAM_BYTES_USED);
+    long queryRamBytesUsed = RamUsageEstimator.sizeOf(query, 0);
+    return LINKED_HASHTABLE_RAM_BYTES_PER_ENTRY + queryRamBytesUsed;

Review Comment:
   Yeah I just timed one 3kb boolean query.
   
   >I am not sure how to benchmark this, but my concern is that non-accountable 
queries just got more expensive
   
   The only way I see it is to micro-benchmark `RamUsageEstimator` with 
different query types and size, might give more idea. I had tried to do same 
but with only term and boolean queries.
   
   >but my concern is that non-accountable queries just got more expensive
   
   I am not sure whether it would be that expensive. Like for `TermQuery` and 
similar cheaper queries, RamUsageEstimator should be pretty fast considering we 
already cache ramBytesUsed for the underlying terms etc. In my opinion, complex 
BooleanQueries with many nested clauses might be more expensive, as we need to 
visit all the underlying children for size calculation. To handle that, we 
already have a limit of 16 clauses beyond which we can cannot cache, and it can 
be refined further if needed. 
   
   I don't see any other good way of calculating query size until we implement 
Accountable for all queries, but it would too intrusive for just the caching 
use-case.
   
   



-- 
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

Reply via email to