RFR(S) 8208658: Make CDS archived heap regions usable even if compressed oop encoding has changed

Tue Aug 7 16:25:49 UTC 2018

Hi Ioi,

This is a welcomed change! Thanks for doing this. I'd like to suggest 
building a relocation table at dump time. The table can contain entries 
with pair of 'offset_to_region_start:narrow_oop_value'. Then at runtime, 
we can avoid iterating the objects. That would help to reduce the 
overhead that you are observing when relocation is needed at runtime. 
This could be done as a further optimization in a follow-up RFE for 12.

- src/hotspot/share/memory/heapShared.inline.hpp

- src/hotspot/share/memory/heapShared.cpp

   33 inline oop HeapShared::decode_not_null(narrowOop v) {
   34   assert(!CompressedOops::is_null(v), "narrow oop value can never 
be zero");
   35   oop result = (oop)(void*)((uintptr_t)_narrow_oop_base + 
((uintptr_t)v << _narrow_oop_shift));
   36   assert(check_obj_alignment(result), "address not aligned: " 
INTPTR_FORMAT, p2i((void*) result));
   37   return result;
   38 }

  510 void HeapShared::init_narrow_oop_decoding(address base, int shift) {
  511   _narrow_oop_base = base;
  512   _narrow_oop_shift = shift;
  513 }

- src/hotspot/share/memory/filemap.cpp

  913     //   dumptime heap end  ------------v
  914     //   [      |archived heap regions| ]         runtime heap end 
------v
  915     //                                       [   |archived heap 
regions| ]
  916     // |<-----delta-------------------->|
  917     //
  918     // At dump time, the archived heap region were near the top of 
the heap.
  919     // At run time, that region may not be inside the heap, so we 
move it so
  920     // that it's now near the top of teh runtime time. This can be 
done by
  921     // the simple math of adding the delta as shown above.
  922     address dumptime_heap_end = (address)_header->_g1_reserved.end();
  923     address runtime_heap_end = 
(address)G1CollectedHeap::heap()->g1_reserved().end();
  924     delta = runtime_heap_end - dumptime_heap_end;
  925   }
  926
  927   HeapShared::init_narrow_oop_decoding(narrow_oop_base() + delta, 
narrow_oop_shift());

1051 class RelocateInternalPointers: public BasicOopIterateClosure {
...
1056   virtual void do_oop(narrowOop *p) {
1057     narrowOop v = *p;
1058     if (!CompressedOops::is_null(v)) {
1059       oop o = HeapShared::decode_not_null(v);
1060       RawAccess<IS_NOT_NULL>::oop_store(p, o);
1061     }
1062   }

Your relocation change boils down to above sections of code. It works as 
both UnscaledNarrowOop encoding and ZeroBasedNarrowOop encoding use 0 as 
the base. I believe we don't handle heap based encoding with CDS 
enabled. Can you please double check that.

The new HeapShared::decode_not_null() decodes the narrowOop value using 
the dump time encoding mode with added base delta. Can you please rename 
it to be more descriptive, for example 
HeapShared::decode_with_archived_encoding_mode()? Please also add some 
comments to include these detail for this function.

- src/hotspot/share/memory/filemap.cpp

  886   if (narrow_klass_base() != Universe::narrow_klass_base() ||
  887       narrow_klass_shift() != Universe::narrow_klass_shift()) {
  888     log_info(cds)("Cached heap data from the CDS archive need to 
be relocated because");
  889     log_info(cds)("the CDS archive was created with an 
incompatible heap size: " UINTX_FORMAT "M.", max_heap_size()/M);
  890     log_info(cds)("Current narrow_klass_base = " PTR_FORMAT ", 
narrow_klass_shift = %d",
  891                   p2i(Universe::narrow_klass_base()), 
Universe::narrow_klass_shift());
  892     return;
  893   }

Please fix the log message at 888 as relocation is not done in above case.

Please move the code at line 895 - 898 to be before the above code, so 
we still have the logging information about the dump time encoding modes 
when runtime narrow_klass_encoding is different. The dump time encoding 
modes are useful debugging information.

1039 void FileMapInfo::relocate_archived_heap_embedded_pointers()

1051 class RelocateInternalPointers: public BasicOopIterateClosure

1068 void FileMapInfo::relocate_archived_heap_embedded_pointers_impl

Could you please place above new code blocks in heapShared.* files? I 
filed JDK-8206009 to move all heap object archiving code to 
heapShared.*. It's helpful to make sure the related new code goes to the 
right files before we completely resolve existing code for JDK-8206009.

- src/hotspot/share/gc/g1/heapRegion.cpp

Can you please check the percentage of unused region space if we force 
8M gc region at dump time? The unused spaces in the archived heap 
regions are filled with dummy objects at runtime and cannot be utilized. 
It would also introduce gaps between the archived heap regions at 
runtime when the GC region size is < 8M with smaller heap size. We 
should check with GC team on this and see if there are other 
alternatives to resolve the overlapping issue. One possible solution I 
think is to relocate the 'open' archive heap region further down at 
runtime if the 'open' and 'closed' archive regions are in the same G1 
region at runtime.

- test/hotspot/jtreg/runtime/appcds/cacheObject/DifferentHeapSizes.java

Looks good! How about adding some scenarios with heap size > 32G as 
sanity checks.

Thanks,

Jiangli

On 8/1/18 11:04 PM, Ioi Lam wrote:
> https://bugs.openjdk.java.net/browse/JDK-8208658
> http://cr.openjdk.java.net/~iklam/jdk12/8208658-relocate-archived-heap-regions.v00/ 
>
>
> PROBLEM:
>
>     At runtime, if you use a maximum heap size (-Xmx) setting that's
>     different than the one used at CDS archive creation time, the 
> archived
>     heap regions may fail to map. The reason is the archived heap regions
>     are hard-coded to a specific compressed oop encoding scheme.
>
>     This causes degradation in start-up time, because the archived heap
>     regions enable many aggressive optimizations, such as JDK-8202035,
>     archiving the system module info.
>
>     This degradation is undesirable for JDK-8204247 (include default
>     CDS archive in JDK binary) -- what we want is to ship a CDS archive
>     that's optimal regardless of the max heap settings.
>
> FIX:
>
>     If the compressed oop encoding scheme is different:
>         - relocate the archived heap regions so it
>           fits within the runtime heap bounds.
>         - patch all reference fields in the archived objects to
>           use the runtime compressed oop encoding scheme.
>
> RESULTS:
>
>     # dump archive
>     java -Xshare:dump -Xmx2000m
>
>     # same compressed oop encoding
>     java -Xshare:on -Xmx2000m -Xlog:cds -version
>
>     # different compressed oop encoding
>     java -Xshare:on -Xmx2090m -Xlog:cds -version
>
>     Note: the archive is dumped with UnscaledNarrowOop encoding.
>     At run time, if -Xmx2090m is specified, ZeroBasedNarrowOop is
>     used instead.
>
> ========================================================================
>     OLD same encoding      34.45 ms +- 0.51%
>     OLD diff encoding      47.76 ms +- 0.94%  <<<<< 13.3 ms degradation!
>
>     NEW same encoding      34.58 ms +- 0.61%
>     NEW diff encoding      35.09 ms +- 0.24% <<<<< 0.5 ms degradation
> ========================================================================
>
>
>     The cost of relocating/patching the archived heap regions is only
>     0.5 ms. This is much better than the 13.3 ms degradation caused by 
> the
>     failure to map the archived heap regions.
>
> TESTING:
>
>     All 150+ CDS tests passed locally. I am starting tiers 1/2/3 runs.
>
> Thanks
> - Ioi