RFR: 8350314: Shenandoah: Capture thread state sync times in GC timings [v4]

[Xiaolong Peng]

On Wed, 26 Feb 2025 02:23:15 GMT, Y. Srinivas Ramakrishna <ysr at openjdk.org> wrote:

>> Xiaolong Peng has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Set _gc_state_changed to false at the end of concurrent_prepare_for_update_refs
>
> src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp line 1288:
> 
>> 1286: 
>> 1287:   _update_refs_iterator.reset();
>> 1288:   _gc_state_changed = false;
> 
> Sorry, what is this and why do we need it. When was it set?
> 
> In `set_update_refs_in_progress()` ?
> 
> May be we need a better abstraction to toggle the `state_changed` boolean. This seems error prone.

You are right, we don't need to set _gc_state_changed to false here, because `concurrent_prepare_for_update_refs` calls `ShenandoahHeap::set_gc_state_concurrent` to set gc state, `ShenandoahHeap::set_gc_state_concurrent` doesn't change  `_gc_state_changed` to true, hence there is no need to reset it false here.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/23759#discussion_r1971094948