RFR (L) JDK-8195100: Use a low latency hashtable for SymbolTable

coleen.phillimore at oracle.com coleen.phillimore at oracle.com
Wed Aug 1 13:23:53 UTC 2018


For some reason, frames in webrev doesn't work for SymbolTable.cpp.

   44 #define PREF_AVG_LIST_LEN           4

Maybe this should be something like 8, because there can be outlier long 
bucket lists for any random hashcode.

   50 #define REHASH_LEN                  32

Maybe this should be 100.   This was the old heuristic for rehashing.

// Check to see if the hashtable is unbalanced.  The caller set a flag to
// rehash at the next safepoint.  If this bucket is 60 times greater 
than the
// expected average bucket length, it's an unbalanced hashtable.
// This is somewhat an arbitrary heuristic but if one bucket gets to
// rehash_count which is currently 100, there's probably something wrong.

It looks like the StringTable sizes are smaller.  Maybe just leave 
these, and we can experiment with shared values for the StringTable and 
SymbolTable at some later time.

  101 private:
  102 public:

Can you remove 101 and indent "public:" by one?   The single indentation 
is somewhat the convention.

  138 _needs_rehashing(false), _items(0), _uncleaned_items(0) {

indent this line too.

  177   Atomic::add((size_t)-1, &(SymbolTable::the_table()->_items));

Should this be Atomic::sub, and is the cast necessary?

  377     *is_dead = (sym->refcount() == 0);
  378     if (*is_dead) {
  379       return false;
  380     }

Did we decide that it was unnecessary to pre-check the refcount, since 
the condition is rare?

  568   Symbol* sym = SymbolTable::lookup_only((char*)name, len, hash);

I don't think you need this cast to char*.

  887   {

I don't see why you have this scope.

Except for these cosmetic comments, everything else looks fine. This is 
great work, Gerard!


On 7/30/18 12:19 PM, Gerard Ziemski wrote:
> Please review this Enhancement, which uses the new concurrent hash table for SymbolTable.
> This is an effort similar to the one behind JDK-8195097 "Make it possible to process StringTable outside safepoint” from a while ago.
> The main expected goal here is to eliminate safepoint pauses needed to cleanup the table. This goal was achieved by using “Service Thread” to do the cleaning. Checking whether we need to clean is performed on “VM Thread”, after class unloading (we check the entire table). We also check the bucket into which we happen to be inserting a new item.
> A few things to note:
> - The SymbolTable implementation follows closely that of StringTable - we might be able to factor out common code later
> - There are a few small, but statistically significant, regressions in startup benchmarks (around 2-5%) that will be addressed later, tracked by JDK-8208142
> - There is an outstanding question about whether we can safely walk the table during a safepoint using do_scan, but without locking, tracked by JDK-8208462
> - There is a cleanup opportunity presented now to remove rehashable hash table, tracked by JDK-8208519
> - There is a new test that validates that we free dead entries when we insert new symbols (using short lived symbols via Class.forName() API)
> Tested using Mach5 hs-tier1,2,3,4,5 (final test running right now…)
> Webrev: http://cr.openjdk.java.net/~gziemski/8195100_rev1/
> Bug: https://bugs.openjdk.java.net/browse/JDK-8195100
> Cheers

More information about the hotspot-runtime-dev mailing list