[ https://issues.apache.org/jira/browse/HDFS-7844?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=14351384#comment-14351384 ]
stack commented on HDFS-7844: ----------------------------- Late to the game. Patch is great. Creative. All comments can be addressed later. High level, any notion of difference in perf when comparing native to offheap to current implementation? If we fail to pick up the configured memory manager (or the default), its worth a WARN log. Otherwise, folks may be confounded that they are getting the native memory manager though they asked for something else: 83 public static MemoryManager create(String name, Configuration conf) { 84 String memoryManagerKey = conf.get( 85 CommonConfigurationKeys.HADOOP_MEMORY_MANAGER_KEY, 86 CommonConfigurationKeys.HADOOP_MEMORY_MANAGER_DEFAULT); 87 if (memoryManagerKey == null) { 88 memoryManagerKey = NativeMemoryManager.class.getCanonicalName(); 89 } This an arbitrary max? private final static long MAX_ADDRESS = 0x3fffffffffffffffL; ByteArrayMemoryManager is just throwaway, testing? Otherwise, protect Log.TRACE with LOG.isTraceEnabled... Its fun the way you did ByteArrayMemoryManager mapping address to a Map. Ok, I see, BAMM is just for testing. Ignore above. nit: make a method rather than dup the below...: 145 Entry<Long, byte[]> entry = buffers.floorEntry(Long.valueOf(addr)); 146 if (entry == null) { 147 throw new RuntimeException("Wrote to unallocated address 0x" + 148 Long.toHexString(addr)); 149 } method would return a byte array gotten from TreeMap... etc. Is logging open at DEBUG but close at TRACE lead to confusion? Stumped debugger? The lose has to let out an IOE? What is the caller going to do w/ this IOE? The ByteArrayMemoryManager close error string construction is same as close on ProbingHashTable? Yeah man, put the Log.TRACE behind a test for TRACEyness. I buy the compactness invariant. I like the compromise put upon the Iterator (that resize is allowed while Iteration...) Seems appropriate given where this is to be deployed. On TestMemoryManager, maybe parameterize so once through with ByteArrayMemoryManager and then a run with the offheap implementation rather than have dedicated test for each: https://github.com/junit-team/junit/wiki/Parameterized-tests TestProbingHashTable is fun with its BlockInfo, etc., implementations. > 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)