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

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

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

Thomas Stuefe
On Wed, 10 Mar 2021 17:41:25 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 seven commits:
>
>  - Merge branch 'master' into jdk-8236847
>  - 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

I think this is fine. "compatible" vs "default" is a good compromise. Small remarks inside. Sorry for all the bikeshedding!

Cheers, Thomas

src/hotspot/os_cpu/bsd_x86/os_bsd_x86.hpp line 30:

> 28: #if defined(COMPATIBLE_CDS_ALIGNMENT)
> 29: #define CDS_CORE_REGION_ALIGNMENT (16*K)
> 30: #endif

Could you limit this to `_APPLE_`? I don't think this affects the BSDs.

(We really should separate BSD and MacOS sources at some point.)

Also maybe a comment like this: "Core region alignment is 16K to be able to run binaries built on MacOS x64 on MacOS aarch64." ?

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

> 123: // os::vm_allocation_granularity() is usually 4K for most OSes. However, on Linux/aarch64,
> 124: // it can be either 4K or 64K and on Macosx-arm it is 16K. To generate archives that are
> 125: // compatible for both settings. An alternative cds core region alignment can be enabled

dot -> comma and lowercase

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

Marked as reviewed by stuefe (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 [v7]

Thomas Stuefe
On Wed, 10 Mar 2021 18:44:18 GMT, Thomas Stuefe <[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 seven commits:
>>
>>  - Merge branch 'master' into jdk-8236847
>>  - 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
>
> I think this is fine. "compatible" vs "default" is a good compromise. Small remarks inside. Sorry for all the bikeshedding!
>
> Cheers, Thomas

> The page size on aarch64 seems to be configurable to 4, 64 and 1MB (and
> also 16KB on some platforms, apparently M1 is the case). Do we know if
> anyone that plans to use 1MB page sizes?

I am not sure if it is possible to use a *base page size* of 1M, and even if it were, it would be terribly inefficient. And nothing prevents you, at least on Linux, from running with -XX:+UseLargePages, which would be a more sensible way to use large pages.

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

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