RFR: 8236847: CDS archive with 4K alignment unusable on machines with 64k pages

[Ioi Lam]

On Fri, 19 Feb 2021 18:15:45 GMT, Yumin Qi <minqi at openjdk.org> wrote:

> Hi, Please review
>   Usually most OSes are configured with page size of 4K, but some others are configured with 64K. If jdk binary is built on 4K platform and run on different configured platforms, CDS fails to be loaded due to region alignment mismatch:
>   Unable to map CDS archive -- os::vm_allocation_granularity() expected: 4096 actual: 65536
>   This change uses 64K as region alignment if OS page size is less than 64K. For most of the current OSes, means always use 64K as file map region alignment.
>    The archive size will increase about 300K due to the change. 
>    Tests: tier1-4
>               Run MacOS/X64 binary on MacOS/aarch64 
> 
>    Thanks
>    Yumin

Changes requested by iklam (Reviewer).

src/hotspot/share/memory/filemap.cpp line 212:

> 210:   }
> 211:   _version = CURRENT_CDS_ARCHIVE_VERSION;
> 212:   _region_alignment = region_alignment;

I think this should be renamed to `_core_region_alignment` to be consistent with `MetaspaceShared::core_region_alignment()`

src/hotspot/share/memory/metaspaceShared.cpp line 128:

> 126: void MetaspaceShared::init_alignments() {
> 127:   assert(_core_region_alignment == 0, "call this only once");
> 128:   _core_region_alignment = (size_t)os::vm_allocation_granularity();

The above assignment should be moved inside the `if (DumpSharedSpaces) { ...` block.

src/hotspot/share/memory/metaspaceShared.cpp line 142:

> 140:     assert(FileMapInfo::current_info() != NULL, "Call init_alignments() after base archive is opened");
> 141:     _core_region_alignment = FileMapInfo::current_info()->region_alignment();
> 142:     assert(_core_region_alignment == 64*K, "CDS always use 64K region alignment");

This should be `_core_region_alignment >= 64*K, "CDS must use minimum 64K region alignment"`. and moved below the `else` block.

This makes the code compatible with future OSes that may use >64KB for `os::vm_allocation_granularity()`.

src/hotspot/share/memory/metaspaceShared.cpp line 1351:

> 1349: 
> 1350:   mapinfo->set_is_mapped(false);
> 1351:   assert(mapinfo->region_alignment() == 64*K, "Should always use 64K alignment");

Should be >= 64K

test/hotspot/jtreg/runtime/cds/appcds/SharedRegionAlignmentTest.java line 28:

> 26:  * @requires vm.cds
> 27:  * @requires vm.gc != "Z"
> 28:  * @summary Testing handling of CDS region alignment

I think the summary should be more specific, such as "Make sure CDS core region alignment is at least 64KB".

Also, I think we should add

* @comment ZGC may exit the VM if -XX:+UseLargePages is specified but
*          unavailable. Since this test is independent of the actual GC type, let's
*          disable it if ZGC is used.

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

PR: https://git.openjdk.java.net/jdk/pull/2651