[GitHub] [hbase] apurtell commented on a diff in pull request #4233: HBASE-26850 Optimize the implementation of LRUCache in LRUDictionary

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

apurtell commented on code in PR #4233:
URL: https://github.com/apache/hbase/pull/4233#discussion_r851782037


##########
hbase-common/src/main/java/org/apache/hadoop/hbase/io/util/LRUDictionary.java:
##########
@@ -95,23 +95,32 @@ private short put(byte[] array, int offset, int length) {
       byte[] stored = new byte[length];
       Bytes.putBytes(stored, 0, array, offset, length);
 
-      if (currSize < initSize) {
-        // There is space to add without evicting.
-        if (indexToNode[currSize] == null) {
-          indexToNode[currSize] = new Node();
-        }
-        indexToNode[currSize].setContents(stored, 0, stored.length);
-        setHead(indexToNode[currSize]);
-        short ret = (short) currSize++;
-        nodeToIndex.put(indexToNode[ret], ret);
-        return ret;
+      Node node = new Node();
+      node.setContents(stored, 0, stored.length);
+      if (nodeToIndex.containsKey(node)) {
+        short index = nodeToIndex.get(node);
+        node = indexToNode[index];
+        moveToHead(node);

Review Comment:
   I am not sure I follow the discussion because in the proposed improvement the old node is not found if that contents being stored are different.
   
         Node node = new Node();
         node.setContents(stored, 0, stored.length);
         if (nodeToIndex.containsKey(node)) {
            ...
         }
    
    containsKey will use hashcode of Node, which is Bytes.hashCode over the contents. A previous short read and a current full read will have different contents so different hashcode, right? 



-- 
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-unsubscribe@hbase.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org