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

[Ioi Lam]

Hi Dmitry,

Thanks for the review. Have you tried the patch on your 4k/64k page size 
hosts to see if the CDS archives are compatible with each other?

Here's the latest version to address comments from you and Calvin:

http://cr.openjdk.java.net/~iklam/jdk15/8236847-SharedRegionAlignment.v03/
http://cr.openjdk.java.net/~iklam/jdk15/8236847-SharedRegionAlignment.v03-delta/


On 2/10/2020 12:34 AM, Dmitry Samersoff wrote:
> Hello Ioi,
>
> The changes looks good for me, besides few nits below.
>
> cds.h
>
> 56: extra brackets in comments
>

Fixed.

> dynamicArchive.cpp
>
> 727: Is it possible to add here a comment explaining 3 * 
> MetaspaceShared::region_alignment()

I changed the code like this:

   size_t total = estimate_archive_size();
   // We may need to round up the size of each core region for proper 
alignment.
   total += MetaspaceShared::num_core_region * 
MetaspaceShared::region_alignment();


>
> metaspaceShared.cpp
>
> 237:  "on Linux/aarch64," - twice in comments
>

Fixed

> 242: if  is_power_of_2 is not necessary here
>

Removed.

Thanks
- Ioi
> -Dmitry
>
>
> On 31.01.20 00:02, Ioi Lam wrote:
>> Hi Dmitry,
>>
>> The goal of my patch is to make it possible to use the CDS archive 
>> anywhere regardless of the OS page size of the build machine. In my  
>> v01 patch, I added SharedRegionAlignment for flexibility, but I guess 
>> that's more of a hassle.
>>
>> Here's a simplified v02 patch that removes the SharedRegionAlignment 
>> flag:
>>
>> http://cr.openjdk.java.net/~iklam/jdk15/8236847-SharedRegionAlignment.v02/ 
>>
>>
>> Now, on AARCH64, we always dump with 64K alignment, even if the host 
>> has 4K pages -- see MetaspaceShared::init_alignments(). At run time, 
>> the check in MetaspaceShared::map_archive() is relaxed to allow 
>> mapping of 64K-aligned archives on a host with 4K page size.
>>
>> Could you try this and see if it works for you?
>>
>> I don't have access to aarch64 hosts with 64k pages, so I tested on 
>> x64 by changing the #if line in init_alignments to "#if 1". All tests 
>> passed.
>>
>> Thanks
>> - Ioi
>>
>>
>> On 1/30/20 12:34 AM, Dmitry Samersoff wrote:
>>> Hello Ioi,
>>>
>>> I'm not sure that explicit specification of 
>>> SharedRegionAlignment=64k helps.  The goal is to build JDK on a 
>>> build machine then use it smoothly on different test/production 
>>> machines with both 4k and 64k pages, ever if it's the same JDK that 
>>> resides in a shared network location.
>>>
>>> For a long term we may consider to revisit entire logic of default 
>>> archive dumping and dump CDS archive of JDK classes on the first JVM 
>>> run to $HOME/.cache but it requires more work and is out of scope of 
>>> this issue.
>>>
>>> So as a short term solution I would propose to implement following 
>>> logic:
>>>
>>> 1. Without any parameters
>>>
>>>     - try to use existing CDS archive
>>>
>>>    - if alignment doesn't match - disable CDS and continue execution.
>>>
>>>         - if debug or fastdebug  - issue a meaningful warning.
>>>
>>> 2. With explicit -Xshare:auto or -Xshare:on
>>>
>>>     - try to use existing CDS archive
>>>
>>>     - if alignment doesn't match - terminate VM with meaningful 
>>> message.
>>>
>>> -Dmitry
>>>
>>>
>>> On 29.01.20 08:37, Ioi Lam wrote:
>>>> https://bugs.openjdk.java.net/browse/JDK-8236847
>>>> http://cr.openjdk.java.net/~iklam/jdk15/8236847-SharedRegionAlignment.v01/ 
>>>>
>>>>
>>>> Hi Dmitry,
>>>>
>>>> Here's a proposal to make it possible to generate CDS archives that 
>>>> can be used on machines with different OS page alignments: generate 
>>>> the CDS archive with
>>>>
>>>>      java -Xshare:dump -XX:SharedRegionAlignment=64k
>>>>
>>>> I want to be conservative, as this seems to affect only 
>>>> Linux/aarch64, so I have limited the range of this flag to be no 
>>>> more than 64KB. You can see checks for other invalid values in the 
>>>> new test case.
>>>>
>>>> If SharedRegionAlignment is not specified (as for most platforms), 
>>>> os::vm_allocation_granularity() will be used.
>>>>
>>>> For simplicity, -XX:SharedRegionAlignment can only be set when 
>>>> dumping the base archive (-Xshare:dump). The dynamic archive 
>>>> (-XX:DynamicDumpSharedSpaces) will always use the value as stored 
>>>> in the base archive.
>>>>
>>>> Please let me know if this solution will work for your situation. 
>>>> If so, I will file a CSR for the new flag and also write more test 
>>>> cases.
>>>>
>>>> (Also, the default CDS archive is created at JDK build time. I'll 
>>>> leave it to the ARM porting folks to do the makefile changes to 
>>>> pass the
>>>> -XX:SharedRegionAlignment=64k flag to BUILD_CDS_ARCHIVE in Images.gmk)
>>>>
>>>> Thanks
>>>> - Ioi
>>