[ 
https://issues.apache.org/jira/browse/HDFS-7844?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14352975#comment-14352975
 ] 

Yi Liu commented on HDFS-7844:
------------------------------

Thanks Colin for addressing the comments, it's cool and only few nits:
*1.*
It's better to assert {{maxLoadFactor < 1}} (maybe < 0.8?), incorrect value 
will cause hash table failed.
It will be great if we can define a configuration property name for max 
loadFactor in CommonConfigurationKeys.java

*2.*
For my last comment#3, in {{maintainCompactness}}, the new implementation is:
{code}
+      E prevE = putInternal(e, false);
+      if (prevE != null) {
+        if (LOG.isTraceEnabled()) {
+          LOG.trace("{}: {} was already in the right place.",
+              this, e.getKey());
+        }
+      } else {
+        // The put didn't actually add anything, it just moved something.
+        // So decrement numEntries to its previous value.
+        numEntries--;
+        adaptor.clear(addr);
+        if (LOG.isTraceEnabled()) {
+          LOG.trace("{}: moved {} to the right place.",
+              this, e.getKey());
+        }
+      }
{code}
It looks brief, but I think it's not effective. {{putInternal}} needs probing 
if the slot was not in the right place, so it's not effictive.
How about only do few change based on your original implementation, like:
{code}
long bestSlot = getSlot(e.getKey(), numSlots);
if (bestSlot != slot) {
  long bestAddr = this.base + (bestSlot * slotSize);
  E occupant = adaptor.load(bestAddr);
  if (occupant == null) {
    adaptor.store(e, bestAddr);
    adaptor.clear(addr);
  }
}
{code}

*3.*
I think {{ByteArrayMemoryManager}} can only used for test for it's performance 
reason. If SUN Unsafe is not available, we should use current implemention on 
Hadoop trunk. We will not remove current implementation on trunk, right?

> Create an off-heap hash table implementation
> --------------------------------------------
>
>                 Key: HDFS-7844
>                 URL: https://issues.apache.org/jira/browse/HDFS-7844
>             Project: Hadoop HDFS
>          Issue Type: Sub-task
>    Affects Versions: HDFS-7836
>            Reporter: Colin Patrick McCabe
>            Assignee: Colin Patrick McCabe
>         Attachments: HDFS-7844-scl.001.patch, HDFS-7844-scl.002.patch, 
> HDFS-7844-scl.003.patch
>
>
> Create an off-heap hash table implementation.



--
This message was sent by Atlassian JIRA
(v6.3.4#6332)

Reply via email to