8290370: Convert SymbolPropertyTable to ResourceHashtable

Reviewed-by: coleenp, iklam

Author: gujustin1

src/hotspot/share/classfile/dictionary.cpp

@@ -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

src/hotspot/share/classfile/dictionary.hpp

@@ -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

src/hotspot/share/classfile/systemDictionary.cpp

@@ -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"
@@ -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;
@@ -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);
+  }
+
 }
 
 // ----------------------------------------------------------------------------
@@ -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
@@ -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.
@@ -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
@@ -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;
 }