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

Tue Apr 28 10:06:00 UTC 2020

Hi Martin,

I am glad to hear it. I will flip 
COMPRESSED_CLASS_POINTERS_DEPENDS_ON_COMPRESSED_OOPS for you on
your platforms. However, this did not work in e.g. Graal for x86_64. So 
regardless, I'm afraid I'm gonna have to
at least temporarily hold on to 
COMPRESSED_CLASS_POINTERS_DEPENDS_ON_COMPRESSED_OOPS at least until
the issue is resolved in Graal upstream. I can not commit atomically to 
Graal and to OpenJDK, so unfortunately
I think I have to introduce the feature incrementally anyway. But I will 
try to get rid of it as soon as possible,
once it is supported by all relevant platforms and compilers.

Thanks,
/Erik

On 2020-04-28 11:30, Doerr, Martin wrote:
> Hi Erik,
>
> I've run your new tests + some other stuff on PPC64 and s390 and it works great.
> If aarch64 is also fine, I'll vote for removing COMPRESSED_CLASS_POINTERS_DEPENDS_ON_COMPRESSED_OOPS.
>
> Best regards,
> Martin
>
>
>> -----Original Message-----
>> From: hotspot-dev <hotspot-dev-bounces at openjdk.java.net> On Behalf Of
>> Erik Österlund
>> Sent: Dienstag, 28. April 2020 10:07
>> To: Thomas Stüfe <thomas.stuefe at gmail.com>
>> Cc: hotspot-dev <hotspot-dev at openjdk.java.net>
>> Subject: Re: RFR: 8241825: Make compressed oops and compressed class
>> pointers independent on x86_64
>>
>> 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
>>>