RFR: 8316969: Improve CDS module graph support for --module option [v6]

[Calvin Cheung]

On Fri, 27 Oct 2023 16:28:10 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> Calvin Cheung has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains ten commits:
>> 
>>  - Merge master
>>  - skip archiving full module graph is there is an incubator module
>>  - fix typo
>>  - simplify some code in modules.cpp
>>  - comments from Alan and Ioi; add missing @run tag in the test
>>  - Merge branch 'master' into improve-CDS-module-graph
>>  - better way to check if a module is a JDK module
>>  - initial review comments from Ioi
>>  - 8316969: Improve CDS module graph support for --module option
>
> src/hotspot/share/cds/metaspaceShared.cpp line 790:
> 
>> 788:   ArchiveHeapWriter::init();
>> 789:   if (use_full_module_graph()) {
>> 790:     HeapShared::reset_archived_object_states(CHECK);
> 
> This block of code needs to be inside `if (CDSConfig::is_dumping_heap())`

Thanks for catching this. Fixed.
Also moved the `HeapShared::init_for_dumping(CHECK)` inside the "if" block.

> test/hotspot/jtreg/runtime/cds/appcds/jigsaw/module/ModuleOption.java line 107:
> 
>> 105:         oa.shouldHaveExitValue(0)
>> 106:           // module graph won't be archived with an incubator module
>> 107:           .shouldContain("archivedBootLayer not available, disabling full module graph");
> 
> For thoroughness, I think we should also check for this log in heapShared.cpp:
> 
> 
>     if (record->is_full_module_graph() && !CDSConfig::is_loading_full_module_graph()) {
>       if (log_is_enabled(Info, cds, heap)) {
>         ResourceMark rm(THREAD);
>         log_info(cds, heap)("subgraph %s cannot be used because full module graph is disabled",
>                             k->external_name());
>       }
>       return nullptr;
>     }

For this case, the record is null. Per our discussion, I've added another log for the null record case:
 ```
968   if (record == nullptr) {
 969     if (log_is_enabled(Info, cds, heap)) {
 970       ResourceMark rm(THREAD);
 971       log_info(cds, heap)("subgraph %s is not recorded",
 972                           k->external_name());
 973     }

and updated the test accordingly.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/16016#discussion_r1377097395
PR Review Comment: https://git.openjdk.org/jdk/pull/16016#discussion_r1377097436