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