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

[Nick Gasson]

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