8230645: CDS regions get loaded with the wrong alignment

[Thomas Stüfe]

That makes sense, Ioi. I will prepare a patch.

..Thomas

On Thu, Sep 5, 2019 at 6:21 PM Ioi Lam <ioi.lam at oracle.com> wrote:

> 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
>
>