[GitHub] [accumulo] belugabehr commented on a change in pull request #1333: LRUCache is Evicting Too Many CachedBlocks

[GitBox <gi...@apache.org>]

belugabehr commented on a change in pull request #1333: LRUCache is Evicting Too Many CachedBlocks
URL: https://github.com/apache/accumulo/pull/1333#discussion_r339729496
 
 

 ##########
 File path: core/src/main/java/org/apache/accumulo/core/file/blockfile/cache/lru/CachedBlockQueue.java
 ##########
 @@ -17,98 +17,89 @@
  */
 package org.apache.accumulo.core.file.blockfile.cache.lru;
 
-import java.util.LinkedList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
 import java.util.PriorityQueue;
 
+import com.google.common.base.Preconditions;
+
 /**
- * A memory-bound queue that will grow until an element brings total size >= maxSize. From then
- * on, only entries that are sorted larger than the smallest current entry will be
- * inserted/replaced.
+ * A memory-bound queue that will grow until an element brings total size >=
+ * maxSize. From then on, only entries that are sorted larger than the smallest
+ * current entry will be inserted and the smallest entry is removed.
  *
  * <p>
- * Use this when you want to find the largest elements (according to their ordering, not their heap
- * size) that consume as close to the specified maxSize as possible. Default behavior is to grow
- * just above rather than just below specified max.
+ * Use this when you want to find the largest elements (according to their
+ * natural ordering, not their heap size) that consume as close to the specified
+ * maxSize as possible. Default behavior is to grow just above rather than just
+ * below specified max.
+ * </p>
  *
  * <p>
- * Object used in this queue must implement {@link HeapSize} as well as {@link Comparable}.
+ * Object used in this queue must implement {@link HeapSize} as well as
+ * {@link Comparable}.
+ * </p>
  */
 public class CachedBlockQueue implements HeapSize {
 
-  private PriorityQueue<CachedBlock> queue;
+  private final PriorityQueue<CachedBlock> queue;
 
+  private final long maxSize;
   private long heapSize;
-  private long maxSize;
 
   /**
-   * @param maxSize
-   *          the target size of elements in the queue
-   * @param blockSize
-   *          expected average size of blocks
+   * Construct a CachedBlockQueue.
+   *
+   * @param maxSize the target size of elements in the queue
+   * @param blockSize expected average size of blocks
    */
-  public CachedBlockQueue(long maxSize, long blockSize) {
-    int initialSize = (int) Math.ceil(maxSize / (double) blockSize);
-    if (initialSize == 0)
-      initialSize++;
-    queue = new PriorityQueue<>(initialSize);
-    heapSize = 0;
+  public CachedBlockQueue(final long maxSize, final long blockSize) {
 
 Review comment:
   @phrocker I've got the following checks already to cover that:
   
   ```
       Preconditions.checkArgument(maxSize > 0L);
       Preconditions.checkArgument(blockSize > 0L);
   ```

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services