RFR: 8270489: Support archived heap objects in EpsilonGC [v2]

[Aleksey Shipilev]

On Wed, 11 Aug 2021 06:07:50 GMT, Ioi Lam <iklam at openjdk.org> wrote:

>> **Overview:**
>> 
>> This is the first step for supporting archived heap objects for non-G1 collectors. We are doing it for EpsilonGC first to iron out the API between GC and CDS. Also we can implement most of the common code (such as copying archived objects into heap), without impacting the overall system stability.
>> 
>> - Only G1 can write archive heap objects into the CDS archive.
>> - Archived objects are "mapped" by G1, but the mapping operation is quite complex.
>> - All other collectors will "load" the archive objects, which is much simpler to implement. The trade off is a small start-up penalty and no heap sharing.
>> 
>> Most of the loading code is implemented in heapShared.cpp. The collectors just need to implement the following two CollectedHeap::APIs in 
>> 
>> 
>>   virtual bool can_load_archived_objects();
>>   virtual HeapWord* allocate_loaded_archive_space(size_t size); // typically return a block in old gen
>> 
>> 
>> **Implementation:**
>> 
>> - Allocate (from the old gen) a buffer that's large enough to contain all the archived heap objects.
>> - Inside the CDS archive file, the heap objects are usually divided into 2~4 disjoint regions (there are gaps between them).
>> - Copy every region in to the buffer consecutively, without any gaps.
>> - Relocate all the oop fields in all the copied objects, taking into account of the gap removal.
>> - The archived strings may be relocated by a full GC, but the CDS "shared string table" cannot handle relocation, so we copy the archived strings into the dynamic string table.
>> 
>> **Benchmarking:** 
>> 
>> We can see significant start-up improvement because the module graph can be loaded from CDS.
>> 
>> 
>> $ perf stat -r 40 java -XX:+UnlockExperimentalVMOptions -XX:+UseEpsilonGC -Xmx256m -version
>> 
>> Before:  43.1ms
>> After:   30.2ms
>> 
>> 
>> Testing:
>> 
>> - Some general clean up of the test cases.
>> - Added support for `-vmoptions:-Dtest.cds.runtime.options=-XX:+UnlockExperimentalVMOptions,-XX:+UseEpsilonGC`: we dump the CDS archive with G1 so we have an archived heap, but run with EpsilonGC to test the new loading code.
>> - Added a mach5 task to run all CDS tests with the above config. Incompatible test cases are tagged with `@require vm.gc == null`. See changes in CDSOption.java and VMProps.java
>
> Ioi Lam has updated the pull request incrementally with one additional commit since the last revision:
> 
>   @iignatev comments

Interesting! Cursory review follows.

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

> 150:   // - Mapped: (G1 only) the regions are directly mapped into the heap
> 151:   // - Loaded: At VM start-up, the objects in the heap regions are copied into the
> 152:   //           heap's old generation. This is easier to implement than mapping but

I'd refrain from saying "heap's old generation", as the interface apparently gives the GC freedom in putting the things where it can.

src/hotspot/share/gc/epsilon/epsilonHeap.cpp line 3:

> 1: /*
> 2:  * Copyright (c) 2017, 2020, Red Hat, Inc. All rights reserved.
> 3:  * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.

Here and later: please update Red Hat's copyright year instead. (Like we do for mostly-Oracle files in other cases).

src/hotspot/share/gc/epsilon/epsilonHeap.cpp line 155:

> 153: 
> 154:   // Allocation successful, update counters
> 155:   if (Metaspace::initialized()) {

Feels odd to protect this entire block. It handles generic allocations, not only the metaspace. (I suspect this is only relevant during the initial allocation at startup, but it still looks weird). Would moving a check to `EpsilonMonitoringSupport::update_counters()` and protecting `MetaspaceCounters::update_performance_counters();` there be enough?

src/hotspot/share/gc/epsilon/epsilonHeap.cpp line 163:

> 161: 
> 162:   // ...and print the occupancy line, if needed
> 163:   if (Metaspace::initialized()) {

Ditto here. Maybe moving this check to `EpsilonHeap::print_metaspace_info()` and printing a relevant `log_info(gc, metaspace)` is better.

test/hotspot/jtreg/runtime/cds/appcds/TestEpsilonGCWithCDS.java line 28:

> 26:  * @bug 8234679
> 27:  * @requires vm.cds
> 28:  * @requires vm.bits == 64

Is this test/support really 64-bit specific? I think GHA would test 32-bit Epsilon in `tier1`, not sure about this test, though.

test/hotspot/jtreg/runtime/cds/appcds/TestZGCWithCDS.java line 2:

> 1: /*
> 2:  * Copyright (c) 2021, Oracle and/or its affiliates. All rights reserved.

Here and later: for the files with no changes, no need to update this line, I think.

test/hotspot/jtreg/runtime/cds/appcds/sharedStrings/FlagCombo.java line 29:

> 27:  * @summary Test relevant combinations of command line flags with shared strings
> 28:  * @requires vm.cds.archived.java.heap & vm.hasJFR
> 29:  * @requires vm.gc == null

Here and later: feels like these changes can be forked out into a separate testbug-only PR? Igor did quite a few touchups already for the tests that set their own options.

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

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