From 2c1da6c6fa2e50856ea71c0e266961171bee1037 Mon Sep 17 00:00:00 2001 From: Axel Boldt-Christmas Date: Wed, 12 Jun 2024 14:06:53 +0000 Subject: [PATCH] 8332139: SymbolTableHash::Node allocations allocates twice the required memory Reviewed-by: iwalulya, coleenp --- src/hotspot/share/classfile/symbolTable.cpp | 4 ++-- src/hotspot/share/oops/symbol.hpp | 4 +--- src/hotspot/share/utilities/concurrentHashTable.hpp | 8 ++++++++ .../share/utilities/concurrentHashTable.inline.hpp | 9 +++++++++ 4 files changed, 20 insertions(+), 5 deletions(-) diff --git a/src/hotspot/share/classfile/symbolTable.cpp b/src/hotspot/share/classfile/symbolTable.cpp index 1fbc49947bf..f680939682a 100644 --- a/src/hotspot/share/classfile/symbolTable.cpp +++ b/src/hotspot/share/classfile/symbolTable.cpp @@ -172,7 +172,7 @@ public: // 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 @@ public: 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); diff --git a/src/hotspot/share/oops/symbol.hpp b/src/hotspot/share/oops/symbol.hpp index 008b979492d..3b3f1366203 100644 --- a/src/hotspot/share/oops/symbol.hpp +++ b/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; } diff --git a/src/hotspot/share/utilities/concurrentHashTable.hpp b/src/hotspot/share/utilities/concurrentHashTable.hpp index c7d5832048c..991ea9fe3c6 100644 --- a/src/hotspot/share/utilities/concurrentHashTable.hpp +++ b/src/hotspot/share/utilities/concurrentHashTable.hpp @@ -91,6 +91,13 @@ class ConcurrentHashTable : public CHeapObj { 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 { 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 diff --git a/src/hotspot/share/utilities/concurrentHashTable.inline.hpp b/src/hotspot/share/utilities/concurrentHashTable.inline.hpp index 2b30e664109..a83b6dd8a58 100644 --- a/src/hotspot/share/utilities/concurrentHashTable.inline.hpp +++ b/src/hotspot/share/utilities/concurrentHashTable.inline.hpp @@ -1055,6 +1055,15 @@ inline size_t ConcurrentHashTable:: return _table->_log2_size; } +template +inline size_t ConcurrentHashTable:: + 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 inline bool ConcurrentHashTable:: shrink(Thread* thread, size_t size_limit_log2)