RFR: 8255495: Support CDS Archived Heap for uncompressed oops

Ioi Lam iklam at openjdk.java.net
Wed Jan 26 22:30:40 UTC 2022


On Wed, 26 Jan 2022 02:48:05 GMT, Calvin Cheung <ccheung at openjdk.org> wrote:

> This proposed change adds support for uncompressed oops for CDS archived heap. It is needed
> for supporting archived heap for ZGC in the future. For now, only G1GC is supported with this change.
> During dump time with compressed oops disabled, the MaxHeapSize is set to 4GB. This is to ensure
> the offset of any object in the heap from the bottom of the heap is always a 32-bit integer.
> 
> Passed CI tiers 1 - 4 and hs-tier7-rt tests.
> 
> Preliminary HelloWorld startup perf. improvement:
> 
> instr delta =    -68813432    -49.7140%
> time  delta =      -14.720 ms -20.3599%

The changes look good overall. I have some suggestions for simplifying the code. Also a question on testing.

src/hotspot/share/cds/filemap.cpp line 2151:

> 2149:     HeapShared::set_runtime_delta(delta);
> 2150:   }
> 2151: 

I think the code in the previous few blocks can be simplified by adding two utility functions:


// The address where the bottom of this shared heap region should be mapped
// at runtime
address FileMapInfo::heap_region_runtime_start_address(FileMapRegion* spc) {
  assert(UseSharedSpaces, "runtime only");
  spc->assert_is_heap_region();
  if (UseCompressedOops) {
    return start_address_as_decoded_from_archive(si);
  } else {
     header()->heap_begin() + si->mapping_offset() + HeapShared::set_runtime_delta();
  }
}

void FileMapInfo::set_shared_heap_runtime_delta(ptrdiff_t delta) {
  if (UseCompressedOops) {
    HeapShared::init_narrow_oop_decoding(narrow_oop_base() + delta, narrow_oop_shift());
  } else {
    HeapShared::set_runtime_delta(delta);
  }
}


Then, many of the `if (UseCompressedOops)` checks above can be eliminated.

src/hotspot/share/cds/filemap.cpp line 2211:

> 2209:         assert(is_aligned(si->mapping_offset(), sizeof(HeapWord)), "must be");
> 2210:         start = (HeapWord*)(header()->heap_begin() + si->mapping_offset() + HeapShared::runtime_delta());
> 2211:       }

This block can also be replaced with


HeapWord* start = (HeapWord*)heap_region_runtime_start_address(si);

The `is_aligned(si->mapping_offset()` check can be moved into `heap_region_runtime_start_address()`

src/hotspot/share/cds/heapShared.hpp line 425:

> 423:   static void clear_root(int index);
> 424: 
> 425:   static void set_runtime_delta(ptrdiff_t offset) {

For consistency, `offset` should be renamed to `delta`.

src/hotspot/share/runtime/arguments.cpp line 3454:

> 3452: 
> 3453:   if (DumpSharedSpaces && !UseCompressedOops) {
> 3454:     FLAG_SET_CMDLINE(MaxHeapSize, (size_t)4 * G);

I think we should override MaxHeapSize only if it's greater than 4G. The user should be able to choose a smaller value.

test/hotspot/jtreg/runtime/cds/appcds/javaldr/HumongousDuringDump.java line 31:

> 29:  * @requires vm.cds.write.archived.java.heap
> 30:  * @requires vm.jvmti
> 31:  * @requires vm.opt.final.UseCompressedOops == true

Why is this change needed?

-------------

PR: https://git.openjdk.java.net/jdk/pull/7223



More information about the hotspot-gc-dev mailing list