RFR: 8241825: Make compressed oops and compressed class pointers independent on x86_64

[Erik Österlund]

Hi Thomas,

On 2020-04-28 08:43, Thomas Stüfe wrote:
> Hi Erik,
>
> nice change, and now I do not have to keep in mind "heap>32G = no 
> class space" :)

Glad you liked it!

> I eyeballed the metaspace changes (what little there are).
>
> This:
>
> - char* base = (char*)align_up(CompressedOops::end(), 
> _reserve_alignment);+ char* base;+ if (UseCompressedOops) {+ base = 
> (char*)align_up(CompressedOops::end(), _reserve_alignment);+ } else {+ 
> base = (char*)HeapBaseMinAddress;+ }
> may not work as intended. You probably want the end of the reserved heap as attach point for ccs, not the HeapBaseMinAddress.

Could you please expand on that? To make it clear, what this code does 
is to put metaspace at the
minimum address we are allowed to map, when compressed oops is disabled. 
The reasoning for doing
that is that if the low address bits (resulting in good compressed 
pointers) are not used by compressed oops,
then they are available to be used by compressed class pointers instead, 
such that at least compressed class
pointers can utilize this VA to get optimal encoding of class pointers.

Is there anything not desirable about this? If so, I can't see it, and 
would appreciate if you expand on why
that would be a bad idea. Perhaps there is something I do not see here.

> Also, could we have some tests which exercise cds/metaspace 
> initialization without coops and with ccps? Easiest way would be to 
> extend "CompressedOops/CompressedClassPointers.java" by some cases.

I will have a look at that for the next webrev.

Thanks for the review.

/Erik

> Thank you, Thomas
>
> On Fri, Apr 24, 2020 at 10:19 AM Erik Österlund 
> <erik.osterlund at oracle.com <mailto:erik.osterlund at oracle.com>> wrote:
>
>     Hi,
>
>     Today, the use of compressed class pointers assumes the use of
>     compressed oops.
>     This patch loosens up the relationship between compressed oops and
>     compressed
>     class pointers, so that compressed class pointers can be used without
>     compressed
>     oops. This benefits for example ZGC that wants compressed class
>     pointers, but
>     not compressed oops, effectively giving ZGC 4 bytes smaller objects.
>
>     Much of the complexity of the change is that r12 used to be used by
>     compressed
>     class pointers as some kind of semi-fast temp register that one would
>     restore
>     to the compressed oops heap base (or zero). That made it
>     effectively a
>     slightly
>     optimized spilling mechanism used to find a temp register. I
>     replaced that
>     mechanism with a plain old normal temp register that you pass in as a
>     parameter.
>     The bad news is that I had to find available temp registers in a
>     lot of
>     places.
>     The good news is that almost always were there temp registers
>     available
>     for free,
>     and hence the new temp register is even faster than the old optimized
>     spilling
>     mechanism. Because we almost never need any spilling at all in the
>     contexts where
>     this is used.
>
>     Since I want the 4 new bytes to actually make objects smaller, I
>     poked
>     the new
>     layout code a bit so that the klass gap is made available for fields.
>     That used
>     to be made available only with compressed oops enabled, due to
>     restrictions in
>     the layout code. Our new layout code does not have such
>     restrictions, and so
>     I will make the 4 bytes available for fields when the new layout
>     code is
>     used
>     and compressed class pointers are used.
>
>     Now I'm only fixing this for HotSpot x86_64. Ideally the use of
>     compressed oops
>     and compressed class pointers should not be entangled in other
>     platforms and
>     Graal. But that is beyond the scope of this change, and I will let
>     them
>     behave
>     the way that they used to, to be potentially fixed later.
>
>     Bug:
>     https://bugs.openjdk.java.net/browse/JDK-8241825
>
>     Webrev:
>     http://cr.openjdk.java.net/~eosterlund/8241825/webrev.00/
>
>     Testing:
>     hs-tier1-7
>
>     Thanks,
>     /Erik
>