RFR: 8344049: Shenandoah: Eliminate init-update-refs safepoint [v4]

[Y. Srinivas Ramakrishna]

On Mon, 13 Jan 2025 18:45:23 GMT, William Kemper <wkemper at openjdk.org> wrote:

>> Shenandoah typically takes 4 safepoints per GC cycle. Although Shenandoah itself does not spend much time on these safepoints, it may still take quite some time for all of the mutator threads to reach the safepoint. The occasionally long time-to-safepoint increases latency in the higher percentiles.
>> 
>> The `init-update-refs` safepoint is responsible for retiring GCLABs (and PLABs) used during evacuation. Once evacuation is complete, no threads will access these LABs. This need not be done on a safepoint. `init-update-refs` is also where the global and thread local copies of the `gc_state` are updated. However, here we are turning off the `WEAK_ROOTS` flag _after_ all of the unmarked weak referents have been `nulled` out, so this does not need to happen atomically with respect to the mutators. Neither is it necessary to change the other state flags (EVACUATION, UPDATE_REFS) atomically across all mutators.
>> 
>> Note that the `init-update-refs` safepoint is still taken if either verification or `ShenandoahPacing` are enabled.
>
> William Kemper has updated the pull request incrementally with two additional commits since the last revision:
> 
>  - Encapsulate and document a method for making concurrent gc_state changes
>  - Control thread doesn't need a gc lab, also make gclabs for safepoint workers parsable

Left a few comments/questions. Looks good modulo those comments. Thanks again for your patience. Rest looks good. No need for a further round from me even if you make further changes in response to my comments.

src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp line 1268:

> 1266: 
> 1267:   // The handshake won't touch non-java threads, so do those separately.
> 1268:   Threads::non_java_threads_do(&prepare_for_update_refs);

Which non-Java threads need to prepare for update refs? (i.e. which make use of this state predicate and/or participate in update refs phase.)

Would be good to document that somewhere (or may be repeat it here).

src/hotspot/share/gc/shenandoah/shenandoahHeap.hpp line 372:

> 370:   // Critically, this method will _not_ flag that the global gc state has changed and threads
> 371:   // will continue to use their thread local copy. This is expected to be used in conjunction
> 372:   // with a handshake operation to propagate the new gc state.

Thank you for this comment!

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

Marked as reviewed by ysr (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/22688#pullrequestreview-2550382471
PR Review Comment: https://git.openjdk.org/jdk/pull/22688#discussion_r1915539302
PR Review Comment: https://git.openjdk.org/jdk/pull/22688#discussion_r1915543381