ben-manes commented on a change in pull request #321: ACCUMULO-4641 Added loading to cache API URL: https://github.com/apache/accumulo/pull/321#discussion_r150140321
########## File path: core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/tinylfu/TinyLfuBlockCache.java ########## @@ -47,7 +47,7 @@ * <li>Cache design: http://highscalability.com/blog/2016/1/25/design-of-a-modern-cache.html</li> * </ul> */ -public final class TinyLfuBlockCache implements BlockCache { +public final class TinyLfuBlockCache extends SynchronousLoadingBlockCache implements BlockCache { Review comment: Sure, no rush. I mostly wanted to point out that the feature is built-in. Like yours, there is a chance of collisions so both are effectively per-entry but not absolutely. In `ConcurrentHashMap` it locks on the bin which contains a small number of entries. The likelihood of collision on a write is stated to be... > Lock contention probability for two threads accessing distinct elements is roughly 1 / (8 * #elements) under random hashes. ---------------------------------------------------------------- This is an automated message from the Apache Git Service. To respond to the message, please log on GitHub and use the URL above to go to the specific comment. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
