RFR: 8263427: Shenandoah: Trigger weak-LRB even when heap is stable [v4]

[Aleksey Shipilev]

On Mon, 15 Mar 2021 18:46:36 GMT, Roman Kennke <rkennke at openjdk.org> wrote:

>> We currently guard all LRBs, including weak-LRB, by a test for heap-stable and only enter the LRB when heap is unstable (e.g. evacuation or update-refs in progress). However, the weak LRB must also be entered when heap is stable and concurrent refs is in progress, otherwise we may accidentally resurrect otherwise unreachable weak referents. This can happen when we take the shortcut cycle and skip evac&update-refs.
>> 
>> I believe this might be the root cause for JDK-8262852. 
>> 
>> The way out of it is change conc-weakroots-in-progress flag to a bit in gc-state, and test for this in weak-LRB gc-state-check, and enter weak-LRB even when heap is stable, but conc-weakroots-in-progress.
>> 
>> There's one gotcha here: we used to change gc-state only at safepoints so that the flag can safely be propagated to all Java threads. But conc-weakroots-in-progress is turned-off concurrently. I deal with this by propagating the flag change to Java threads via the rendevouz (that we do anyway), and change the global flag only once all threads got the thread-local flag change.
>> 
>> This stuff makes the verifier unhappy, because it doesn't know about the new bit. And it'd be difficult to properly verify it, because sometimes it is set (conc-cycle) and sometimes it is not (degen-cycle), so instead of additing extra verification, I figured we could keep ignoring the flag (for now?)
>> 
>> Testing:
>>  - [x] New testcase failed without change, passes now
>>  - [x] hotspot_gc_shenandoah
>>  - [ ] tier1 (+Shenandoah)
>
> Roman Kennke has updated the pull request incrementally with one additional commit since the last revision:
> 
>   Disable weakroots together with evacuation at safepoint, and in a separate vmop in shortcut cycles

src/hotspot/share/gc/shenandoah/shenandoahPhaseTimings.hpp line 102:

> 100:                                                                                        \
> 101:   f(disable_weakroots_gross,                        "Disable Weak Roots (G)")          \
> 102:   f(disable_weakroots,                              "Disable Weak Roots (N)")          \

Should start with "Pause". And should probably be something generic, like "Pause Final Roots"? This probably percolates to the names of everything else...

src/hotspot/share/gc/shenandoah/shenandoahConcurrentGC.cpp line 226:

> 224: }
> 225: 
> 226: 

Double new line.

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

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