8209387: Follow ups to JDK-8195100 Use a low latency hashtable for SymbolTable
authorgziemski
Mon, 10 Dec 2018 11:59:55 -0600
changeset 52931 3b0fe3d6c3d7
parent 52930 df629b081ff6
child 52932 3add7ef7c40c
8209387: Follow ups to JDK-8195100 Use a low latency hashtable for SymbolTable Summary: Use size_t, replaced macros with const, reverted incorrect API name change. Reviewed-by: coleenp, kbarrett
src/hotspot/share/classfile/stringTable.cpp
src/hotspot/share/classfile/stringTable.hpp
src/hotspot/share/classfile/symbolTable.cpp
src/hotspot/share/classfile/symbolTable.hpp
--- a/src/hotspot/share/classfile/stringTable.cpp	Mon Dec 10 09:51:23 2018 -0800
+++ b/src/hotspot/share/classfile/stringTable.cpp	Mon Dec 10 11:59:55 2018 -0600
@@ -54,13 +54,13 @@
 #include "utilities/macros.hpp"
 
 // We prefer short chains of avg 2
-#define PREF_AVG_LIST_LEN   2
+const double PREF_AVG_LIST_LEN = 2.0;
 // 2^24 is max size
-#define END_SIZE           24
+const size_t END_SIZE = 24;
 // If a chain gets to 32 something might be wrong
-#define REHASH_LEN         32
+const size_t REHASH_LEN = 32;
 // If we have as many dead items as 50% of the number of bucket
-#define CLEAN_DEAD_HIGH_WATER_MARK 0.5
+const double CLEAN_DEAD_HIGH_WATER_MARK = 0.5;
 
 #if INCLUDE_CDS_JAVA_HEAP
 inline oop read_string_from_compact_hashtable(address base_address, u4 offset) {
@@ -216,7 +216,7 @@
   return Atomic::add((size_t)1, &(the_table()->_items_count));
 }
 
-size_t StringTable::add_items_count_to_clean(size_t ndead) {
+size_t StringTable::add_items_to_clean(size_t ndead) {
   size_t total = Atomic::add((size_t)ndead, &(the_table()->_uncleaned_items_count));
   log_trace(stringtable)(
      "Uncleaned items:" SIZE_FORMAT " added: " SIZE_FORMAT " total:" SIZE_FORMAT,
@@ -228,11 +228,11 @@
   Atomic::add((size_t)-1, &(the_table()->_items_count));
 }
 
-double StringTable::get_load_factor() {
+double StringTable::get_load_factor() const {
   return (double)_items_count/_current_size;
 }
 
-double StringTable::get_dead_factor() {
+double StringTable::get_dead_factor() const {
   return (double)_uncleaned_items_count/_current_size;
 }
 
@@ -432,7 +432,7 @@
   _par_state_string->weak_oops_do(&stiac, &dnc);
 
   // Accumulate the dead strings.
-  the_table()->add_items_count_to_clean(stiac._count);
+  the_table()->add_items_to_clean(stiac._count);
 
   *processed = stiac._count_total;
   *removed = stiac._count;
--- a/src/hotspot/share/classfile/stringTable.hpp	Mon Dec 10 09:51:23 2018 -0800
+++ b/src/hotspot/share/classfile/stringTable.hpp	Mon Dec 10 11:59:55 2018 -0600
@@ -72,15 +72,15 @@
   volatile size_t _uncleaned_items_count;
   DEFINE_PAD_MINUS_SIZE(2, DEFAULT_CACHE_LINE_SIZE, sizeof(volatile size_t));
 
-  double get_load_factor();
-  double get_dead_factor();
+  double get_load_factor() const;
+  double get_dead_factor() const;
 
   void check_concurrent_work();
   void trigger_concurrent_work();
 
   static size_t item_added();
   static void item_removed();
-  size_t add_items_count_to_clean(size_t ndead);
+  size_t add_items_to_clean(size_t ndead);
 
   StringTable();
 
@@ -125,7 +125,7 @@
   // If GC uses ParState directly it should add the number of cleared
   // strings to this method.
   static void inc_dead_counter(size_t ndead) {
-    the_table()->add_items_count_to_clean(ndead);
+    the_table()->add_items_to_clean(ndead);
   }
 
   //   Delete pointers to otherwise-unreachable objects.
--- a/src/hotspot/share/classfile/symbolTable.cpp	Mon Dec 10 09:51:23 2018 -0800
+++ b/src/hotspot/share/classfile/symbolTable.cpp	Mon Dec 10 11:59:55 2018 -0600
@@ -42,19 +42,19 @@
 // We used to not resize at all, so let's be conservative
 // and not set it too short before we decide to resize,
 // to match previous startup behavior
-#define PREF_AVG_LIST_LEN           8
+const double PREF_AVG_LIST_LEN = 8.0;
 // 2^17 (131,072) is max size, which is about 6.5 times as large
 // as the previous table size (used to be 20,011),
 // which never resized
-#define END_SIZE                    17
+const size_t END_SIZE = 17;
 // If a chain gets to 100 something might be wrong
-#define REHASH_LEN                  100
+const size_t REHASH_LEN = 100;
 // We only get a chance to check whether we need
 // to clean infrequently (on class unloading),
 // so if we have even one dead entry then mark table for cleaning
-#define CLEAN_DEAD_HIGH_WATER_MARK  0.0
+const double CLEAN_DEAD_HIGH_WATER_MARK = 0.0;
 
-#define ON_STACK_BUFFER_LENGTH 128
+const size_t ON_STACK_BUFFER_LENGTH = 128;
 
 // --------------------------------------------------------------------------
 
@@ -171,8 +171,9 @@
   log_trace(symboltable)("Set uncleaned items:" SIZE_FORMAT, SymbolTable::the_table()->_uncleaned_items_count);
 }
 
+// Mark one item as needing to be cleaned, but only if no other items are marked yet
 void SymbolTable::mark_item_clean_count() {
-  if (Atomic::cmpxchg((size_t)1, &(SymbolTable::the_table()->_uncleaned_items_count), (size_t)0) == 0) { // only mark if unset
+  if (Atomic::cmpxchg((size_t)1, &(SymbolTable::the_table()->_uncleaned_items_count), (size_t)0) == 0) {
     log_trace(symboltable)("Marked uncleaned items:" SIZE_FORMAT, SymbolTable::the_table()->_uncleaned_items_count);
   }
 }
@@ -182,11 +183,11 @@
   Atomic::dec(&(SymbolTable::the_table()->_items_count));
 }
 
-double SymbolTable::get_load_factor() {
+double SymbolTable::get_load_factor() const {
   return (double)_items_count/_current_size;
 }
 
-double SymbolTable::get_dead_factor() {
+double SymbolTable::get_dead_factor() const {
   return (double)_uncleaned_items_count/_current_size;
 }
 
@@ -386,7 +387,7 @@
     assert(*value != NULL, "value should point to a symbol");
     _return = *value;
   }
-  Symbol* get_res_sym() {
+  Symbol* get_res_sym() const {
     return _return;
   }
 };
@@ -694,7 +695,7 @@
 }
 
 struct SymbolTableDoDelete : StackObj {
-  int _deleted;
+  size_t _deleted;
   SymbolTableDoDelete() : _deleted(0) {}
   void operator()(Symbol** value) {
     assert(value != NULL, "expected valid value");
@@ -706,7 +707,7 @@
 };
 
 struct SymbolTableDeleteCheck : StackObj {
-  int _processed;
+  size_t _processed;
   SymbolTableDeleteCheck() : _processed(0) {}
   bool operator()(Symbol** value) {
     assert(value != NULL, "expected valid value");
@@ -738,9 +739,9 @@
     bdt.done(jt);
   }
 
-  Atomic::add((size_t)stdc._processed, &_symbols_counted);
+  Atomic::add(stdc._processed, &_symbols_counted);
 
-  log_debug(symboltable)("Cleaned " INT32_FORMAT " of " INT32_FORMAT,
+  log_debug(symboltable)("Cleaned " SIZE_FORMAT " of " SIZE_FORMAT,
                          stdd._deleted, stdc._processed);
 }
 
@@ -775,7 +776,7 @@
 }
 
 class CountDead : StackObj {
-  int _count;
+  size_t _count;
 public:
   CountDead() : _count(0) {}
   bool operator()(Symbol** value) {
@@ -787,7 +788,7 @@
     }
     return true;
   };
-  int get_dead_count() {
+  size_t get_dead_count() const {
     return _count;
   }
 };
--- a/src/hotspot/share/classfile/symbolTable.hpp	Mon Dec 10 09:51:23 2018 -0800
+++ b/src/hotspot/share/classfile/symbolTable.hpp	Mon Dec 10 11:59:55 2018 -0600
@@ -123,8 +123,8 @@
   volatile size_t _items_count;
   volatile size_t _uncleaned_items_count;
 
-  double get_load_factor();
-  double get_dead_factor();
+  double get_load_factor() const;
+  double get_dead_factor() const;
 
   void check_concurrent_work();
   void trigger_concurrent_work();