8332139: SymbolTableHash::Node allocations allocates twice the required memory

Reviewed-by: iwalulya, coleenp

Author: xmas92

src/hotspot/share/classfile/symbolTable.cpp

@@ -172,7 +172,7 @@ class SymbolTableConfig : public AllStatic {
       // Deleting permanent symbol should not occur very often (insert race condition),
       // so log it.
       log_trace_symboltable_helper(&value, "Freeing permanent symbol");
-      size_t alloc_size = _local_table->get_node_size() + value.byte_size() + value.effective_length();
+      size_t alloc_size = SymbolTableHash::get_dynamic_node_size(value.byte_size());
       if (!SymbolTable::arena()->Afree(memory, alloc_size)) {
         log_trace_symboltable_helper(&value, "Leaked permanent symbol");
       }
@@ -182,7 +182,7 @@ class SymbolTableConfig : public AllStatic {
 
 private:
   static void* allocate_node_impl(size_t size, Value const& value) {
-    size_t alloc_size = size + value.byte_size() + value.effective_length();
+    size_t alloc_size = SymbolTableHash::get_dynamic_node_size(value.byte_size());
 #if INCLUDE_CDS
     if (CDSConfig::is_dumping_static_archive()) {
       MutexLocker ml(DumpRegion_lock, Mutex::_no_safepoint_check_flag);

src/hotspot/share/oops/symbol.hpp

@@ -122,7 +122,7 @@ class Symbol : public MetaspaceObj {
   };
 
   static int byte_size(int length) {
-    // minimum number of natural words needed to hold these bits (no non-heap version)
+    // minimum number of bytes needed to hold these bits (no non-heap version)
     return (int)(sizeof(Symbol) + (length > 2 ? length - 2 : 0));
   }
   static int size(int length) {
@@ -146,8 +146,6 @@ class Symbol : public MetaspaceObj {
 
   int size()      const     { return size(utf8_length()); }
   int byte_size() const     { return byte_size(utf8_length()); };
-  // length without the _body
-  size_t effective_length() const { return (size_t)byte_size() - sizeof(Symbol); }
 
   // Symbols should be stored in the read-only region of CDS archive.
   static bool is_read_only_by_default() { return true; }

src/hotspot/share/utilities/concurrentHashTable.hpp

@@ -91,6 +91,13 @@ class ConcurrentHashTable : public CHeapObj<F> {
 
     void print_on(outputStream* st) const {};
     void print_value_on(outputStream* st) const {};
+
+    static bool is_dynamic_sized_value_compatible() {
+      // To support dynamically sized Value types, where part of the payload is
+      // allocated beyond the end of the object, it must be that the _value
+      // field ends where the Node object ends. (No end padding).
+      return offset_of(Node, _value) + sizeof(_value) == sizeof(Node);
+    }
   };
 
   // Only constructed with placement new from an array allocated with MEMFLAGS
@@ -419,6 +426,7 @@ class ConcurrentHashTable : public CHeapObj<F> {
 
   size_t get_size_log2(Thread* thread);
   static size_t get_node_size() { return sizeof(Node); }
+  static size_t get_dynamic_node_size(size_t value_size);
   bool is_max_size_reached() { return _size_limit_reached; }
 
   // This means no paused bucket resize operation is going to resume

src/hotspot/share/utilities/concurrentHashTable.inline.hpp

@@ -1055,6 +1055,15 @@ inline size_t ConcurrentHashTable<CONFIG, F>::
   return _table->_log2_size;
 }
 
+template <typename CONFIG, MEMFLAGS F>
+inline size_t ConcurrentHashTable<CONFIG, F>::
+  get_dynamic_node_size(size_t value_size)
+{
+  assert(Node::is_dynamic_sized_value_compatible(), "VALUE must be compatible");
+  assert(value_size >= sizeof(VALUE), "must include the VALUE");
+  return sizeof(Node) - sizeof(VALUE) + value_size;
+}
+
 template <typename CONFIG, MEMFLAGS F>
 inline bool ConcurrentHashTable<CONFIG, F>::
   shrink(Thread* thread, size_t size_limit_log2)