[GitHub] [incubator-druid] clintropolis commented on issue #8116: remove unnecessary lock in ForegroundCachePopulator leading to a lot of contention

[GitBox]

clintropolis commented on issue #8116: remove unnecessary lock in 
ForegroundCachePopulator leading to a lot of contention
URL: https://github.com/apache/incubator-druid/pull/8116#issuecomment-514139870
 
 
   I have not had a chance to setup benchmarks for cache population to compare 
the before and after, but sampling a query locally with the same tool the 
original report was collected with does produce a nicer looking results with 
regards to the `ForegroundCachePopulator`. 
   
   I only had the example wiki data handy, so test was done with the same query 
repeated 3 times over a 5 second window with `"useCache":false` and 
`"populateCache":true` and the flame graph definitely looks a bit more chill to 
me
   
   before:
   <img width="1650" alt="Screen Shot 2019-07-23 at 2 36 03 AM" 
src="https://user-images.githubusercontent.com/1577461/61701366-e37a0a80-acf2-11e9-9934-14f9d6150db3.png";>
   
   after:
   <img width="1650" alt="Screen Shot 2019-07-23 at 2 36 29 AM" 
src="https://user-images.githubusercontent.com/1577461/61701386-ebd24580-acf2-11e9-84f2-d9daa47a1973.png";>
   
   I do think that having benchmarks in place for cache population sounds 
useful, so hopefully I can carve out some time to add those in a future PR or 
something.
   

----------------------------------------------------------------
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.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@druid.apache.org
For additional commands, e-mail: commits-h...@druid.apache.org