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

[Thomas Stüfe]

Hi Andrew,

On Wed, Apr 29, 2020 at 4:21 PM Andrew Haley <aph at redhat.com> wrote:

> On 4/28/20 3:54 PM, Thomas Stüfe wrote:
> > These two methods should give the platform enough control to implement
> > their own scheme for optimized class space placement without bothering
> any
> > shared code about it.
>
> There's still something I don't like. If we have a compressed class space
> in the lower 32G but above 4G, we do this:
>
>     // Otherwise we attempt to use a zero base if the range fits in lower
> 32G.
>     if (end <= (address)ClassEncodingMetaspaceMax) {
>       base = 0;
>     } else {
>       base = addr;
>     }
>
>     // Highest offset a Class* can ever have in relation to base.
>     range = end - base;
>
>     // We may not even need a shift if the range fits into 32bit:
>     const uint64_t UnscaledClassSpaceMax = (uint64_t(max_juint) + 1);
>     if (range < UnscaledClassSpaceMax) {
>       shift = 0;
>     } else {
>       shift = LogClassAlignmentInBytes;
>     }
>
> ... which means that we end up with zero base, shifted compressed
> class pointers, *despite the fact* that we carefully chose a
> nicely-aligned compressed class base we could encode efficiently.
>
> I guess this code above is really optimized for x86; it certainly
> seems to prefer shifts to offsets, which makes sense on that part. It
> doesn't make much difference (if any) for AArch64, I admit, but it is
> odd.
>
>
I understand.

First off, this patch is supposed to be an almost clean code reshuffle. It
was not the intent of this patch to change functionality, at least not by
much, since the patch is complicated enough as it is. It just wants to
improve maintainability, so that we can improve the code in the future
easier. So, compressed class pointer encoding should work as it did before,
modulo those little details about CompressedKlassPointers::range.

That said, I agree with you, this is not optimal. There are other
possibilities to improve matters, e.g. we miss opportunities to go zero
based if the heap is very large since the class space gets always allocated
behind the heap.

Since the intent of this code is to give platforms greater leeway to do
their thing without disturbing shared code, maybe we should make
CompressedKlassPointers::initialize() platform dependent too? Or, add a
hook at the end of it allowing platforms to overwrite the default behavior.
If aarch64 prefers shift=0 and base=<nice base address> it could do so.

..Thomas


-- 
> Andrew Haley  (he/him)
> Java Platform Lead Engineer
> Red Hat UK Ltd. <https://www.redhat.com>
> https://keybase.io/andrewhaley
> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
>
>