diff --git a/src/hotspot/share/classfile/symbolTable.cpp b/src/hotspot/share/classfile/symbolTable.cpp index 0c0d63204f1..f277aa5f99d 100644 --- a/src/hotspot/share/classfile/symbolTable.cpp +++ b/src/hotspot/share/classfile/symbolTable.cpp @@ -126,39 +126,84 @@ static uintx hash_shared_symbol(const char* s, int len) { #endif class SymbolTableConfig : public AllStatic { -private: + public: - typedef Symbol* Value; // value of the Node in the hashtable + typedef Symbol Value; // value of the Node in the hashtable static uintx get_hash(Value const& value, bool* is_dead) { - *is_dead = (value->refcount() == 0); + *is_dead = (value.refcount() == 0); if (*is_dead) { return 0; } else { - return hash_symbol((const char*)value->bytes(), value->utf8_length(), _alt_hash); + return hash_symbol((const char*)value.bytes(), value.utf8_length(), _alt_hash); } } // We use default allocation/deallocation but counted static void* allocate_node(void* context, size_t size, Value const& value) { SymbolTable::item_added(); - return AllocateHeap(size, mtSymbol); + return allocate_node_impl(size, value); } - static void free_node(void* context, void* memory, Value const& value) { + static void free_node(void* context, void* memory, Value & value) { // We get here because #1 some threads lost a race to insert a newly created Symbol // or #2 we're cleaning up unused symbol. // If #1, then the symbol can be either permanent, // or regular newly created one (refcount==1) // If #2, then the symbol is dead (refcount==0) - assert(value->is_permanent() || (value->refcount() == 1) || (value->refcount() == 0), - "refcount %d", value->refcount()); - if (value->refcount() == 1) { - value->decrement_refcount(); - assert(value->refcount() == 0, "expected dead symbol"); + assert(value.is_permanent() || (value.refcount() == 1) || (value.refcount() == 0), + "refcount %d", value.refcount()); +#if INCLUDE_CDS + if (DumpSharedSpaces) { + // no deallocation is needed + return; + } +#endif + if (value.refcount() == 1) { + value.decrement_refcount(); + assert(value.refcount() == 0, "expected dead symbol"); + } + if (value.refcount() != PERM_REFCOUNT) { + FreeHeap(memory); + } else { + MutexLocker ml(SymbolArena_lock, Mutex::_no_safepoint_check_flag); // Protect arena + // 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(); + if (!SymbolTable::arena()->Afree(memory, alloc_size)) { + log_trace_symboltable_helper(&value, "Leaked permanent symbol"); + } } - SymbolTable::delete_symbol(value); - FreeHeap(memory); SymbolTable::item_removed(); } + +private: + static void* allocate_node_impl(size_t size, Value const& value) { + size_t alloc_size = size + value.byte_size() + value.effective_length(); +#if INCLUDE_CDS + if (DumpSharedSpaces) { + MutexLocker ml(DumpRegion_lock, Mutex::_no_safepoint_check_flag); + // To get deterministic output from -Xshare:dump, we ensure that Symbols are allocated in + // increasing addresses. When the symbols are copied into the archive, we preserve their + // relative address order (sorted, see ArchiveBuilder::gather_klasses_and_symbols). + // + // We cannot use arena because arena chunks are allocated by the OS. As a result, for example, + // the archived symbol of "java/lang/Object" may sometimes be lower than "java/lang/String", and + // sometimes be higher. This would cause non-deterministic contents in the archive. + DEBUG_ONLY(static void* last = 0); + void* p = (void*)MetaspaceShared::symbol_space_alloc(alloc_size); + assert(p > last, "must increase monotonically"); + DEBUG_ONLY(last = p); + return p; + } +#endif + if (value.refcount() != PERM_REFCOUNT) { + return AllocateHeap(alloc_size, mtSymbol); + } else { + // Allocate to global arena + MutexLocker ml(SymbolArena_lock, Mutex::_no_safepoint_check_flag); // Protect arena + return SymbolTable::arena()->Amalloc(alloc_size); + } + } }; void SymbolTable::create_table () { @@ -176,20 +221,6 @@ void SymbolTable::create_table () { } } -void SymbolTable::delete_symbol(Symbol* sym) { - if (sym->is_permanent()) { - MutexLocker ml(SymbolArena_lock, Mutex::_no_safepoint_check_flag); // Protect arena - // Deleting permanent symbol should not occur very often (insert race condition), - // so log it. - log_trace_symboltable_helper(sym, "Freeing permanent symbol"); - if (!arena()->Afree(sym, sym->size())) { - log_trace_symboltable_helper(sym, "Leaked permanent symbol"); - } - } else { - delete sym; - } -} - void SymbolTable::reset_has_items_to_clean() { Atomic::store(&_has_items_to_clean, false); } void SymbolTable::mark_has_items_to_clean() { Atomic::store(&_has_items_to_clean, true); } bool SymbolTable::has_items_to_clean() { return Atomic::load(&_has_items_to_clean); } @@ -217,39 +248,13 @@ void SymbolTable::trigger_cleanup() { Service_lock->notify_all(); } -Symbol* SymbolTable::allocate_symbol(const char* name, int len, bool c_heap) { - assert (len <= Symbol::max_length(), "should be checked by caller"); - - Symbol* sym; - if (DumpSharedSpaces) { - // TODO: Special handling of Symbol allocation for DumpSharedSpaces will be removed - // in JDK-8250989 - c_heap = false; - } - if (c_heap) { - // refcount starts as 1 - sym = new (len) Symbol((const u1*)name, len, 1); - assert(sym != nullptr, "new should call vm_exit_out_of_memory if C_HEAP is exhausted"); - } else if (DumpSharedSpaces) { - // See comments inside Symbol::operator new(size_t, int) - sym = new (len) Symbol((const u1*)name, len, PERM_REFCOUNT); - assert(sym != nullptr, "new should call vm_exit_out_of_memory if failed to allocate symbol during DumpSharedSpaces"); - } else { - // Allocate to global arena - MutexLocker ml(SymbolArena_lock, Mutex::_no_safepoint_check_flag); // Protect arena - sym = new (len, arena()) Symbol((const u1*)name, len, PERM_REFCOUNT); - } - return sym; -} - class SymbolsDo : StackObj { SymbolClosure *_cl; public: SymbolsDo(SymbolClosure *cl) : _cl(cl) {} - bool operator()(Symbol** value) { + bool operator()(Symbol* value) { assert(value != nullptr, "expected valid value"); - assert(*value != nullptr, "value should point to a symbol"); - _cl->do_symbol(value); + _cl->do_symbol(&value); return true; }; }; @@ -334,7 +339,7 @@ Symbol* SymbolTable::new_symbol(const char* name, int len) { unsigned int hash = hash_symbol(name, len, _alt_hash); Symbol* sym = lookup_common(name, len, hash); if (sym == nullptr) { - sym = do_add_if_needed(name, len, hash, true); + sym = do_add_if_needed(name, len, hash, /* is_permanent */ false); } assert(sym->refcount() != 0, "lookup should have incremented the count"); assert(sym->equals(name, len), "symbol must be properly initialized"); @@ -349,7 +354,7 @@ Symbol* SymbolTable::new_symbol(const Symbol* sym, int begin, int end) { unsigned int hash = hash_symbol(name, len, _alt_hash); Symbol* found = lookup_common(name, len, hash); if (found == nullptr) { - found = do_add_if_needed(name, len, hash, true); + found = do_add_if_needed(name, len, hash, /* is_permanent */ false); } return found; } @@ -365,10 +370,9 @@ public: uintx get_hash() const { return _hash; } - bool equals(Symbol** value, bool* is_dead) { + bool equals(Symbol* value, bool* is_dead) { assert(value != nullptr, "expected valid value"); - assert(*value != nullptr, "value should point to a symbol"); - Symbol *sym = *value; + Symbol *sym = value; if (sym->equals(_str, _len)) { if (sym->try_increment_refcount()) { // something is referencing this symbol now. @@ -389,10 +393,9 @@ class SymbolTableGet : public StackObj { Symbol* _return; public: SymbolTableGet() : _return(nullptr) {} - void operator()(Symbol** value) { + void operator()(Symbol* value) { assert(value != nullptr, "expected valid value"); - assert(*value != nullptr, "value should point to a symbol"); - _return = *value; + _return = value; } Symbol* get_res_sym() const { return _return; @@ -453,37 +456,51 @@ Symbol* SymbolTable::lookup_only_unicode(const jchar* name, int utf16_length, void SymbolTable::new_symbols(ClassLoaderData* loader_data, const constantPoolHandle& cp, int names_count, const char** names, int* lengths, int* cp_indices, unsigned int* hashValues) { - // Note that c_heap will be true for non-strong hidden classes. + // Note that is_permanent will be false for non-strong hidden classes. // even if their loader is the boot loader because they will have a different cld. - bool c_heap = !loader_data->is_the_null_class_loader_data(); + bool is_permanent = loader_data->is_the_null_class_loader_data(); for (int i = 0; i < names_count; i++) { const char *name = names[i]; int len = lengths[i]; unsigned int hash = hashValues[i]; assert(lookup_shared(name, len, hash) == nullptr, "must have checked already"); - Symbol* sym = do_add_if_needed(name, len, hash, c_heap); + Symbol* sym = do_add_if_needed(name, len, hash, is_permanent); assert(sym->refcount() != 0, "lookup should have incremented the count"); cp->symbol_at_put(cp_indices[i], sym); } } -Symbol* SymbolTable::do_add_if_needed(const char* name, int len, uintx hash, bool heap) { +Symbol* SymbolTable::do_add_if_needed(const char* name, int len, uintx hash, bool is_permanent) { SymbolTableLookup lookup(name, len, hash); SymbolTableGet stg; bool clean_hint = false; bool rehash_warning = false; - Symbol* sym = nullptr; Thread* current = Thread::current(); + Symbol* sym; + + ResourceMark rm(current); + const int alloc_size = Symbol::byte_size(len); + u1* u1_buf = NEW_RESOURCE_ARRAY_IN_THREAD(current, u1, alloc_size); + Symbol* tmp = ::new ((void*)u1_buf) Symbol((const u1*)name, len, + (is_permanent || DumpSharedSpaces) ? PERM_REFCOUNT : 1); do { - // Callers have looked up the symbol once, insert the symbol. - sym = allocate_symbol(name, len, heap); - if (_local_table->insert(current, lookup, sym, &rehash_warning, &clean_hint)) { - break; + if (_local_table->insert(current, lookup, *tmp, &rehash_warning, &clean_hint)) { + if (_local_table->get(current, lookup, stg, &rehash_warning)) { + sym = stg.get_res_sym(); + // The get adds one to ref count, but we inserted with our ref already included. + // Therefore decrement with one. + if (sym->refcount() != PERM_REFCOUNT) { + sym->decrement_refcount(); + } + break; + } } + // In case another thread did a concurrent add, return value already in the table. // This could fail if the symbol got deleted concurrently, so loop back until success. if (_local_table->get(current, lookup, stg, &rehash_warning)) { + // The lookup added a refcount, which is ours. sym = stg.get_res_sym(); break; } @@ -505,7 +522,7 @@ Symbol* SymbolTable::new_permanent_symbol(const char* name) { int len = (int)strlen(name); Symbol* sym = SymbolTable::lookup_only(name, len, hash); if (sym == nullptr) { - sym = do_add_if_needed(name, len, hash, false); + sym = do_add_if_needed(name, len, hash, /* is_permanent */ true); } if (!sym->is_permanent()) { sym->make_permanent(); @@ -515,10 +532,9 @@ Symbol* SymbolTable::new_permanent_symbol(const char* name) { } struct SizeFunc : StackObj { - size_t operator()(Symbol** value) { + size_t operator()(Symbol* value) { assert(value != nullptr, "expected valid value"); - assert(*value != nullptr, "value should point to a symbol"); - return (*value)->size() * HeapWordSize; + return (value)->size() * HeapWordSize; }; }; @@ -545,10 +561,9 @@ void SymbolTable::print_table_statistics(outputStream* st) { // Verification class VerifySymbols : StackObj { public: - bool operator()(Symbol** value) { + bool operator()(Symbol* value) { guarantee(value != nullptr, "expected valid value"); - guarantee(*value != nullptr, "value should point to a symbol"); - Symbol* sym = *value; + Symbol* sym = value; guarantee(sym->equals((const char*)sym->bytes(), sym->utf8_length()), "symbol must be internally consistent"); return true; @@ -577,10 +592,9 @@ class DumpSymbol : StackObj { outputStream* _st; public: DumpSymbol(Thread* thr, outputStream* st) : _thr(thr), _st(st) {} - bool operator()(Symbol** value) { + bool operator()(Symbol* value) { assert(value != nullptr, "expected valid value"); - assert(*value != nullptr, "value should point to a symbol"); - print_symbol(_st, *value); + print_symbol(_st, value); return true; }; }; @@ -695,10 +709,9 @@ void SymbolTable::grow(JavaThread* jt) { struct SymbolTableDoDelete : StackObj { size_t _deleted; SymbolTableDoDelete() : _deleted(0) {} - void operator()(Symbol** value) { + void operator()(Symbol* value) { assert(value != nullptr, "expected valid value"); - assert(*value != nullptr, "value should point to a symbol"); - Symbol *sym = *value; + Symbol *sym = value; assert(sym->refcount() == 0, "refcount"); _deleted++; } @@ -707,11 +720,10 @@ struct SymbolTableDoDelete : StackObj { struct SymbolTableDeleteCheck : StackObj { size_t _processed; SymbolTableDeleteCheck() : _processed(0) {} - bool operator()(Symbol** value) { + bool operator()(Symbol* value) { assert(value != nullptr, "expected valid value"); - assert(*value != nullptr, "value should point to a symbol"); _processed++; - Symbol *sym = *value; + Symbol *sym = value; return (sym->refcount() == 0); } }; @@ -849,10 +861,9 @@ public: sizes[i] = 0; } } - bool operator()(Symbol** value) { + bool operator()(Symbol* value) { assert(value != nullptr, "expected valid value"); - assert(*value != nullptr, "value should point to a symbol"); - Symbol* sym = *value; + Symbol* sym = value; size_t size = sym->size(); size_t len = sym->utf8_length(); if (len < results_length) { diff --git a/src/hotspot/share/classfile/symbolTable.hpp b/src/hotspot/share/classfile/symbolTable.hpp index 278e529f74e..d3d279987c0 100644 --- a/src/hotspot/share/classfile/symbolTable.hpp +++ b/src/hotspot/share/classfile/symbolTable.hpp @@ -59,7 +59,6 @@ class SymbolTable : public AllStatic { // Set if one bucket is out of balance due to hash algorithm deficiency static volatile bool _needs_rehashing; - static void delete_symbol(Symbol* sym); static void grow(JavaThread* jt); static void clean_dead_entries(JavaThread* jt); @@ -75,9 +74,8 @@ class SymbolTable : public AllStatic { static void mark_has_items_to_clean(); static bool has_items_to_clean(); - static Symbol* allocate_symbol(const char* name, int len, bool c_heap); // Assumes no characters larger than 0x7F static Symbol* do_lookup(const char* name, int len, uintx hash); - static Symbol* do_add_if_needed(const char* name, int len, uintx hash, bool heap); + static Symbol* do_add_if_needed(const char* name, int len, uintx hash, bool is_permanent); // lookup only, won't add. Also calculate hash. Used by the ClassfileParser. static Symbol* lookup_only(const char* name, int len, unsigned int& hash); diff --git a/src/hotspot/share/oops/symbol.cpp b/src/hotspot/share/oops/symbol.cpp index 5aa5f4e5dcf..de5859a59e5 100644 --- a/src/hotspot/share/oops/symbol.cpp +++ b/src/hotspot/share/oops/symbol.cpp @@ -65,38 +65,11 @@ Symbol::Symbol(const u1* name, int length, int refcount) { memcpy(_body, name, length); } -void* Symbol::operator new(size_t sz, int len) throw() { -#if INCLUDE_CDS - if (DumpSharedSpaces) { - MutexLocker ml(DumpRegion_lock, Mutex::_no_safepoint_check_flag); - // To get deterministic output from -Xshare:dump, we ensure that Symbols are allocated in - // increasing addresses. When the symbols are copied into the archive, we preserve their - // relative address order (sorted, see ArchiveBuilder::gather_klasses_and_symbols). - // - // We cannot use arena because arena chunks are allocated by the OS. As a result, for example, - // the archived symbol of "java/lang/Object" may sometimes be lower than "java/lang/String", and - // sometimes be higher. This would cause non-deterministic contents in the archive. - DEBUG_ONLY(static void* last = 0); - void* p = (void*)MetaspaceShared::symbol_space_alloc(size(len)*wordSize); - assert(p > last, "must increase monotonically"); - DEBUG_ONLY(last = p); - return p; - } -#endif - int alloc_size = size(len)*wordSize; - address res = (address) AllocateHeap(alloc_size, mtSymbol); - return res; -} - -void* Symbol::operator new(size_t sz, int len, Arena* arena) throw() { - int alloc_size = size(len)*wordSize; - address res = (address)arena->AmallocWords(alloc_size); - return res; -} - -void Symbol::operator delete(void *p) { - assert(((Symbol*)p)->refcount() == 0, "should not call this"); - FreeHeap(p); +// This copies the symbol when it is added to the ConcurrentHashTable. +Symbol::Symbol(const Symbol& s1) { + _hash_and_refcount = s1._hash_and_refcount; + _length = s1._length; + memcpy(_body, s1._body, _length); } #if INCLUDE_CDS diff --git a/src/hotspot/share/oops/symbol.hpp b/src/hotspot/share/oops/symbol.hpp index 2773b9b6c29..02dbb11af79 100644 --- a/src/hotspot/share/oops/symbol.hpp +++ b/src/hotspot/share/oops/symbol.hpp @@ -131,10 +131,6 @@ class Symbol : public MetaspaceObj { } Symbol(const u1* name, int length, int refcount); - void* operator new(size_t size, int len) throw(); - void* operator new(size_t size, int len, Arena* arena) throw(); - - void operator delete(void* p); static short extract_hash(uint32_t value) { return (short)(value >> 16); } static int extract_refcount(uint32_t value) { return value & 0xffff; } @@ -143,11 +139,15 @@ class Symbol : public MetaspaceObj { int length() const { return _length; } public: + Symbol(const Symbol& s1); + // Low-level access (used with care, since not GC-safe) const u1* base() const { return &_body[0]; } - int size() { return size(utf8_length()); } - int byte_size() { return byte_size(utf8_length()); } + 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; }