RFR: 8337511: Implement JEP 404: Generational Shenandoah (Experimental) [v7]

[Aleksey Shipilev]

On Wed, 13 Nov 2024 19:32:53 GMT, William Kemper <wkemper at openjdk.org> wrote:

>> This PR merges JEP 404, a generational mode for the Shenandoah garbage collector. The JEP can be viewed here: https://openjdk.org/jeps/404. We would like to target JDK24 with this PR.
>
> William Kemper has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 510 commits:
> 
>  - Merge branch 'merge-latest' into great-genshen-pr-redux
>  - Use new CompactHeader forwarding APIs in generational mode
>  - Merge remote-tracking branch 'jdk/master' into merge-latest
>  - Merge
>  - 8343649: Shenandoah: ShenandoahEvacInfo event does not follow JFR guidelines
>    
>    Reviewed-by: wkemper
>  - Merge
>  - 8343227: GenShen: Fold resource mark into management of preselected regions
>    
>    Reviewed-by: kdnilsen
>  - Merge openjdk/jdk tip into great-genshen-pr-redux
>  - Merge remote-tracking branch 'jdk/master' into merge-latest
>  - Merge remote-tracking branch 'jdk/master' into merge-latest
>  - ... and 500 more: https://git.openjdk.org/jdk/compare/889f9062...5e02b5d8

Dropping the rest of my comments from today's read before I sign off. I have reviewed 200/230 files, will continue tomorrow.

src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.hpp line 76:

> 74:   void vmop_entry_final_roots();
> 75: 
> 76: private:

Feel free to just make all these `protected`. It is more fuzz to try to provide the minimal possible visibility, especially if it gives the ragged visibility blocks in source. The real reason these are `private` is to protect them from accidental external use. `protected` is also good for this.

src/hotspot/share/gc/shenandoah/shenandoahMarkClosures.hpp line 34:

> 32: class ShenandoahHeapRegion;
> 33: 
> 34: class ShenandoahFinalMarkUpdateRegionStateClosure : public ShenandoahHeapRegionClosure {

There is the `shenandoahHeapRegionClosures.hpp` for these, no?

src/hotspot/share/gc/shenandoah/shenandoahMarkingContext.cpp line 44:

> 42:   for (size_t idx = 0; idx < num_regions; idx++) {
> 43:     ShenandoahHeapRegion* r = heap->get_region(idx);
> 44:     if (r->is_affiliated() && heap->is_bitmap_slice_committed(r) && !is_bitmap_clear_range(r->bottom(), r->end())) {

I don't understand this for single gen mode. In that mode `is_affiliated() == false` always, right? So this check never passes, and `is_bitmap_clear` always returns `true`?

src/hotspot/share/gc/shenandoah/shenandoahVerifier.cpp line 657:

> 655:           _generation(nullptr) {
> 656:     if (_options._verify_marked == ShenandoahVerifier::_verify_marked_complete_satb_empty) {
> 657:       Threads::change_thread_claim_token();

It is fairly odd to see Verifier touching the claim token, since the bug _may be_ somewhere in parallel thread oop iteration infra. I think it is fine to just use `Threads::threads_do` (non-parallel version), which AFAIU does not require token modifications.

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

PR Review: https://git.openjdk.org/jdk/pull/21273#pullrequestreview-2445988182
PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1848675048
PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1848786419
PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1848781508
PR Review Comment: https://git.openjdk.org/jdk/pull/21273#discussion_r1848711178