RFR: 8298601: Refactor archiving of java.lang.Module objects

[Coleen Phillimore]

On Mon, 19 Dec 2022 05:28:39 GMT, Ioi Lam <iklam at openjdk.org> wrote:

> This RFE is a prerequisite for [JDK-8296344: Remove dependency on G1 for writing the CDS archive heap](https://bugs.openjdk.org/browse/JDK-8296344). 
> 
> The main change of this RFE is in `HeapShared::archive_reachable_objects_from()`. We avoid the use of "archived objects" in the interface between CDS and the module system, and use integer "root indices" instead. (See note [1] in [JDK-8298600](https://bugs.openjdk.org/browse/JDK-8298600).)
> 
> Also, for better encapsulation, the module-specific code has been moved from `HeapShared::check_module_oop()` to the three new functions in modules.cpp.
> 
> ----
> 
> Besides the refactoring, this RFE also tightens up the code that deals with `java.lang.Module` oops and the corresponding C++ `ModuleEntry` objects.
> 
> As this is admittedly an obscured area of the CDS archive heap, I've added the following:
> 
> - Assertions to make sure that when an oop is archived, the corresponding C++ object is also archived, and vice-versa.
> - More comments

I suggested some name changes that might be helpful to people who don't already know this code, but the movement seems fine.

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

> 501: }
> 502: 
> 503: char* ModuleEntry::debug_info() const {

Isn't this just print_on?  or print_string, although print_on(outputStream*) would be more consistent with other metadata.

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

> 521: 
> 522: #ifndef PRODUCT
> 523: void ModuleEntry::validate_archived_module_entries() {

Can this have a check_module_entries name since it's called by a similarly sounding function name in Modules?  It doesn't actually validate anything in the module entries except the count.

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

> 550:   if (log_is_enabled(Info, cds, module)) {
> 551:     ResourceMark rm;
> 552:     log_info(cds, module)("Restored archived %s", debug_info());

I don't know if stringStream needs a resource mark so if it doesn't, you can just have one log line for these.

src/hotspot/share/classfile/modules.cpp line 492:

> 490: 
> 491:   ModuleEntry* orig_module_ent = java_lang_Module::module_entry_raw(orig_module_obj);
> 492:   if (orig_module_ent == NULL) {

use nullptr in new code.

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

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