Skip to content

Commit

Permalink
8290370: Convert SymbolPropertyTable to ResourceHashtable
Browse files Browse the repository at this point in the history 
Reviewed-by: coleenp, iklam
  • Loading branch information
@gujustin1 @coleenp
gujustin1 authored and coleenp committed Aug 3, 2022
1 parent 67f0011 commit 13f0f12
Show file tree
Hide file tree
Showing 6 changed files with 93 additions and 230 deletions.
83 changes: 0 additions & 83 deletions src/hotspot/share/classfile/dictionary.cpp
View file
Expand Up @@ -481,89 +481,6 @@ void Dictionary::clean_cached_protection_domains(GrowableArray<ProtectionDomainE
}
}

oop SymbolPropertyEntry::method_type() const {
return _method_type.resolve();
}

void SymbolPropertyEntry::set_method_type(oop p) {
_method_type = OopHandle(Universe::vm_global(), p);
}

void SymbolPropertyEntry::free_entry() {
// decrement Symbol refcount here because hashtable doesn't.
literal()->decrement_refcount();
// Free OopHandle
_method_type.release(Universe::vm_global());
}

void SymbolPropertyEntry::print_entry(outputStream* st) const {
symbol()->print_value_on(st);
st->print("/mode=" INTX_FORMAT, symbol_mode());
st->print(" -> ");
bool printed = false;
if (method() != NULL) {
method()->print_value_on(st);
printed = true;
}
if (method_type() != NULL) {
if (printed) st->print(" and ");
st->print(INTPTR_FORMAT, p2i((void *)method_type()));
printed = true;
}
st->print_cr(printed ? "" : "(empty)");
}

SymbolPropertyTable::SymbolPropertyTable(int table_size)
: Hashtable<Symbol*, mtSymbol>(table_size, sizeof(SymbolPropertyEntry))
{
}
SymbolPropertyTable::SymbolPropertyTable(int table_size, HashtableBucket<mtSymbol>* t,
int number_of_entries)
: Hashtable<Symbol*, mtSymbol>(table_size, sizeof(SymbolPropertyEntry), t, number_of_entries)
{
}


SymbolPropertyEntry* SymbolPropertyTable::find_entry(int index, unsigned int hash,
Symbol* sym,
intptr_t sym_mode) {
assert(index == index_for(sym, sym_mode), "incorrect index?");
for (SymbolPropertyEntry* p = bucket(index); p != NULL; p = p->next()) {
if (p->hash() == hash && p->symbol() == sym && p->symbol_mode() == sym_mode) {
return p;
}
}
return NULL;
}


SymbolPropertyEntry* SymbolPropertyTable::add_entry(int index, unsigned int hash,
Symbol* sym, intptr_t sym_mode) {
assert_locked_or_safepoint(SystemDictionary_lock);
assert(index == index_for(sym, sym_mode), "incorrect index?");
assert(find_entry(index, hash, sym, sym_mode) == NULL, "no double entry");

SymbolPropertyEntry* p = new_entry(hash, sym, sym_mode);
Hashtable<Symbol*, mtSymbol>::add_entry(index, p);
return p;
}

void SymbolPropertyTable::methods_do(void f(Method*)) {
for (int index = 0; index < table_size(); index++) {
for (SymbolPropertyEntry* p = bucket(index); p != NULL; p = p->next()) {
Method* prop = p->method();
if (prop != NULL) {
f((Method*)prop);
}
}
}
}

void SymbolPropertyTable::free_entry(SymbolPropertyEntry* entry) {
entry->free_entry();
BasicHashtable<mtSymbol>::free_entry(entry);
}

void DictionaryEntry::verify_protection_domain_set() {
assert(SafepointSynchronize::is_at_safepoint(), "must only be called as safepoint");
for (ProtectionDomainEntry* current = pd_set_acquire(); // accessed at a safepoint
Expand Down
94 changes: 0 additions & 94 deletions src/hotspot/share/classfile/dictionary.hpp
View file
Expand Up @@ -153,98 +153,4 @@ class DictionaryEntry : public HashtableEntry<InstanceKlass*, mtClass> {
void verify();
};

// Entry in a SymbolPropertyTable, mapping a single Symbol*
// to a managed and an unmanaged pointer.
class SymbolPropertyEntry : public HashtableEntry<Symbol*, mtSymbol> {
friend class VMStructs;
private:
intptr_t _symbol_mode; // secondary key
Method* _method;
OopHandle _method_type;

public:
Symbol* symbol() const { return literal(); }

intptr_t symbol_mode() const { return _symbol_mode; }
void set_symbol_mode(intptr_t m) { _symbol_mode = m; }

Method* method() const { return _method; }
void set_method(Method* p) { _method = p; }

oop method_type() const;
void set_method_type(oop p);

// We need to clear the OopHandle because these hashtable entries are not constructed properly.
void clear_method_type() { _method_type = OopHandle(); }

void free_entry();

SymbolPropertyEntry* next() const {
return (SymbolPropertyEntry*)HashtableEntry<Symbol*, mtSymbol>::next();
}

SymbolPropertyEntry** next_addr() {
return (SymbolPropertyEntry**)HashtableEntry<Symbol*, mtSymbol>::next_addr();
}

void print_entry(outputStream* st) const;
};

// A system-internal mapping of symbols to pointers, both managed
// and unmanaged. Used to record the auto-generation of each method
// MethodHandle.invoke(S)T, for all signatures (S)T.
class SymbolPropertyTable : public Hashtable<Symbol*, mtSymbol> {
friend class VMStructs;
private:
// The following method is not MT-safe and must be done under lock.
SymbolPropertyEntry** bucket_addr(int i) {
return (SymbolPropertyEntry**) Hashtable<Symbol*, mtSymbol>::bucket_addr(i);
}

void add_entry(int index, SymbolPropertyEntry* new_entry) {
ShouldNotReachHere();
}
void set_entry(int index, SymbolPropertyEntry* new_entry) {
ShouldNotReachHere();
}

SymbolPropertyEntry* new_entry(unsigned int hash, Symbol* symbol, intptr_t symbol_mode) {
SymbolPropertyEntry* entry = (SymbolPropertyEntry*) Hashtable<Symbol*, mtSymbol>::new_entry(hash, symbol);
// Hashtable with Symbol* literal must increment and decrement refcount.
symbol->increment_refcount();
entry->set_symbol_mode(symbol_mode);
entry->set_method(NULL);
entry->clear_method_type();
return entry;
}

public:
SymbolPropertyTable(int table_size);
SymbolPropertyTable(int table_size, HashtableBucket<mtSymbol>* t, int number_of_entries);

void free_entry(SymbolPropertyEntry* entry);

unsigned int compute_hash(Symbol* sym, intptr_t symbol_mode) {
// Use the regular identity_hash.
return Hashtable<Symbol*, mtSymbol>::compute_hash(sym) ^ symbol_mode;
}

int index_for(Symbol* name, intptr_t symbol_mode) {
return hash_to_index(compute_hash(name, symbol_mode));
}

// need not be locked; no state change
SymbolPropertyEntry* find_entry(int index, unsigned int hash, Symbol* name, intptr_t name_mode);

// must be done under SystemDictionary_lock
SymbolPropertyEntry* add_entry(int index, unsigned int hash, Symbol* name, intptr_t name_mode);

void methods_do(void f(Method*));

void verify();

SymbolPropertyEntry* bucket(int i) {
return (SymbolPropertyEntry*) Hashtable<Symbol*, mtSymbol>::bucket(i);
}
};
#endif // SHARE_CLASSFILE_DICTIONARY_HPP
134 changes: 88 additions & 46 deletions src/hotspot/share/classfile/systemDictionary.cpp
View file
Expand Up @@ -62,6 +62,8 @@
#include "oops/objArrayKlass.hpp"
#include "oops/objArrayOop.inline.hpp"
#include "oops/oop.inline.hpp"
#include "oops/oop.hpp"
#include "oops/oopHandle.hpp"
#include "oops/oopHandle.inline.hpp"
#include "oops/symbol.hpp"
#include "oops/typeArrayKlass.hpp"
Expand All @@ -87,7 +89,32 @@
#include "jfr/jfr.hpp"
#endif

SymbolPropertyTable* SystemDictionary::_invoke_method_table = NULL;
class InvokeMethodKey : public StackObj {
private:
Symbol* _symbol;
intptr_t _iid;

public:
InvokeMethodKey(Symbol* symbol, intptr_t iid) :
_symbol(symbol),
_iid(iid) {}

static bool key_comparison(InvokeMethodKey const &k1, InvokeMethodKey const &k2){
return k1._symbol == k2._symbol && k1._iid == k2._iid;
}

static unsigned int compute_hash(const InvokeMethodKey &k) {
Symbol* sym = k._symbol;
intptr_t iid = k._iid;
unsigned int hash = (unsigned int) sym -> identity_hash();
return (unsigned int) (hash ^ iid);
}

};

ResourceHashtable<InvokeMethodKey, Method*, 139, ResourceObj::C_HEAP, mtClass,
InvokeMethodKey::compute_hash, InvokeMethodKey::key_comparison> _invoke_method_intrinsic_table;
ResourceHashtable<Symbol*, OopHandle, 139, ResourceObj::C_HEAP, mtClass> _invoke_method_type_table;
ProtectionDomainCacheTable* SystemDictionary::_pd_cache_table = NULL;

OopHandle SystemDictionary::_java_system_loader;
Expand Down Expand Up @@ -1633,10 +1660,21 @@ bool SystemDictionary::do_unloading(GCTimer* gc_timer) {

void SystemDictionary::methods_do(void f(Method*)) {
// Walk methods in loaded classes
MutexLocker ml(ClassLoaderDataGraph_lock);
ClassLoaderDataGraph::methods_do(f);
// Walk method handle intrinsics
invoke_method_table()->methods_do(f);

{
MutexLocker ml(ClassLoaderDataGraph_lock);
ClassLoaderDataGraph::methods_do(f);
}

auto doit = [&] (InvokeMethodKey key, Method* method) {
f(method);
};

{
MutexLocker ml(InvokeMethodTable_lock);
_invoke_method_intrinsic_table.iterate_all(doit);
}

}

// ----------------------------------------------------------------------------
Expand All @@ -1646,7 +1684,6 @@ void SystemDictionary::initialize(TRAPS) {
// Allocate arrays
_placeholders = new PlaceholderTable(_placeholder_table_size);
_loader_constraints = new LoaderConstraintTable(_loader_constraint_size);
_invoke_method_table = new SymbolPropertyTable(_invoke_method_size);
_pd_cache_table = new ProtectionDomainCacheTable(defaultProtectionDomainCacheSize);

#if INCLUDE_CDS
Expand Down Expand Up @@ -1994,48 +2031,48 @@ Symbol* SystemDictionary::check_signature_loaders(Symbol* signature,
Method* SystemDictionary::find_method_handle_intrinsic(vmIntrinsicID iid,
Symbol* signature,
TRAPS) {

methodHandle empty;
const int iid_as_int = vmIntrinsics::as_int(iid);
assert(MethodHandles::is_signature_polymorphic(iid) &&
MethodHandles::is_signature_polymorphic_intrinsic(iid) &&
iid != vmIntrinsics::_invokeGeneric,
"must be a known MH intrinsic iid=%d: %s", iid_as_int, vmIntrinsics::name_at(iid));

unsigned int hash = invoke_method_table()->compute_hash(signature, iid_as_int);
int index = invoke_method_table()->hash_to_index(hash);
SymbolPropertyEntry* spe = invoke_method_table()->find_entry(index, hash, signature, iid_as_int);
methodHandle m;
if (spe == NULL || spe->method() == NULL) {
spe = NULL;
// Must create lots of stuff here, but outside of the SystemDictionary lock.
m = Method::make_method_handle_intrinsic(iid, signature, CHECK_NULL);
if (!Arguments::is_interpreter_only() || iid == vmIntrinsics::_linkToNative) {
// Generate a compiled form of the MH intrinsic.
Method** met;
InvokeMethodKey key(signature, iid_as_int);
{
MutexLocker ml(THREAD, InvokeMethodTable_lock);
met = _invoke_method_intrinsic_table.get(key);
if (met != nullptr) {
return *met;
}
}

methodHandle m = Method::make_method_handle_intrinsic(iid, signature, CHECK_NULL);
if (!Arguments::is_interpreter_only() || iid == vmIntrinsics::_linkToNative) {
// Generate a compiled form of the MH intrinsic
// linkToNative doesn't have interpreter-specific implementation, so always has to go through compiled version.
AdapterHandlerLibrary::create_native_wrapper(m);
// Check if have the compiled code.
if (!m->has_compiled_code()) {
THROW_MSG_NULL(vmSymbols::java_lang_VirtualMachineError(),
"Out of space in CodeCache for method handle intrinsic");
}
}
// Now grab the lock. We might have to throw away the new method,
// if a racing thread has managed to install one at the same time.
{
MutexLocker ml(THREAD, SystemDictionary_lock);
spe = invoke_method_table()->find_entry(index, hash, signature, iid_as_int);
if (spe == NULL)
spe = invoke_method_table()->add_entry(index, hash, signature, iid_as_int);
if (spe->method() == NULL)
spe->set_method(m());
}
}

assert(spe != NULL && spe->method() != NULL, "");
assert(Arguments::is_interpreter_only() || (spe->method()->has_compiled_code() &&
spe->method()->code()->entry_point() == spe->method()->from_compiled_entry()),
// Now grab the lock. We might have to throw away the new method,
// if a racing thread has managed to install one at the same time.
{
MutexLocker ml(THREAD, InvokeMethodTable_lock);
signature->make_permanent(); // The signature is never unloaded.
bool created;
met = _invoke_method_intrinsic_table.put_if_absent(key, m(), &created);
Method* saved_method = *met;
assert(Arguments::is_interpreter_only() || (saved_method->has_compiled_code() &&
saved_method->code()->entry_point() == saved_method->from_compiled_entry()),
"MH intrinsic invariant");
return spe->method();
return saved_method;
}
}

// Helper for unpacking the return value from linkMethod and linkCallSite.
Expand Down Expand Up @@ -2176,13 +2213,16 @@ Handle SystemDictionary::find_method_handle_type(Symbol* signature,
Klass* accessing_klass,
TRAPS) {
Handle empty;
int null_iid = vmIntrinsics::as_int(vmIntrinsics::_none); // distinct from all method handle invoker intrinsics
unsigned int hash = invoke_method_table()->compute_hash(signature, null_iid);
int index = invoke_method_table()->hash_to_index(hash);
SymbolPropertyEntry* spe = invoke_method_table()->find_entry(index, hash, signature, null_iid);
if (spe != NULL && spe->method_type() != NULL) {
assert(java_lang_invoke_MethodType::is_instance(spe->method_type()), "");
return Handle(THREAD, spe->method_type());
OopHandle* o;
{
MutexLocker ml(THREAD, InvokeMethodTable_lock);
o = _invoke_method_type_table.get(signature);
}

if (o != nullptr) {
oop mt = o->resolve();
assert(java_lang_invoke_MethodType::is_instance(mt), "");
return Handle(THREAD, mt);
} else if (!THREAD->can_call_java()) {
warning("SystemDictionary::find_method_handle_type called from compiler thread"); // FIXME
return Handle(); // do not attempt from within compiler, unless it was cached
Expand Down Expand Up @@ -2244,15 +2284,17 @@ Handle SystemDictionary::find_method_handle_type(Symbol* signature,

if (can_be_cached) {
// We can cache this MethodType inside the JVM.
MutexLocker ml(THREAD, SystemDictionary_lock);
spe = invoke_method_table()->find_entry(index, hash, signature, null_iid);
if (spe == NULL)
spe = invoke_method_table()->add_entry(index, hash, signature, null_iid);
if (spe->method_type() == NULL) {
spe->set_method_type(method_type());
MutexLocker ml(THREAD, InvokeMethodTable_lock);
bool created = false;
assert(method_type != NULL, "unexpected null");
OopHandle* h = _invoke_method_type_table.get(signature);
if (h == nullptr) {
signature->make_permanent(); // The signature is never unloaded.
OopHandle elem = OopHandle(Universe::vm_global(), method_type());
bool created = _invoke_method_type_table.put(signature, elem);
assert(created, "better be created");
}
}

// report back to the caller with the MethodType
return method_type;
}
Expand Down

1 comment on commit 13f0f12

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Issues

Please sign in to comment.