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

[Langer, Christoph]

Hi Jiangli,

the patch from your second webrev worked well, no regressions spotted. So go ahead with pushing at your convenience.

I, however, noticed that your patch also doesn't contain metadata (e.g. change description, reviewed-by, etc.). So, Im wondering whether you use the workflow to do a hg export --git -r ... > ... and then hg qimport/hg qpush/hg qrefresh. That should keep the metadata intact and also take care that no files get lost... See here for reference, in case you didn't know: https://wiki.openjdk.java.net/display/JDKUpdates/How+to+contribute+a+fix

Best regards
Christoph

> -----Original Message-----
> From: Jiangli Zhou <jianglizhou at google.com>
> Sent: Samstag, 25. Januar 2020 00:35
> 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,
> 
> Oops, forgot to run 'hg add ...' before generating the webrev. The
> webrev was missing the following two files. Thank you for catching
> that!
> 
>   src/hotspot/share/memory/heapShared.inline.hpp
>   test/hotspot/jtreg/runtime/appcds/cacheObject/DifferentHeapSizes.java
> 
> Here is the updated webrev with the missing files included. Could you
> please try the new patch? Thanks!
> 
> http://cr.openjdk.java.net/~jiangli/8208658-backport/webrev.01/
> 
> Best regards,
> 
> Jiangli
> 
> On Fri, Jan 24, 2020 at 2:33 PM Langer, Christoph
> <christoph.langer at sap.com> wrote:
> >
> > 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