[11u] RFR: backport of JDK-8208658: Make CDS archived heap regions usable even if compressed oop encoding has changed

[Langer, Christoph]

Hi Jiangli,

unfortunately your patch seems to break every build because the file memory/heapShared.inline.hpp does not exist in 11u. So I think you have to update the patch a bit further...

Best regards
Christoph

> -----Original Message-----
> From: Jiangli Zhou <jianglizhou at google.com>
> Sent: Freitag, 24. Januar 2020 16:32
> To: Langer, Christoph <christoph.langer at sap.com>
> Cc: jdk-updates-dev at openjdk.java.net; hotspot-runtime-dev <hotspot-
> runtime-dev at openjdk.java.net>
> Subject: Re: [11u] RFR: backport of JDK-8208658: Make CDS archived heap
> regions usable even if compressed oop encoding has changed
>
> Hi Christoph,
>
> On Fri, Jan 24, 2020 at 6:33 AM Langer, Christoph
> <christoph.langer at sap.com> wrote:
> >
> > Hi Jiangli,
> >
> > This backport looks good to me.
> >
> > Would you want me to run it through our test infrastructure to get
> additional confidence?
> >
>
> That would be most helpful and reassuring. I will wait for your
> testing result before pushing. Thank you!
>
> Best regards,
>
> Jiangli
> > Cheers
> > Christoph
> >
> > > -----Original Message-----
> > > From: hotspot-runtime-dev <hotspot-runtime-dev-
> > > bounces at openjdk.java.net> On Behalf Of Jiangli Zhou
> > > Sent: Mittwoch, 22. Januar 2020 02:05
> > > To: jdk-updates-dev at openjdk.java.net; hotspot-runtime-dev <hotspot-
> > > runtime-dev at openjdk.java.net>
> > > Subject: [11u] RFR: backport of JDK-8208658: Make CDS archived heap
> > > regions usable even if compressed oop encoding has changed
> > >
> > > I'd like to request a backport of JDK-8208658 to JDK 11u. It was an
> > > enhancement that improves CDS usability and would improve startup
> time
> > > if there's an existing default CDS archive (currently does not exist
> > > in OpenJDK 11).
> > >
> > >   webrev: http://cr.openjdk.java.net/~jiangli/8208658-
> backport/webrev.00
> > >
> > > The backport applied to jdk11u-dev mostly clean. The following small
> > > set of changes were hand merged to filemap.cpp without additional
> > > modifications. The backport was tested with all existing cds and
> > > appcds tests, and tier 1 tests.
> > >
> > > --- filemap.cpp
> > > +++ filemap.cpp
> > > @@ -967,7 +1069,9 @@
> > >      if (base == NULL || base != addr) {
> > >        // dealloc the regions from java heap
> > >        dealloc_archive_heap_regions(regions, region_num);
> > > -      log_info(cds)("UseSharedSpaces: Unable to map at required
> > > address in java heap.");
> > > +      log_info(cds)("UseSharedSpaces: Unable to map at required
> > > address in java heap. "
> > > +                    INTPTR_FORMAT ", size = " SIZE_FORMAT " bytes",
> > > +                    p2i(addr), regions[i].byte_size());
> > >        return false;
> > >      }
> > >    }
> > > @@ -1216,34 +1348,25 @@
> > >           idx == MetaspaceShared::rw ||
> > >           idx == MetaspaceShared::mc ||
> > >           idx == MetaspaceShared::md, "invalid region index");
> > > -  char* base = _header->region_addr(idx);
> > > +  char* base = region_addr(idx);
> > >    if (p >= base && p < base + space_at(idx)->_used) {
> > >      return true;
> > >    }
> > >    return false;
> > >  }
> > >
> > > -void FileMapInfo::print_shared_spaces() {
> > > -  tty->print_cr("Shared Spaces:");
> > > -  for (int i = 0; i < MetaspaceShared::n_regions; i++) {
> > > -    CDSFileMapRegion* si = space_at(i);
> > > -    char *base = _header->region_addr(i);
> > > -    tty->print("  %s " INTPTR_FORMAT "-" INTPTR_FORMAT,
> > > -                        shared_region_name[i],
> > > -                        p2i(base), p2i(base + si->_used));
> > > -  }
> > > -}
> > > -
> > >  // Unmap mapped regions of shared space.
> > >  void FileMapInfo::stop_sharing_and_unmap(const char* msg) {
> > >    FileMapInfo *map_info = FileMapInfo::current_info();
> > >    if (map_info) {
> > >      map_info->fail_continue("%s", msg);
> > >      for (int i = 0; i < MetaspaceShared::num_non_heap_spaces; i++) {
> > > -      char *addr = map_info->_header->region_addr(i);
> > > -      if (addr != NULL && !MetaspaceShared::is_heap_region(i)) {
> > > -        map_info->unmap_region(i);
> > > -        map_info->space_at(i)->_addr._base = NULL;
> > > +      if (!MetaspaceShared::is_heap_region(i)) {
> > > +        char *addr = map_info->region_addr(i);
> > > +        if (addr != NULL) {
> > > +          map_info->unmap_region(i);
> > > +          map_info->space_at(i)->_addr._base = NULL;
> > > +        }
> > >
> > > Best regards,
> > > Jiangli