diff -r 01b07229a6ad -r 688e5cbd0b91 src/hotspot/share/classfile/stringTable.cpp --- a/src/hotspot/share/classfile/stringTable.cpp Mon Jan 08 09:58:38 2018 -0500 +++ b/src/hotspot/share/classfile/stringTable.cpp Mon Jan 08 16:21:23 2018 +0100 @@ -35,6 +35,7 @@ #include "memory/filemap.hpp" #include "memory/metaspaceShared.hpp" #include "memory/resourceArea.hpp" +#include "oops/access.inline.hpp" #include "oops/oop.inline.hpp" #include "runtime/atomic.hpp" #include "runtime/mutexLocker.hpp" @@ -43,7 +44,6 @@ #include "utilities/macros.hpp" #if INCLUDE_ALL_GCS #include "gc/g1/g1CollectedHeap.hpp" -#include "gc/g1/g1SATBCardTableModRefBS.hpp" #include "gc/g1/g1StringDedup.hpp" #endif @@ -124,6 +124,22 @@ } } +oop StringTable::string_object(HashtableEntry* entry) { + return RootAccess::oop_load(entry->literal_addr()); +} + +oop StringTable::string_object_no_keepalive(HashtableEntry* entry) { + // The AS_NO_KEEPALIVE peeks at the oop without keeping it alive. + // This is *very dangerous* in general but is okay in this specific + // case. The subsequent oop_load keeps the oop alive if it it matched + // the jchar* string. + return RootAccess::oop_load(entry->literal_addr()); +} + +void StringTable::set_string_object(HashtableEntry* entry, oop string) { + RootAccess::oop_store(entry->literal_addr(), string); +} + oop StringTable::lookup_shared(jchar* name, int len, unsigned int hash) { assert(hash == java_lang_String::hash_code(name, len), "hash must be computed using java_lang_String::hash_code"); @@ -131,13 +147,16 @@ } oop StringTable::lookup_in_main_table(int index, jchar* name, - int len, unsigned int hash) { + int len, unsigned int hash) { int count = 0; for (HashtableEntry* l = bucket(index); l != NULL; l = l->next()) { count++; if (l->hash() == hash) { - if (java_lang_String::equals(l->literal(), name, len)) { - return l->literal(); + if (java_lang_String::equals(string_object_no_keepalive(l), name, len)) { + // We must perform a new load with string_object() that keeps the string + // alive as we must expose the oop as strongly reachable when exiting + // this context, in case the oop gets published. + return string_object(l); } } } @@ -192,18 +211,6 @@ return lookup(chars, length); } -// Tell the GC that this string was looked up in the StringTable. -static void ensure_string_alive(oop string) { - // A lookup in the StringTable could return an object that was previously - // considered dead. The SATB part of G1 needs to get notified about this - // potential resurrection, otherwise the marking might not find the object. -#if INCLUDE_ALL_GCS - if (UseG1GC && string != NULL) { - G1SATBCardTableModRefBS::enqueue(string); - } -#endif -} - oop StringTable::lookup(jchar* name, int len) { // shared table always uses java_lang_String::hash_code unsigned int hash = java_lang_String::hash_code(name, len); @@ -217,8 +224,6 @@ int index = the_table()->hash_to_index(hash); string = the_table()->lookup_in_main_table(index, name, len, hash); - ensure_string_alive(string); - return string; } @@ -238,9 +243,6 @@ // Found if (found_string != NULL) { - if (found_string != string_or_null()) { - ensure_string_alive(found_string); - } return found_string; } @@ -276,10 +278,6 @@ hashValue, CHECK_NULL); } - if (added_or_found != string()) { - ensure_string_alive(added_or_found); - } - return added_or_found; } @@ -388,9 +386,9 @@ while (entry != NULL) { assert(!entry->is_shared(), "CDS not used for the StringTable"); - if (is_alive->do_object_b(entry->literal())) { + if (is_alive->do_object_b(string_object_no_keepalive(entry))) { if (f != NULL) { - f->do_oop((oop*)entry->literal_addr()); + f->do_oop(entry->literal_addr()); } p = entry->next_addr(); } else { @@ -429,7 +427,7 @@ for (int i = 0; i < the_table()->table_size(); ++i) { HashtableEntry* p = the_table()->bucket(i); for ( ; p != NULL; p = p->next()) { - oop s = p->literal(); + oop s = string_object_no_keepalive(p); guarantee(s != NULL, "interned string is NULL"); unsigned int h = hash_string(s); guarantee(p->hash() == h, "broken hash in string table entry"); @@ -448,10 +446,10 @@ for (int i = 0; i < the_table()->table_size(); ++i) { HashtableEntry* p = the_table()->bucket(i); for ( ; p != NULL; p = p->next()) { - oop s = p->literal(); - typeArrayOop value = java_lang_String::value(s); - int length = java_lang_String::length(s); - bool is_latin1 = java_lang_String::is_latin1(s); + oop s = string_object_no_keepalive(p); + typeArrayOop value = java_lang_String::value_no_keepalive(s); + int length = java_lang_String::length(s); + bool is_latin1 = java_lang_String::is_latin1(s); if (length <= 0) { st->print("%d: ", length); @@ -484,8 +482,8 @@ HashtableEntry* e_ptr2) { // These entries are sanity checked by verify_and_compare_entries() // before this function is called. - oop str1 = e_ptr1->literal(); - oop str2 = e_ptr2->literal(); + oop str1 = string_object_no_keepalive(e_ptr1); + oop str2 = string_object_no_keepalive(e_ptr2); if (str1 == str2) { tty->print_cr("ERROR: identical oop values (0x" PTR_FORMAT ") " @@ -505,12 +503,12 @@ } StringTable::VerifyRetTypes StringTable::verify_entry(int bkt, int e_cnt, - HashtableEntry* e_ptr, - StringTable::VerifyMesgModes mesg_mode) { + HashtableEntry* e_ptr, + StringTable::VerifyMesgModes mesg_mode) { VerifyRetTypes ret = _verify_pass; // be optimistic - oop str = e_ptr->literal(); + oop str = string_object_no_keepalive(e_ptr); if (str == NULL) { if (mesg_mode == _verify_with_mesgs) { tty->print_cr("ERROR: NULL oop value in entry @ bucket[%d][%d]", bkt, @@ -684,7 +682,7 @@ assert(DumpSharedSpaces, "this function is only used with -Xshare:dump"); oop new_s = NULL; - typeArrayOop v = java_lang_String::value(s); + typeArrayOop v = java_lang_String::value_no_keepalive(s); typeArrayOop new_v = (typeArrayOop)MetaspaceShared::archive_heap_object(v, THREAD); if (new_v == NULL) { return NULL; @@ -708,7 +706,7 @@ for (int i = 0; i < the_table()->table_size(); ++i) { HashtableEntry* bucket = the_table()->bucket(i); for ( ; bucket != NULL; bucket = bucket->next()) { - oop s = bucket->literal(); + oop s = string_object_no_keepalive(bucket); unsigned int hash = java_lang_String::hash_code(s); if (hash == 0) { continue; @@ -721,7 +719,7 @@ } // set the archived string in bucket - bucket->set_literal(new_s); + set_string_object(bucket, new_s); // add to the compact table writer->add(hash, new_s); @@ -763,4 +761,3 @@ _shared_table.oops_do(f); } #endif //INCLUDE_CDS_JAVA_HEAP -