Re: RFR: 8236847: CDS archive with 4K alignment unusable on machines with 64k pages [v6]

classic Classic list List threaded Threaded
5 messages Options
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8236847: CDS archive with 4K alignment unusable on machines with 64k pages [v6]

Yumin Qi-3
> 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

Yumin Qi has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains six commits:

 - Merge master
 - Add --enable-compatible-cds-alignment for linux-aarch64 and macosx-x64 in jib job
 - Switch to enble compatible-cds-alignment at configuration
 - Make CDS core region alignment configurable at build time
 - Make 64K core region alignment only for specific platforms, also fixed comments as suggestions.
 - 8236847: CDS archive with 4K alignment unusable on machines with 64k pages

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

Changes: https://git.openjdk.java.net/jdk/pull/2651/files
 Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2651&range=05
  Stats: 221 lines in 16 files changed: 161 ins; 16 del; 44 mod
  Patch: https://git.openjdk.java.net/jdk/pull/2651.diff
  Fetch: git fetch https://git.openjdk.java.net/jdk pull/2651/head:pull/2651

PR: https://git.openjdk.java.net/jdk/pull/2651
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8236847: CDS archive with 4K alignment unusable on machines with 64k pages [v6]

Ioi Lam-2
On Mon, 1 Mar 2021 17:08:02 GMT, Yumin Qi <[hidden email]> 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
>
> Yumin Qi has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains six commits:
>
>  - Merge master
>  - Add --enable-compatible-cds-alignment for linux-aarch64 and macosx-x64 in jib job
>  - Switch to enble compatible-cds-alignment at configuration
>  - Make CDS core region alignment configurable at build time
>  - Make 64K core region alignment only for specific platforms, also fixed comments as suggestions.
>  - 8236847: CDS archive with 4K alignment unusable on machines with 64k pages

LGTM

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

Marked as reviewed by iklam (Reviewer).

PR: https://git.openjdk.java.net/jdk/pull/2651
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8236847: CDS archive with 4K alignment unusable on machines with 64k pages [v6]

Ioi Lam-2
On Wed, 3 Mar 2021 20:59:28 GMT, Ioi Lam <[hidden email]> wrote:

>> Yumin Qi has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains six commits:
>>
>>  - Merge master
>>  - Add --enable-compatible-cds-alignment for linux-aarch64 and macosx-x64 in jib job
>>  - Switch to enble compatible-cds-alignment at configuration
>>  - Make CDS core region alignment configurable at build time
>>  - Make 64K core region alignment only for specific platforms, also fixed comments as suggestions.
>>  - 8236847: CDS archive with 4K alignment unusable on machines with 64k pages
>
> LGTM

I think this part of test/hotspot/jtreg/runtime/cds/appcds/SharedArchiveConsistency.java needs to be changed, too:

    public static long align_up_page(long l) throws Exception {
        // wb is obtained in getFileOffsetInfo() which is called first in main() else we should call
        // WhiteBox.getWhiteBox() here first.
        int pageSize = wb.getVMPageSize();    <<<<<
        return (l + pageSize -1) & (~ (pageSize - 1));
    }

The `pageSize` should be `FileMapHeader::_core_region_alignment`.

There's a similar problem in the test dynamicArchive/ArchiveConsistency.java.

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

PR: https://git.openjdk.java.net/jdk/pull/2651
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8236847: CDS archive with 4K alignment unusable on machines with 64k pages [v6]

Yumin Qi-3
On Thu, 4 Mar 2021 23:40:33 GMT, Ioi Lam <[hidden email]> wrote:

> SharedArchiveConsistency.java

I would like to do this in a separate PR. Look at the test, WhiteBox.getWhiteBox().metaspaceReserveAlignment(), I believe this should be _core_region_alignment too. Before this PR, they are all page size so no different but after this PR, they could have different values.

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

PR: https://git.openjdk.java.net/jdk/pull/2651
Reply | Threaded
Open this post in threaded view
|

Re: RFR: 8236847: CDS archive with 4K alignment unusable on machines with 64k pages [v6]

Magnus Ihse Bursie-3
In reply to this post by Yumin Qi-3
On Mon, 1 Mar 2021 17:08:02 GMT, Yumin Qi <[hidden email]> 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
>
> Yumin Qi has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains six commits:
>
>  - Merge master
>  - Add --enable-compatible-cds-alignment for linux-aarch64 and macosx-x64 in jib job
>  - Switch to enble compatible-cds-alignment at configuration
>  - Make CDS core region alignment configurable at build time
>  - Make 64K core region alignment only for specific platforms, also fixed comments as suggestions.
>  - 8236847: CDS archive with 4K alignment unusable on machines with 64k pages

Build changes look good.

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

Marked as reviewed by ihse (Reviewer).

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