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

Tue Apr 28 10:12:41 UTC 2020

Hi Erik,

ok, thank you.

Best regards,
Martin

> -----Original Message-----
> From: Erik Österlund <erik.osterlund at oracle.com>
> Sent: Dienstag, 28. April 2020 12:06
> To: Doerr, Martin <martin.doerr at sap.com>; 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 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
> >>>