Re: RFR: 8236847: CDS archive with 4K alignment unusable on machines with 64k pages [v7]
On Wed, 10 Mar 2021 18:33:53 GMT, Thomas Stuefe 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 > > 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 Thanks. Fixed. > 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." ? Fixed. Thanks. - PR: https://git.openjdk.java.net/jdk/pull/2651
Re: RFR: 8236847: CDS archive with 4K alignment unusable on machines with 64k pages [v7]
On Wed, 10 Mar 2021 18:44:18 GMT, Thomas Stuefe 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
Re: RFR: 8236847: CDS archive with 4K alignment unusable on machines with 64k pages [v7]
On Wed, 10 Mar 2021 17:41:25 GMT, Yumin Qi 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
Re: RFR: 8236847: CDS archive with 4K alignment unusable on machines with 64k pages [v7]
> 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 - Changes: https://git.openjdk.java.net/jdk/pull/2651/files Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=2651&range=06 Stats: 220 lines in 16 files changed: 161 ins; 16 del; 43 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
