[aarch64-port-dev ] RFR(M): 8243392: Remodel CDS/Metaspace storage reservation

[Thomas Stüfe]

Thank you Nick. You are of course right, I should use speaking constants
instead. I will change that.

..Thomas

On Wed, Apr 29, 2020 at 10:14 AM Nick Gasson <nick.gasson at arm.com> wrote:

> Hi Thomas,
>
> On 04/28/20 22:54 pm, Thomas Stüfe wrote:
> >
> > These are two new methods:
> >
> > - bool CompressedKlassPointers::is_valid_base(address p)
> >
> > to let the platform tell you whether it considers p to be a valid
> encoding
> > base. The only platform having these restrictions currently is aarch64.
> >
> > - ReservedSpace
> > Metaspace::reserve_address_space_for_compressed_classes(size_t size);
> >
> > this hands over the process of allocating a range suitable for compressed
> > class pointer encoding to the platform. Most platforms will allocate just
> > anywhere, but some platforms may have a better strategy (e.g. trying low
> > memory first, trying only correctly aligned addresses and so on).
> >
> > Beforehand, this coding existed in a similar form in metaspace.cpp for
> > aarch64 and AIX. For now, I left the AIX part out - it seems only half
> > done, and I want to check further if we even need it, if yes why not on
> > Linux ppc, and C1 does not seem to support anything other than
> base+offset
> > with shift either, but I may be mistaken.
>
> Just a small comment:
>
>   33 bool CompressedKlassPointers::is_valid_base(address p) {
>   34
>   35   // Below 32G, base must be aligned to 4G.
>   36   // Above that point, base must be aligned to 32G
>   37
>   38   if (p < (address)(32 * G)) {
>   39     return is_aligned(p, 4 * G);
>   40   }
>   41
>   42   return is_aligned(p, 32 * G);
>   43
>   44 }
>
> On line 42 I'd prefer to use (4 << LogKlassAlignmentInBytes)*G as it
> currently is in metaspace.cpp instead of the literal 32. This makes the
> relationship with the compressed class decode logic a bit clearer as the
> restriction comes from the MOV and MOVK instructions we use to
> decompress the pointer: we have to ensure the bits of the base and bits
> of the offset after shifting do not overlap. Similarly for the
> `increment` field in metaspace_aarch64.cpp line 51.
>
>
> Thanks,
> Nick
>