8292383: Create a SymbolHandle type to use for ResourceHashtable

Reviewed-by: iklam, hseigel

src/hotspot/share/cds/cdsProtectionDomain.cpp

@@ -28,7 +28,6 @@
 #include "classfile/classLoaderExt.hpp"
 #include "classfile/javaClasses.hpp"
 #include "classfile/moduleEntry.hpp"
-#include "classfile/symbolTable.hpp"
 #include "classfile/systemDictionaryShared.hpp"
 #include "classfile/vmClasses.hpp"
 #include "classfile/vmSymbols.hpp"

src/hotspot/share/ci/ciEnv.cpp

@@ -36,7 +36,6 @@
 #include "ci/ciUtilities.inline.hpp"
 #include "classfile/javaClasses.hpp"
 #include "classfile/javaClasses.inline.hpp"
-#include "classfile/symbolTable.hpp"
 #include "classfile/systemDictionary.hpp"
 #include "classfile/vmClasses.hpp"
 #include "classfile/vmSymbols.hpp"
@@ -65,6 +64,7 @@
 #include "oops/objArrayKlass.hpp"
 #include "oops/objArrayOop.inline.hpp"
 #include "oops/oop.inline.hpp"
+#include "oops/symbolHandle.hpp"
 #include "prims/jvmtiExport.hpp"
 #include "prims/methodHandles.hpp"
 #include "runtime/fieldDescriptor.inline.hpp"

src/hotspot/share/classfile/classFileParser.hpp

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 1997, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 1997, 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
@@ -47,7 +47,6 @@ class GrowableArray;
 class InstanceKlass;
 class RecordComponent;
 class Symbol;
-class TempNewSymbol;
 class FieldLayoutBuilder;
 
 // Utility to collect and compact oop maps during layout

src/hotspot/share/classfile/loaderConstraints.cpp

@@ -32,6 +32,7 @@
 #include "memory/resourceArea.hpp"
 #include "oops/klass.inline.hpp"
 #include "oops/oop.inline.hpp"
+#include "oops/symbolHandle.hpp"
 #include "runtime/handles.inline.hpp"
 #include "runtime/mutexLocker.hpp"
 #include "runtime/safepoint.hpp"
@@ -110,7 +111,7 @@ class ConstraintSet {                               // copied into hashtable as
 };
 
 
-ResourceHashtable<Symbol*, ConstraintSet, 107, ResourceObj::C_HEAP, mtClass> _loader_constraint_table;
+ResourceHashtable<SymbolHandle, ConstraintSet, 107, ResourceObj::C_HEAP, mtClass, SymbolHandle::compute_hash> _loader_constraint_table;
 
 void LoaderConstraint::extend_loader_constraint(Symbol* class_name,
                                                 Handle loader,
@@ -171,8 +172,6 @@ void LoaderConstraintTable::add_loader_constraint(Symbol* name, InstanceKlass* k
   bool created;
   ConstraintSet* set = _loader_constraint_table.put_if_absent(name, &created);
   if (created) {
-    // Increment the key refcount when putting in the table.
-    name->increment_refcount();
     set->initialize(constraint);
   } else {
     set->add_constraint(constraint);
@@ -181,7 +180,7 @@ void LoaderConstraintTable::add_loader_constraint(Symbol* name, InstanceKlass* k
 
 class PurgeUnloadedConstraints : public StackObj {
  public:
-  bool do_entry(Symbol*& name, ConstraintSet& set) {
+  bool do_entry(SymbolHandle& name, ConstraintSet& set) {
     LogTarget(Info, class, loader, constraints) lt;
     int len = set.num_constraints();
     for (int i = len - 1; i >= 0; i--) {
@@ -242,8 +241,6 @@ class PurgeUnloadedConstraints : public StackObj {
       }
     }
     if (set.num_constraints() == 0) {
-      // decrement name refcount before freeing
-      name->decrement_refcount();
       return true;
     }
     // Don't unlink this set
@@ -455,14 +452,14 @@ void LoaderConstraintTable::merge_loader_constraints(Symbol* class_name,
 
 void LoaderConstraintTable::verify() {
   Thread* thread = Thread::current();
-  auto check = [&] (Symbol*& key, ConstraintSet& set) {
+  auto check = [&] (SymbolHandle& key, ConstraintSet& set) {
     // foreach constraint in the set, check the klass is in the dictionary or placeholder table.
     int len = set.num_constraints();
     for (int i = 0; i < len; i++) {
       LoaderConstraint* probe = set.constraint_at(i);
       if (probe->klass() != NULL) {
         InstanceKlass* ik = probe->klass();
-        guarantee(ik->name() == key, "name should match");
+        guarantee(key == ik->name(), "name should match");
         Symbol* name = ik->name();
         ClassLoaderData* loader_data = ik->class_loader_data();
         Dictionary* dictionary = loader_data->dictionary();
@@ -492,7 +489,7 @@ void LoaderConstraintTable::verify() {
 }
 
 void LoaderConstraintTable::print_table_statistics(outputStream* st) {
-  auto size = [&] (Symbol*& key, ConstraintSet& set) {
+  auto size = [&] (SymbolHandle& key, ConstraintSet& set) {
     // sizeof set is included in the size of the hashtable node
     int sum = 0;
     int len = set.num_constraints();
@@ -508,7 +505,7 @@ void LoaderConstraintTable::print_table_statistics(outputStream* st) {
 
 // Called with the system dictionary lock held
 void LoaderConstraintTable::print_on(outputStream* st) {
-  auto printer = [&] (Symbol*& key, ConstraintSet& set) {
+  auto printer = [&] (SymbolHandle& key, ConstraintSet& set) {
     int len = set.num_constraints();
     for (int i = 0; i < len; i++) {
       LoaderConstraint* probe = set.constraint_at(i);

src/hotspot/share/classfile/moduleEntry.cpp

@@ -355,7 +355,7 @@ ModuleEntryTable::ModuleEntryTable() { }
 ModuleEntryTable::~ModuleEntryTable() {
   class ModuleEntryTableDeleter : public StackObj {
    public:
-    bool do_entry(const Symbol*& name, ModuleEntry*& entry) {
+    bool do_entry(const SymbolHandle& name, ModuleEntry*& entry) {
       if (log_is_enabled(Info, module, unload) || log_is_enabled(Debug, module)) {
         ResourceMark rm;
         const char* str = name->as_C_string();
@@ -511,7 +511,7 @@ static int compare_module_by_name(ModuleEntry* a, ModuleEntry* b) {
 }
 
 void ModuleEntryTable::iterate_symbols(MetaspaceClosure* closure) {
-  auto syms = [&] (const Symbol*& key, ModuleEntry*& m) {
+  auto syms = [&] (const SymbolHandle& key, ModuleEntry*& m) {
       m->iterate_symbols(closure);
   };
   _table.iterate_all(syms);
@@ -520,7 +520,7 @@ void ModuleEntryTable::iterate_symbols(MetaspaceClosure* closure) {
 Array<ModuleEntry*>* ModuleEntryTable::allocate_archived_entries() {
   Array<ModuleEntry*>* archived_modules = ArchiveBuilder::new_rw_array<ModuleEntry*>(_table.number_of_entries());
   int n = 0;
-  auto grab = [&] (const Symbol*& key, ModuleEntry*& m) {
+  auto grab = [&] (const SymbolHandle& key, ModuleEntry*& m) {
     archived_modules->at_put(n++, m);
   };
   _table.iterate_all(grab);
@@ -603,7 +603,7 @@ ModuleEntry* ModuleEntryTable::lookup_only(Symbol* name) {
 // This should only occur at class unloading.
 void ModuleEntryTable::purge_all_module_reads() {
   assert_locked_or_safepoint(Module_lock);
-  auto purge = [&] (const Symbol*& key, ModuleEntry*& entry) {
+  auto purge = [&] (const SymbolHandle& key, ModuleEntry*& entry) {
     entry->purge_reads();
   };
   _table.iterate_all(purge);
@@ -670,7 +670,7 @@ void ModuleEntryTable::patch_javabase_entries(Handle module_handle) {
 
 void ModuleEntryTable::print(outputStream* st) {
   ResourceMark rm;
-  auto printer = [&] (const Symbol*& name, ModuleEntry*& entry) {
+  auto printer = [&] (const SymbolHandle& name, ModuleEntry*& entry) {
     entry->print(st);
   };
   st->print_cr("Module Entry Table (table_size=%d, entries=%d)",
@@ -680,14 +680,14 @@ void ModuleEntryTable::print(outputStream* st) {
 }
 
 void ModuleEntryTable::modules_do(void f(ModuleEntry*)) {
-  auto do_f = [&] (const Symbol*& key, ModuleEntry*& entry) {
+  auto do_f = [&] (const SymbolHandle& key, ModuleEntry*& entry) {
     f(entry);
   };
   _table.iterate_all(do_f);
 }
 
 void ModuleEntryTable::modules_do(ModuleClosure* closure) {
-  auto do_f = [&] (const Symbol*& key, ModuleEntry*& entry) {
+  auto do_f = [&] (const SymbolHandle& key, ModuleEntry*& entry) {
     closure->do_module(entry);
   };
   _table.iterate_all(do_f);
@@ -705,7 +705,7 @@ void ModuleEntry::print(outputStream* st) {
 }
 
 void ModuleEntryTable::verify() {
-  auto do_f = [&] (const Symbol*& key, ModuleEntry*& entry) {
+  auto do_f = [&] (const SymbolHandle& key, ModuleEntry*& entry) {
     entry->verify();
   };
   assert_locked_or_safepoint(Module_lock);

src/hotspot/share/classfile/moduleEntry.hpp

@@ -28,6 +28,7 @@
 #include "jni.h"
 #include "oops/oopHandle.hpp"
 #include "oops/symbol.hpp"
+#include "oops/symbolHandle.hpp"
 #include "runtime/mutexLocker.hpp"
 #include "utilities/growableArray.hpp"
 #include "utilities/macros.hpp"
@@ -205,8 +206,8 @@ class ModuleClosure: public StackObj {
 class ModuleEntryTable : public CHeapObj<mtModule> {
 private:
   static ModuleEntry* _javabase_module;
-  ResourceHashtable<const Symbol*, ModuleEntry*, 109, ResourceObj::C_HEAP, mtModule,
-                    Symbol::compute_hash> _table;
+  ResourceHashtable<SymbolHandle, ModuleEntry*, 109, ResourceObj::C_HEAP, mtModule,
+                    SymbolHandle::compute_hash> _table;
 
 public:
   ModuleEntryTable();

src/hotspot/share/classfile/packageEntry.cpp

@@ -193,7 +193,7 @@ PackageEntryTable::PackageEntryTable() { }
 PackageEntryTable::~PackageEntryTable() {
   class PackageEntryTableDeleter : public StackObj {
    public:
-    bool do_entry(const Symbol*& name, PackageEntry*& entry) {
+    bool do_entry(const SymbolHandle& name, PackageEntry*& entry) {
       if (log_is_enabled(Info, module, unload) || log_is_enabled(Debug, module)) {
         ResourceMark rm;
         const char* str = name->as_C_string();
@@ -270,7 +270,7 @@ static int compare_package_by_name(PackageEntry* a, PackageEntry* b) {
 }
 
 void PackageEntryTable::iterate_symbols(MetaspaceClosure* closure) {
-  auto syms = [&] (const Symbol*& key, PackageEntry*& p) {
+  auto syms = [&] (const SymbolHandle& key, PackageEntry*& p) {
       p->iterate_symbols(closure);
   };
   _table.iterate_all(syms);
@@ -279,7 +279,7 @@ void PackageEntryTable::iterate_symbols(MetaspaceClosure* closure) {
 Array<PackageEntry*>* PackageEntryTable::allocate_archived_entries() {
   // First count the packages in named modules
   int n = 0;
-  auto count = [&] (const Symbol*& key, PackageEntry*& p) {
+  auto count = [&] (const SymbolHandle& key, PackageEntry*& p) {
     if (p->module()->is_named()) {
       n++;
     }
@@ -289,7 +289,7 @@ Array<PackageEntry*>* PackageEntryTable::allocate_archived_entries() {
   Array<PackageEntry*>* archived_packages = ArchiveBuilder::new_rw_array<PackageEntry*>(n);
   // reset n
   n = 0;
-  auto grab = [&] (const Symbol*& key, PackageEntry*& p) {
+  auto grab = [&] (const SymbolHandle& key, PackageEntry*& p) {
     if (p->module()->is_named()) {
       // We don't archive unnamed modules, or packages in unnamed modules. They will be
       // created on-demand at runtime as classes in such packages are loaded.
@@ -374,7 +374,7 @@ PackageEntry* PackageEntryTable::locked_lookup_only(Symbol* name) {
 // Verify the packages loaded thus far are in java.base's package list.
 void PackageEntryTable::verify_javabase_packages(GrowableArray<Symbol*> *pkg_list) {
   assert_lock_strong(Module_lock);
-  auto verifier = [&] (const Symbol*& name, PackageEntry*& entry) {
+  auto verifier = [&] (const SymbolHandle& name, PackageEntry*& entry) {
     ModuleEntry* m = entry->module();
     Symbol* module_name = (m == NULL ? NULL : m->name());
     if (module_name != NULL &&
@@ -410,7 +410,7 @@ bool PackageEntry::exported_pending_delete() const {
 // Remove dead entries from all packages' exported list
 void PackageEntryTable::purge_all_package_exports() {
   assert_locked_or_safepoint(Module_lock);
-  auto purge = [&] (const Symbol*& name, PackageEntry*& entry) {
+  auto purge = [&] (const SymbolHandle& name, PackageEntry*& entry) {
     if (entry->exported_pending_delete()) {
       // exported list is pending deletion due to a transition
       // from qualified to unqualified
@@ -423,7 +423,7 @@ void PackageEntryTable::purge_all_package_exports() {
 }
 
 void PackageEntryTable::packages_do(void f(PackageEntry*)) {
-  auto doit = [&] (const Symbol*&name, PackageEntry*& entry) {
+  auto doit = [&] (const SymbolHandle&name, PackageEntry*& entry) {
     f(entry);
   };
   assert_locked_or_safepoint(Module_lock);
@@ -433,7 +433,7 @@ void PackageEntryTable::packages_do(void f(PackageEntry*)) {
 
 GrowableArray<PackageEntry*>*  PackageEntryTable::get_system_packages() {
   GrowableArray<PackageEntry*>* loaded_class_pkgs = new GrowableArray<PackageEntry*>(50);
-  auto grab = [&] (const Symbol*&name, PackageEntry*& entry) {
+  auto grab = [&] (const SymbolHandle& name, PackageEntry*& entry) {
     if (entry->has_loaded_class()) {
       loaded_class_pkgs->append(entry);
     }
@@ -446,7 +446,7 @@ GrowableArray<PackageEntry*>*  PackageEntryTable::get_system_packages() {
 }
 
 void PackageEntryTable::print(outputStream* st) {
-  auto printer = [&] (const Symbol*& name, PackageEntry*& entry) {
+  auto printer = [&] (const SymbolHandle& name, PackageEntry*& entry) {
     entry->print(st);
   };
   st->print_cr("Package Entry Table (table_size=%d, entries=%d)",

src/hotspot/share/classfile/packageEntry.hpp

@@ -27,6 +27,7 @@
 
 #include "classfile/moduleEntry.hpp"
 #include "oops/symbol.hpp"
+#include "oops/symbolHandle.hpp"
 #include "runtime/atomic.hpp"
 #include "utilities/growableArray.hpp"
 #include "utilities/resourceHash.hpp"
@@ -236,8 +237,8 @@ class PackageEntry : public CHeapObj<mtModule> {
 // The PackageEntryTable is a Hashtable containing a list of all packages defined
 // by a particular class loader.  Each package is represented as a PackageEntry node.
 class PackageEntryTable : public CHeapObj<mtModule> {
-  ResourceHashtable<const Symbol*, PackageEntry*, 109, ResourceObj::C_HEAP, mtModule,
-                    Symbol::compute_hash> _table;
+  ResourceHashtable<SymbolHandle, PackageEntry*, 109, ResourceObj::C_HEAP, mtModule,
+                    SymbolHandle::compute_hash> _table;
 public:
   PackageEntryTable();
   ~PackageEntryTable();

src/hotspot/share/classfile/placeholders.cpp

@@ -29,12 +29,13 @@
 #include "logging/logTag.hpp"
 #include "logging/logStream.hpp"
 #include "memory/resourceArea.hpp"
+#include "oops/symbolHandle.hpp"
 #include "runtime/javaThread.hpp"
 #include "runtime/mutexLocker.hpp"
 #include "utilities/resourceHash.hpp"
 
 class PlaceholderKey {
-  Symbol* _name;
+  SymbolHandle _name;
   ClassLoaderData* _loader_data;
  public:
   PlaceholderKey(Symbol* name, ClassLoaderData* l) : _name(name), _loader_data(l) {}
@@ -204,8 +205,6 @@ PlaceholderEntry* add_entry(Symbol* class_name, ClassLoaderData* loader_data,
   PlaceholderEntry entry;
   entry.set_supername(supername);
   PlaceholderKey key(class_name, loader_data);
-  // Since we're storing this key in the hashtable, we need to increment the refcount.
-  class_name->increment_refcount();
   bool created;
   PlaceholderEntry* table_copy = _placeholders.put_if_absent(key, entry, &created);
   assert(created, "better be absent");
@@ -218,8 +217,6 @@ void remove_entry(Symbol* class_name, ClassLoaderData* loader_data) {
 
   PlaceholderKey key(class_name, loader_data);
   _placeholders.remove(key);
-  // Decrement the refcount in key, since it's no longer in the table.
-  class_name->decrement_refcount();
 }