diff -r d8a240abb23a -r 246d977b51f2 src/share/vm/classfile/symbolTable.cpp --- a/src/share/vm/classfile/symbolTable.cpp Fri Jun 22 15:39:16 2012 -0700 +++ b/src/share/vm/classfile/symbolTable.cpp Mon Jun 25 21:33:35 2012 -0400 @@ -46,11 +46,8 @@ jint SymbolTable::_seed = 0; Symbol* SymbolTable::allocate_symbol(const u1* name, int len, bool c_heap, TRAPS) { - // Don't allow symbols to be created which cannot fit in a Symbol*. - if (len > Symbol::max_length()) { - THROW_MSG_0(vmSymbols::java_lang_InternalError(), - "name is too long to represent"); - } + assert (len <= Symbol::max_length(), "should be checked by caller"); + Symbol* sym; // Allocate symbols in the C heap when dumping shared spaces in case there // are temporary symbols we can remove. @@ -95,9 +92,14 @@ int total = 0; size_t memory_total = 0; for (int i = 0; i < the_table()->table_size(); ++i) { - for (HashtableEntry** p = the_table()->bucket_addr(i); *p != NULL; ) { - HashtableEntry* entry = *p; - if (entry->is_shared()) { + HashtableEntry** p = the_table()->bucket_addr(i); + HashtableEntry* entry = the_table()->bucket(i); + while (entry != NULL) { + // Shared entries are normally at the end of the bucket and if we run into + // a shared entry, then there is nothing more to remove. However, if we + // have rehashed the table, then the shared entries are no longer at the + // end of the bucket. + if (entry->is_shared() && !use_alternate_hashcode()) { break; } Symbol* s = entry->literal(); @@ -106,6 +108,7 @@ assert(s != NULL, "just checking"); // If reference count is zero, remove. if (s->refcount() == 0) { + assert(!entry->is_shared(), "shared entries should be kept live"); delete s; removed++; *p = entry->next(); @@ -113,6 +116,8 @@ } else { p = entry->next_addr(); } + // get next entry + entry = (HashtableEntry*)HashtableEntry::make_ptr(*p); } } symbols_removed += removed; @@ -135,7 +140,8 @@ // with the existing strings. Set flag to use the alternate hash code afterwards. void SymbolTable::rehash_table() { assert(SafepointSynchronize::is_at_safepoint(), "must be at safepoint"); - assert(!DumpSharedSpaces, "this should never happen with -Xshare:dump"); + // This should never happen with -Xshare:dump but it might in testing mode. + if (DumpSharedSpaces) return; // Create a new symbol table SymbolTable* new_table = new SymbolTable(); @@ -176,12 +182,11 @@ return NULL; } -// Pick hashing algorithm, but return value already given if not using a new -// hash algorithm. -unsigned int SymbolTable::hash_symbol(const char* s, int len, unsigned int hashValue) { +// Pick hashing algorithm. +unsigned int SymbolTable::hash_symbol(const char* s, int len) { return use_alternate_hashcode() ? AltHashing::murmur3_32(seed(), (const jbyte*)s, len) : - (hashValue != 0 ? hashValue : java_lang_String::to_hash(s, len)); + java_lang_String::to_hash(s, len); } @@ -201,6 +206,9 @@ // Found if (s != NULL) return s; + // Grab SymbolTable_lock first. + MutexLocker ml(SymbolTable_lock, THREAD); + // Otherwise, add to symbol to table return the_table()->basic_add(index, (u1*)name, len, hashValue, true, CHECK_NULL); } @@ -238,6 +246,9 @@ // We can't include the code in No_Safepoint_Verifier because of the // ResourceMark. + // Grab SymbolTable_lock first. + MutexLocker ml(SymbolTable_lock, THREAD); + return the_table()->basic_add(index, (u1*)buffer, len, hashValue, true, CHECK_NULL); } @@ -306,6 +317,9 @@ int names_count, const char** names, int* lengths, int* cp_indices, unsigned int* hashValues, TRAPS) { + // Grab SymbolTable_lock first. + MutexLocker ml(SymbolTable_lock, THREAD); + SymbolTable* table = the_table(); bool added = table->basic_add(class_loader, cp, names_count, names, lengths, cp_indices, hashValues, CHECK); @@ -326,22 +340,39 @@ if (result != NULL) { return result; } + // Grab SymbolTable_lock first. + MutexLocker ml(SymbolTable_lock, THREAD); + SymbolTable* table = the_table(); int index = table->hash_to_index(hash); return table->basic_add(index, (u1*)name, (int)strlen(name), hash, false, THREAD); } -Symbol* SymbolTable::basic_add(int index, u1 *name, int len, +Symbol* SymbolTable::basic_add(int index_arg, u1 *name, int len, unsigned int hashValue_arg, bool c_heap, TRAPS) { assert(!Universe::heap()->is_in_reserved(name) || GC_locker::is_active(), "proposed name of symbol must be stable"); - // Grab SymbolTable_lock first. - MutexLocker ml(SymbolTable_lock, THREAD); + // Don't allow symbols to be created which cannot fit in a Symbol*. + if (len > Symbol::max_length()) { + THROW_MSG_0(vmSymbols::java_lang_InternalError(), + "name is too long to represent"); + } + + // Cannot hit a safepoint in this function because the "this" pointer can move. + No_Safepoint_Verifier nsv; // Check if the symbol table has been rehashed, if so, need to recalculate - // the hash value. - unsigned int hashValue = hash_symbol((const char*)name, len, hashValue_arg); + // the hash value and index. + unsigned int hashValue; + int index; + if (use_alternate_hashcode()) { + hashValue = hash_symbol((const char*)name, len); + index = hash_to_index(hashValue); + } else { + hashValue = hashValue_arg; + index = index_arg; + } // Since look-up was done lock-free, we need to check if another // thread beat us in the race to insert the symbol. @@ -377,13 +408,18 @@ } } - // Hold SymbolTable_lock through the symbol creation - MutexLocker ml(SymbolTable_lock, THREAD); + // Cannot hit a safepoint in this function because the "this" pointer can move. + No_Safepoint_Verifier nsv; for (int i=0; iis_in_reserved(name) || GC_locker::is_active(), - "proposed name of symbol must be stable"); - - Handle string; - // try to reuse the string if possible - if (!string_or_null.is_null() && (!JavaObjectsInPerm || string_or_null()->is_perm())) { - string = string_or_null; - } else { - string = java_lang_String::create_tenured_from_unicode(name, len, CHECK_NULL); - } - - // Allocation must be done before grapping the SymbolTable_lock lock - MutexLocker ml(StringTable_lock, THREAD); assert(java_lang_String::equals(string(), name, len), "string must be properly initialized"); + // Cannot hit a safepoint in this function because the "this" pointer can move. + No_Safepoint_Verifier nsv; // Check if the symbol table has been rehashed, if so, need to recalculate - // the hash value before second lookup. - unsigned int hashValue = hash_string(name, len, hashValue_arg); + // the hash value and index before second lookup. + unsigned int hashValue; + int index; + if (use_alternate_hashcode()) { + hashValue = hash_string(name, len); + index = hash_to_index(hashValue); + } else { + hashValue = hashValue_arg; + index = index_arg; + } // Since look-up was done lock-free, we need to check if another // thread beat us in the race to insert the symbol. @@ -664,13 +696,29 @@ int len, TRAPS) { unsigned int hashValue = hash_string(name, len); int index = the_table()->hash_to_index(hashValue); - oop string = the_table()->lookup(index, name, len, hashValue); + oop found_string = the_table()->lookup(index, name, len, hashValue); // Found - if (string != NULL) return string; + if (found_string != NULL) return found_string; + + debug_only(StableMemoryChecker smc(name, len * sizeof(name[0]))); + assert(!Universe::heap()->is_in_reserved(name) || GC_locker::is_active(), + "proposed name of symbol must be stable"); + + Handle string; + // try to reuse the string if possible + if (!string_or_null.is_null() && (!JavaObjectsInPerm || string_or_null()->is_perm())) { + string = string_or_null; + } else { + string = java_lang_String::create_tenured_from_unicode(name, len, CHECK_NULL); + } + + // Grab the StringTable_lock before getting the_table() because it could + // change at safepoint. + MutexLocker ml(StringTable_lock, THREAD); // Otherwise, add to symbol to table - return the_table()->basic_add(index, string_or_null, name, len, + return the_table()->basic_add(index, string, name, len, hashValue, CHECK_NULL); } @@ -713,18 +761,24 @@ // entries at a safepoint. assert(SafepointSynchronize::is_at_safepoint(), "must be at safepoint"); for (int i = 0; i < the_table()->table_size(); ++i) { - for (HashtableEntry** p = the_table()->bucket_addr(i); *p != NULL; ) { - HashtableEntry* entry = *p; - if (entry->is_shared()) { + HashtableEntry** p = the_table()->bucket_addr(i); + HashtableEntry* entry = the_table()->bucket(i); + while (entry != NULL) { + // Shared entries are normally at the end of the bucket and if we run into + // a shared entry, then there is nothing more to remove. However, if we + // have rehashed the table, then the shared entries are no longer at the + // end of the bucket. + if (entry->is_shared() && !use_alternate_hashcode()) { break; } assert(entry->literal() != NULL, "just checking"); - if (is_alive->do_object_b(entry->literal())) { + if (entry->is_shared() || is_alive->do_object_b(entry->literal())) { p = entry->next_addr(); } else { *p = entry->next(); the_table()->free_entry(entry); } + entry = (HashtableEntry*)HashtableEntry::make_ptr(*p); } } } @@ -795,7 +849,8 @@ // with the existing strings. Set flag to use the alternate hash code afterwards. void StringTable::rehash_table() { assert(SafepointSynchronize::is_at_safepoint(), "must be at safepoint"); - assert(!DumpSharedSpaces, "this should never happen with -Xshare:dump"); + // This should never happen with -Xshare:dump but it might in testing mode. + if (DumpSharedSpaces) return; StringTable* new_table = new StringTable(); // Initialize new global seed for hashing.