8230645: CDS regions get loaded with the wrong alignment

[Ioi Lam]

Hi Thomas,

I am not quite sure why we use a mix of os::vm_allocation_granularity() 
and Metaspace::reserve_alignment() in the CDS code. I guess the code 
that uses the VirtualSpace API started using 
Metaspace::reserve_alignment() when cut-and-pasting from the regular 
metaspace code. The code that deals with mmap started with 
os::vm_allocation_granularity(). So we end up with the inconsistency. 
Fortunately they are usually both 4096 by default.

I think it's better to clean all of these up. How about adding a new 
function

     MetaspaceShared::reserve_alignment()

and use that to replace all use of os::vm_allocation_granularity() and 
Metaspace::reserve_alignment() in the CDS-related files 
(dynamicArchive.cpp, filemap.cpp, metaspaceShared.cpp)?

I think it can just return os::vm_allocation_granularity().

Thanks
- Ioi


On 9/5/19 1:38 AM, Thomas Stüfe wrote:
> Hi,
>
> may I have reviews for this tiny fix please. CDS requires that its regions
> are loaded with metaspace alignment - I am not sure why. But when loading,
> it aligns loaded regions with alloc granularity.
>
> Since metaspace reserve alignment is usually alloc granularity, this is
> only a problem when one plays around with metaspace, in which case CDS
> asserts on startup.
>
> Bug: https://bugs.openjdk.java.net/browse/JDK-8230645
> webrev:
> http://cr.openjdk.java.net/~stuefe/webrevs/8230645-cds-regions-get-loaded-with-wrong-alignment/webrev.00/webrev/
>
> Note that I am not sure why CDS uses metaspace reserve alignment at all. I
> do not see a reason for this since metaspace reserve alignment does not
> affect CDS regions?
>
> Thanks, Thomas