[master] RFR: Implement sliding forwarding scheme that preserves upper header bits [v14]

[Aleksey Shipilev]

On Mon, 5 Jul 2021 18:14:08 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

>> The current way of storing forwarding information - by storing the forwardee address into the header - poses a problem for sliding collectors: it overrides the upper bits that keep the (compressed) Klass*. Sliding collectors have no way to recover the Klass*, and therefore need a different forwarding scheme that preserves the upper bits. I propose a scheme that compresses the forwarding pointer and by taking advantage of the fact that each region only ever forwards to at most two other regions (when dividing the heap into equal-sized logical regions), it can address all of the heap, regardless of its size. This obviouly works well with regionalized collectors, but it also works with contiguous-heap collectors like serial or parallel GC. The latter would divide the heap into logical regions sized by 4G (or single region if heap is smaller than that). Notice that evacuating and scavenging collectors don't have this problem: they can safely stomp over the from-space copy of objects, 
 the Klass* information is preserved in the to-space copy.
>> 
>> G1 is special: the fallback from parallel to serial full GC means that we can have maximum of N target regions with N == ParallelGCThreads. For this reason, I use an encoding with 5 bits for target region index, that is enough to address 32 target regions from each region, assuming a maximum heap region size of 32M. In the future we will be able to steal bits from the Klass* in the upper half of the header, allowing us to address even more regions. Alternatively, we can change the full-GC fallback to re-forward all objects serially, instead of trying to re-use the already-forwarded state from the parallel full GC.
>> 
>> Serial GC and Shenandoah GC use only 1 bit to address target regions, because they really can only ever have 2 target regions out of a single region.
>> 
>> Testing:
>> - [x] manual testing with +passive and -degen-gc
>> - [x] hotspot_gc_shenandoah
>> - [x] tier1
>> - [x] tier2
>> - [x] tier1 (x86_32)
>
> Roman Kennke has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Some cosmetic fixes

Okay, I admit this is cute! I have a few cosmetic comments, but otherwise, it looks good for the experimental code. I would probably have a round of code tideups later...

src/hotspot/share/gc/g1/g1FullCollector.cpp line 309:

> 307: 
> 308:   // To avoid OOM when there is memory left.
> 309:   // NOTE: Disabled for now because it violates sliding-forwarding assumption.

I think it should be "TODO", so source analyzers can highlight this.

src/hotspot/share/gc/g1/g1FullGCOopClosures.inline.hpp line 92:

> 90:   // Forwarded, just update.
> 91:   oop forwardee = _forwarding->forwardee(obj);
> 92:   assert(G1CollectedHeap::heap()->is_in_reserved(forwardee), "should be in object space: " PTR_FORMAT "(" PTR_FORMAT ", " PTR_FORMAT ") " INTPTR_FORMAT, p2i(forwardee), p2i(G1CollectedHeap::heap()->reserved().start()), p2i(G1CollectedHeap::heap()->reserved().end()), obj->mark().value());

Style: break the line here.

src/hotspot/share/gc/shared/preservedMarks.hpp line 67:

> 65:   // Iterate over the stack, adjust all preserved marks according
> 66:   // to their forwarding location stored in the mark.
> 67:   void adjust_during_full_gc();

Does this method have any uses? Should those be updated to use `forwarding`? If not, it should be removed for safety?

src/hotspot/share/gc/shared/slidingForwarding.hpp line 40:

> 38:  * The idea is to use a pointer compression scheme very similar to the one that is used for compressed oops.
> 39:  * We divide the heap into number of logical regions. Each region spans maximum of 2^NUM_BITS words.
> 40:  * We take advantage of the fact that sliding compaction can forward objects from ore region to a maximum of

Suggestion:

 * We take advantage of the fact that sliding compaction can forward objects from one region to a maximum of

src/hotspot/share/gc/shared/slidingForwarding.inline.hpp line 50:

> 48:   HeapWord* encode_base;
> 49:   uintptr_t region_idx;
> 50:   for (region_idx = 0; region_idx < (ONE << NUM_REGION_BITS); region_idx++) {

I think `ONE << NUM_REGION_BITS` really deserves a separate constant.

src/hotspot/share/gc/shenandoah/shenandoahFullGC.cpp line 377:

> 375: 
> 376: public:
> 377:   ShenandoahPrepareForCompactionTask(PreservedMarksSet* preserved_marks, ShenandoahHeapRegionSet **worker_slices) :

Irrelevant change.

test/hotspot/jtreg/gc/stress/TestMultiThreadStressRSet.java line 52:

> 50:  * @run main/othervm -Xbootclasspath/a:. -XX:+UnlockDiagnosticVMOptions -XX:+WhiteBoxAPI
> 51:  *   -XX:+UseG1GC -XX:G1SummarizeRSetStatsPeriod=100 -Xlog:gc
> 52:  *   -Xmx1100M -XX:G1HeapRegionSize=8m -XX:MaxGCPauseMillis=1000 gc.stress.TestMultiThreadStressRSet 60 16

Why this change?

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

Marked as reviewed by shade (Committer).

PR: https://git.openjdk.java.net/lilliput/pull/8