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

Tue Jan 28 01:32:05 UTC 2020

Hi Christoph,

On Mon, Jan 27, 2020 at 11:41 AM Langer, Christoph
<christoph.langer at sap.com> wrote:
>
> Hi Jiangli,
>
> the patch from your second webrev worked well, no regressions spotted. So go ahead with pushing at your convenience.
>

Thanks for confirming!

> 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
>

I didn't know about the process indeed. Thanks a lot for the pointer!
Will include the metadata by following the instructions.

Best regards,
Jiangli

> 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