RFR: 8292068: Convert ModuleEntryTable into ResourceHashtable

[Ioi Lam]

On Tue, 9 Aug 2022 12:27:46 GMT, Coleen Phillimore <coleenp at openjdk.org> wrote:

> Converted the ModuleEntryTable to use ResourceHashtable.
> Ran tiers 1-7.  Also ran performance tests with this and other changes with no performance difference, although the footprint of this should be better.

Changes requested by iklam (Reviewer).

src/hotspot/share/classfile/moduleEntry.cpp line 287:

> 285:   }
> 286: 
> 287:   Symbol::maybe_increment_refcount(_version);

Use set_version() instead, to be consistent with set_location() below.

src/hotspot/share/classfile/moduleEntry.cpp line 359:

> 357:  public:
> 358:   bool do_entry(const Symbol*& name, ModuleEntry*& entry) {
> 359:     ResourceMark rm;

The ResourceMark should be used inside `log_is_enabled(...) `

src/hotspot/share/classfile/moduleEntry.cpp line 595:

> 593: // lookup_only by Symbol* to find a ModuleEntry.
> 594: ModuleEntry* ModuleEntryTable::lookup_only(Symbol* name) {
> 595:   assert_locked_or_safepoint(Module_lock);

Is it true that all public functions in the ModuleEntryTable class require the Module_lock to be held? If so, maybe this should be documented in the class declaration, and the assert to be added to all the functions.

src/hotspot/share/classfile/moduleEntry.cpp line 681:

> 679: 
> 680: void ModuleEntryTable::modules_do(void f(ModuleEntry*)) {
> 681:   auto do_f = [&] (const Symbol*& key, ModuleEntry*& entry) {

assert locked?

src/hotspot/share/classfile/moduleEntry.cpp line 688:

> 686: 
> 687: void ModuleEntryTable::modules_do(ModuleClosure* closure) {
> 688:   auto do_f = [&] (const Symbol*& key, ModuleEntry*& entry) {

assert locked?

src/hotspot/share/classfile/moduleEntry.hpp line 209:

> 207:   static ModuleEntry* _javabase_module;
> 208:   ResourceHashtable<const Symbol*, ModuleEntry*, 109, ResourceObj::C_HEAP, mtModule,
> 209:                     Symbol::compute_hash, Symbol::compare> _table;

Is the `Symbol::compare` necessary? The `_invoke_method_type_table` table in systemDictionary.hpp is able to use the default comparison method.

-------------

PR: https://git.openjdk.org/jdk/pull/9809