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

[Calvin Cheung]

On Thu, 1 Sep 2022 03:36:17 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> Currently, the CDS archive heap supports a fixed set of built-in classes, specified by hard-coded lists in heapShared.cpp:
>> 
>> https://github.com/openjdk/jdk/blob/372fc58e897d25713db0dfe289ed25c40d9a3985/src/hotspot/share/cds/heapShared.cpp#L104-L107
>> 
>> As we plan to improve the archive heap, it becomes critical to be able to write specific test cases for different scenarios.
>> 
>> For example, supporting LambdaForms in the CDS archive heap would require archiving enums of the type [sun.invoke.util.Wrapper](https://github.com/openjdk/jdk/blob/607612899678234c093dc644d3a40cb831c7e43b/src/java.base/share/classes/sun/invoke/util/Wrapper.java), which doesn't work yet (see [JDK-8293187](https://bugs.openjdk.org/browse/JDK-8293187)). This RFE makes it possible to develop support for sun.invoke.util.Wrapper separately, without implementing the full support of LambdaForms in a single colossal step.
>> 
>> - Added `-XX:ArchiveHeapTestClass` to inject extra classes into the archive heap. See the test case ArchiveHeapTestClass.java for examples. For paranoia, This flag is available only in debug builds.
>> - I also tighten the requirement for the type of classes that can be stored into the archive heap. E.g., we no longer allow classes outside of the `java.base` module.
>
> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
> 
>   fixed build (gcc spurious warning)

Some initial comments...

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."

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");`

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");`

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

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