sgup432 commented on PR #15558:
URL: https://github.com/apache/lucene/pull/15558#issuecomment-4128193278

   >is this trace/usage using the latest stuff?
   
   Yeah this is using the latest stuff. As you mentioned, SynchronizedMap(ie 
uniqueQueries) independently maintans a global mutex lock for read/write, and 
we constantly keeping seeing this causing high lock wait times for high 
throughput workloads for a hot partition.
   
   >I wonder if we can make that part better. The main reason its still a 
synchronized map is because LinkedHashMap isn't read threadsafe (e.g. even 
reads will update the inner structure).
   
   I did try that ie having a custom LRU implementation but gains were not 
immense. I think the current approach this PR takes is to divide the cache into 
multiple partitions(similar to what guava cache does) and each partition has 
its own read/write lock. So overall for different queries, we go to a different 
partition(hashkey being "query + segment") which has improved the read/write 
throughput quite significantly(as seen in above benchmarks). And internally 
this has solved SynchronizedMap.get issue as well. Overall even without 
SynchronizedMap.get issues, I think this cache was not very performant in the 
first place IMO.
   
   >uniqueQueries really does seem like the main synchronization bottle neck on 
read
   
   Yes


-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to