RFR: 8293182: Improve testing of CDS archive heap [v2]

[Ioi Lam]

On Thu, 1 Sep 2022 19:00:18 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:

>> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   fixed build (gcc spurious warning)
>
> src/hotspot/share/cds/cds_globals.hpp line 78:
> 
>> 76:           "product builds. If specified, the static field named "           \
>> 77:           "\"archivedObjects\" in this class is stored into the CDS "       \
>> 78:           "archive heap")                                                   \
> 
> The “this class” wasn’t clear to me until I read the test case. I’d suggest something like the following:
> 
> 
> “The ""archivedObjects" static field of the specified”
> “class is stored in the CDS archive heap."

Fixed as you suggested.

> src/hotspot/share/cds/heapShared.cpp line 1619:
> 
>> 1617:   int num_slots = sizeof(open_archive_subgraph_entry_fields) / sizeof(ArchivableStaticFieldInfo);
>> 1618:   assert(p[num_slots - 2].klass_name == NULL, "must have empty slot");
>> 1619:   assert(p[num_slots - 1].klass_name == NULL, "must have empty slot");
> 
> Should the first assert be the following?
> 
> `assert(p[num_slots - 1].field_name == NULL, "must have empty slot");`

The first assert actually checks the slot that's patched a few lines below. I updated the assert message.

> src/hotspot/share/classfile/systemDictionary.cpp line 1056:
> 
>> 1054:       assert(HeapShared::is_a_test_class_in_unnamed_module(ik),
>> 1055:              "Loading non-bootstrap classes before the module system is initialized");
>> 1056:     }
> 
> Should there be an else case with the original assert?
> 
> `assert(class_loader.is_null(), "sanity");`

I removed the `assert(class_loader.is_null(), "sanity");` by mistake. I restored it, and also updated the assert message.

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

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