8292375: Convert ProtectionDomainCacheTable to ResourceHashtable

Reviewed-by: dholmes, iklam

Author: coleenp

src/hotspot/share/classfile/dictionary.cpp

@@ -166,9 +166,9 @@ bool DictionaryEntry::contains_protection_domain(oop protection_domain) const {
 void DictionaryEntry::add_protection_domain(ClassLoaderData* loader_data, Handle protection_domain) {
   assert_lock_strong(SystemDictionary_lock);
   if (!contains_protection_domain(protection_domain())) {
-    ProtectionDomainCacheEntry* entry = SystemDictionary::pd_cache_table()->get(protection_domain);
+    WeakHandle obj = ProtectionDomainCacheTable::add_if_absent(protection_domain);
     // Additions and deletions hold the SystemDictionary_lock, readers are lock-free
-    ProtectionDomainEntry* new_head = new ProtectionDomainEntry(entry, _pd_set);
+    ProtectionDomainEntry* new_head = new ProtectionDomainEntry(obj, _pd_set);
     release_set_pd_set(new_head);
   }
   LogTarget(Trace, protectiondomain) lt;

src/hotspot/share/classfile/protectionDomainCache.cpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -35,23 +35,28 @@
 #include "oops/oop.inline.hpp"
 #include "oops/weakHandle.inline.hpp"
 #include "runtime/atomic.hpp"
+#include "runtime/mutexLocker.hpp"
 #include "utilities/growableArray.hpp"
-#include "utilities/hashtable.inline.hpp"
+#include "utilities/resourceHash.hpp"
 
-unsigned int ProtectionDomainCacheTable::compute_hash(Handle protection_domain) {
-  // Identity hash can safepoint, so keep protection domain in a Handle.
-  return (unsigned int)(protection_domain->identity_hash());
+unsigned int ProtectionDomainCacheTable::compute_hash(const WeakHandle& protection_domain) {
+  // The protection domain in the hash computation is passed from a Handle so cannot resolve to null.
+  assert(protection_domain.peek() != nullptr, "Must be live");
+  return (unsigned int)(protection_domain.resolve()->identity_hash());
 }
 
-int ProtectionDomainCacheTable::index_for(Handle protection_domain) {
-  return hash_to_index(compute_hash(protection_domain));
+bool ProtectionDomainCacheTable::equals(const WeakHandle& protection_domain1, const WeakHandle& protection_domain2) {
+  return protection_domain1.peek() == protection_domain2.peek();
 }
 
-ProtectionDomainCacheTable::ProtectionDomainCacheTable(int table_size)
-  : Hashtable<WeakHandle, mtClass>(table_size, sizeof(ProtectionDomainCacheEntry))
-{   _dead_entries = false;
-    _total_oops_removed = 0;
-}
+// WeakHandle is both the key and the value.  We need it as the key to compare the oops that each point to
+// for equality.  We need it as the value to return the one that already exists to link in the DictionaryEntry.
+ResourceHashtable<WeakHandle, WeakHandle, 1009, ResourceObj::C_HEAP, mtClass,
+                  ProtectionDomainCacheTable::compute_hash,
+                  ProtectionDomainCacheTable::equals> _pd_cache_table;
+
+bool ProtectionDomainCacheTable::_dead_entries = false;
+int  ProtectionDomainCacheTable::_total_oops_removed = 0;
 
 void ProtectionDomainCacheTable::trigger_cleanup() {
   MutexLocker ml(Service_lock, Mutex::_no_safepoint_check_flag);
@@ -129,107 +134,91 @@ void ProtectionDomainCacheTable::unlink() {
   // Purge any deleted entries outside of the SystemDictionary_lock.
   purge_deleted_entries();
 
+  // Reacquire the lock to remove entries from the hashtable.
   MutexLocker ml(SystemDictionary_lock);
-  int oops_removed = 0;
-  for (int i = 0; i < table_size(); ++i) {
-    ProtectionDomainCacheEntry** p = bucket_addr(i);
-    ProtectionDomainCacheEntry* entry = bucket(i);
-    while (entry != NULL) {
-      oop pd = entry->object_no_keepalive();
-      if (pd != NULL) {
-        p = entry->next_addr();
-      } else {
-        oops_removed++;
+
+  struct Deleter {
+    int _oops_removed;
+    Deleter() : _oops_removed(0) {}
+
+    bool do_entry(WeakHandle& key, WeakHandle& value) {
+      oop pd = value.peek();
+      if (value.peek() == nullptr) {
+        _oops_removed++;
         LogTarget(Debug, protectiondomain, table) lt;
         if (lt.is_enabled()) {
           LogStream ls(lt);
-          ls.print_cr("protection domain unlinked at %d", i);
+          ls.print_cr("protection domain unlinked %d", _oops_removed);
         }
-        entry->literal().release(Universe::vm_weak());
-        *p = entry->next();
-        free_entry(entry);
+        value.release(Universe::vm_weak());
+        return true;
+      } else {
+        return false;
       }
-      entry = *p;
     }
-  }
-  _total_oops_removed += oops_removed;
+  };
+
+  Deleter deleter;
+  _pd_cache_table.unlink(&deleter);
+
+  _total_oops_removed += deleter._oops_removed;
   _dead_entries = false;
 }
 
-void ProtectionDomainCacheTable::print_on(outputStream* st) const {
+void ProtectionDomainCacheTable::print_on(outputStream* st) {
   assert_locked_or_safepoint(SystemDictionary_lock);
-  st->print_cr("Protection domain cache table (table_size=%d, classes=%d)",
-               table_size(), number_of_entries());
-  for (int index = 0; index < table_size(); index++) {
-    for (ProtectionDomainCacheEntry* probe = bucket(index);
-                                     probe != NULL;
-                                     probe = probe->next()) {
-      st->print_cr("%4d: protection_domain: " PTR_FORMAT, index, p2i(probe->object_no_keepalive()));
-    }
-  }
+  auto printer = [&] (WeakHandle& key, WeakHandle& value) {
+      st->print_cr("  protection_domain: " PTR_FORMAT, p2i(value.peek()));
+  };
+  st->print_cr("Protection domain cache table (table_size=%d, protection domains=%d)",
+                _pd_cache_table.table_size(), _pd_cache_table.number_of_entries());
+  _pd_cache_table.iterate_all(printer);
 }
 
 void ProtectionDomainCacheTable::verify() {
-  verify_table<ProtectionDomainCacheEntry>("Protection Domain Table");
-}
-
-oop ProtectionDomainCacheEntry::object() {
-  return literal().resolve();
+  auto verifier = [&] (WeakHandle& key, WeakHandle& value) {
+    guarantee(value.peek() == nullptr || oopDesc::is_oop(value.peek()), "must be an oop");
+  };
+  _pd_cache_table.iterate_all(verifier);
 }
 
 // The object_no_keepalive() call peeks at the phantomly reachable oop without
-// keeping it alive. This is okay to do in the VM thread state if it is not
-// leaked out to become strongly reachable.
-oop ProtectionDomainCacheEntry::object_no_keepalive() {
-  return literal().peek();
-}
-
+// keeping it alive.  This is used for traversing DictionaryEntry pd_set.
 oop ProtectionDomainEntry::object_no_keepalive() {
-  return _pd_cache->object_no_keepalive();
-}
-
-void ProtectionDomainCacheEntry::verify() {
-  guarantee(object_no_keepalive() == NULL || oopDesc::is_oop(object_no_keepalive()), "must be an oop");
+  return _object.peek();
 }
 
-ProtectionDomainCacheEntry* ProtectionDomainCacheTable::get(Handle protection_domain) {
-  unsigned int hash = compute_hash(protection_domain);
-  int index = hash_to_index(hash);
-
-  ProtectionDomainCacheEntry* entry = find_entry(index, protection_domain);
-  if (entry == NULL) {
-    entry = add_entry(index, hash, protection_domain);
-  }
-  // keep entry alive
-  (void)entry->object();
-  return entry;
-}
-
-ProtectionDomainCacheEntry* ProtectionDomainCacheTable::find_entry(int index, Handle protection_domain) {
+WeakHandle ProtectionDomainCacheTable::add_if_absent(Handle protection_domain) {
   assert_locked_or_safepoint(SystemDictionary_lock);
-  for (ProtectionDomainCacheEntry* e = bucket(index); e != NULL; e = e->next()) {
-    if (e->object_no_keepalive() == protection_domain()) {
-      return e;
+  WeakHandle w(Universe::vm_weak(), protection_domain);
+  bool created;
+  WeakHandle* wk = _pd_cache_table.put_if_absent(w, w, &created);
+  if (!created) {
+    // delete the one created since we already had it in the table
+    w.release(Universe::vm_weak());
+  } else {
+    LogTarget(Debug, protectiondomain, table) lt;
+    if (lt.is_enabled()) {
+      LogStream ls(lt);
+      ls.print("protection domain added ");
+      protection_domain->print_value_on(&ls);
+      ls.cr();
     }
   }
+  // Keep entry alive
+  (void)wk->resolve();
+  return *wk;
+}
 
-  return NULL;
+void ProtectionDomainCacheTable::print_table_statistics(outputStream* st) {
+  auto size = [&] (WeakHandle& key, WeakHandle& value) {
+    // The only storage is in OopStorage for an oop
+    return sizeof(oop);
+  };
+  TableStatistics ts = _pd_cache_table.statistics_calculate(size);
+  ts.print(st, "ProtectionDomainCacheTable");
 }
 
-ProtectionDomainCacheEntry* ProtectionDomainCacheTable::add_entry(int index, unsigned int hash, Handle protection_domain) {
-  assert_locked_or_safepoint(SystemDictionary_lock);
-  assert(index == index_for(protection_domain), "incorrect index?");
-  assert(find_entry(index, protection_domain) == NULL, "no double entry");
-
-  LogTarget(Debug, protectiondomain, table) lt;
-  if (lt.is_enabled()) {
-    LogStream ls(lt);
-    ls.print("protection domain added ");
-    protection_domain->print_value_on(&ls);
-    ls.cr();
-  }
-  WeakHandle w(Universe::vm_weak(), protection_domain);
-  ProtectionDomainCacheEntry* p = new_entry(hash, w);
-  Hashtable<WeakHandle, mtClass>::add_entry(index, p);
-  return p;
+int ProtectionDomainCacheTable::number_of_entries() {
+  return _pd_cache_table.number_of_entries();
 }

src/hotspot/share/classfile/protectionDomainCache.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2017, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2017, 2022, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -28,88 +28,43 @@
 #include "oops/oop.hpp"
 #include "oops/weakHandle.hpp"
 #include "runtime/atomic.hpp"
-#include "utilities/hashtable.hpp"
 
-// This class caches the approved protection domains that can access loaded classes.
-// Dictionary entry pd_set point to entries in this hashtable.   Please refer
-// to dictionary.hpp pd_set for more information about how protection domain entries
-// are used.
-// This table is walked during GC, rather than the class loader data graph dictionaries.
-class ProtectionDomainCacheEntry : public HashtableEntry<WeakHandle, mtClass> {
-  friend class VMStructs;
- public:
-  oop object();
-  oop object_no_keepalive();
-
-  ProtectionDomainCacheEntry* next() {
-    return (ProtectionDomainCacheEntry*)HashtableEntry<WeakHandle, mtClass>::next();
-  }
-
-  ProtectionDomainCacheEntry** next_addr() {
-    return (ProtectionDomainCacheEntry**)HashtableEntry<WeakHandle, mtClass>::next_addr();
-  }
-
-  void verify();
-};
-
-// The ProtectionDomainCacheTable contains all protection domain oops. The
-// dictionary entries reference its entries instead of having references to oops
-// directly.
-// This is used to speed up system dictionary iteration: the oops in the
-// protection domain are the only ones referring the Java heap. So when there is
-// need to update these, instead of going over every entry of the system dictionary,
-// we only need to iterate over this set.
+// The ProtectionDomainCacheTable maps all java.security.ProtectionDomain objects that are
+// registered by DictionaryEntry::add_protection_domain() to a unique WeakHandle.
 // The amount of different protection domains used is typically magnitudes smaller
 // than the number of system dictionary entries (loaded classes).
-class ProtectionDomainCacheTable : public Hashtable<WeakHandle, mtClass> {
-  ProtectionDomainCacheEntry* bucket(int i) const {
-    return (ProtectionDomainCacheEntry*) Hashtable<WeakHandle, mtClass>::bucket(i);
-  }
-
-  // The following method is not MT-safe and must be done under lock.
-  ProtectionDomainCacheEntry** bucket_addr(int i) {
-    return (ProtectionDomainCacheEntry**) Hashtable<WeakHandle, mtClass>::bucket_addr(i);
-  }
-
-  ProtectionDomainCacheEntry* new_entry(unsigned int hash, WeakHandle protection_domain) {
-    ProtectionDomainCacheEntry* entry = (ProtectionDomainCacheEntry*)
-      Hashtable<WeakHandle, mtClass>::new_entry(hash, protection_domain);
-    return entry;
-  }
-
-  static unsigned int compute_hash(Handle protection_domain);
-
-  int index_for(Handle protection_domain);
-  ProtectionDomainCacheEntry* add_entry(int index, unsigned int hash, Handle protection_domain);
-  ProtectionDomainCacheEntry* find_entry(int index, Handle protection_domain);
+class ProtectionDomainCacheTable : public AllStatic {
 
-  bool _dead_entries;
-  int _total_oops_removed;
+  static bool _dead_entries;
+  static int _total_oops_removed;
 
 public:
-  ProtectionDomainCacheTable(int table_size);
-  ProtectionDomainCacheEntry* get(Handle protection_domain);
+  static unsigned int compute_hash(const WeakHandle& protection_domain);
+  static bool equals(const WeakHandle& protection_domain1, const WeakHandle& protection_domain2);
 
-  void unlink();
+  static WeakHandle add_if_absent(Handle protection_domain);
+  static void unlink();
 
-  void print_on(outputStream* st) const;
-  void verify();
+  static void print_on(outputStream* st);
+  static void verify();
 
-  bool has_work() { return _dead_entries; }
-  void trigger_cleanup();
+  static bool has_work() { return _dead_entries; }
+  static void trigger_cleanup();
 
-  int removed_entries_count() { return _total_oops_removed; };
+  static int removed_entries_count() { return _total_oops_removed; };
+  static int number_of_entries();
+  static void print_table_statistics(outputStream* st);
 };
 
 
 // This describes the linked list protection domain for each DictionaryEntry in pd_set.
 class ProtectionDomainEntry :public CHeapObj<mtClass> {
-  ProtectionDomainCacheEntry* _pd_cache;
+  WeakHandle _object;
   ProtectionDomainEntry* volatile _next;
  public:
 
-  ProtectionDomainEntry(ProtectionDomainCacheEntry* pd_cache,
-                        ProtectionDomainEntry* head) : _pd_cache(pd_cache), _next(head) {}
+  ProtectionDomainEntry(WeakHandle obj,
+                        ProtectionDomainEntry* head) : _object(obj), _next(head) {}
 
   ProtectionDomainEntry* next_acquire() { return Atomic::load_acquire(&_next); }
   void release_set_next(ProtectionDomainEntry* entry) { Atomic::release_store(&_next, entry); }