RFR(S) 8210875 Refactor CompactHashtable

[Jiangli Zhou]

Hi Ioi,

Looks good overall. Please see my comments below.

- src/hotspot/share/classfile/compactHashtable.hpp

138 // CompactHashtable is used to stored the CDS archive's 
symbol/string tables.

Typo: stored. It's a preexisting issue.


+  V if_equals(const K key, int len, u4 offset) const {
+    V value = DECODE(_base_address, offset);
+    if (EQUALS(value, key, len)) {
+      return value;
+    } else {
+      return NULL;
+    }
+  }

+        V res = if_equals(key, len, entry[0]);
+        if (res != NULL) {
+          return res;
+        }

The name of if_equals() function is a bit confusing. I was going to 
suggest a different name, but the function is only called in two places 
and both call sites could simply do the following without if_equals():

V res = DECODE(_base_address, entry);
if (EQUALS(res, key, len)) {
   return res;
}


- src/hotspot/share/classfile/stringTable.cpp

+inline bool string_equals_compact_hashtable_entry(oop value, const 
jchar* key, int len) {
+  return java_lang_String::equals(value, (jchar*)key, len);
+}

Can java_lang_String::equals() be used for _shared_table for string 
directly? Or you simply want to mirror the 
symbol_equals_compact_hashtable_entry()?


The _shared_table is only used for INCLUDE_CDS_JAVA_HEAP, can you please 
move it under INCLUDE_CDS_JAVA_HEAP?

   77 static CompactHashtable<
   78   const jchar*, oop,
   79   read_string_from_compact_hashtable,
   80   string_equals_compact_hashtable_entry
   81 > _shared_table;


Thanks,
Jiangli

On 9/18/18 10:35 AM, Ioi Lam wrote:
> https://bugs.openjdk.java.net/browse/JDK-8210875
> http://cr.openjdk.java.net/~iklam/jdk12/refactor-compact-hash.v01/
>
> The template class CompactHashtable contains code for symbols
> and strings, which should really be provided by the instantiations
> of this template (i.e., {StringTable,SymbolTable}::_shared_table).
>
> I have refactored CompactHashtable to make it similar to
> ResourceHashtable.
>
> That way we can more easily create other instantiations, which would be
> handy in JDK-8210388 (Use hash table to store archived subgraph_info
> records.)
>
> Thanks
> - Ioi