[ 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)