RFR: 8342444: Shenandoah: Uncommit regions from a separate, STS aware thread [v2]

[William Kemper]

On Tue, 12 Nov 2024 17:46:26 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> William Kemper has updated the pull request incrementally with two additional commits since the last revision:
>> 
>>  - Style and formatting fixes
>>  - Alphabetize includes in shenandoahHeap.cpp
>
> src/hotspot/share/gc/shenandoah/shenandoahHeap.cpp line 1932:
> 
>> 1930:   if (_uncommit_thread != nullptr) {
>> 1931:     _uncommit_thread->stop();
>> 1932:   }
> 
> Are there limits on proper sequencing here? Can we shutdown uncommit thread before cancelling the GC and waiting for control thread to exit? This would save end-to-end time for short commands, as we would hide the uncommit thread shutdown in the shadow of control thread exiting.

I'm not sure the order matters here. `ConcurrentGCThread::stop` waits until the target thread sets `_has_terminated`.

> src/hotspot/share/gc/shenandoah/shenandoahUncommitThread.cpp line 139:
> 
>> 137:     _heap->notify_heap_changed();
>> 138:     double elapsed = os::elapsedTime() - start;
>> 139:     log_info(gc)("Uncommitted " SIZE_FORMAT " regions, in %.3fs", count, elapsed);
> 
> If we can, can we match the current log format? E.g. print `Concurrent uncommit`, with appropriately formatted timestamp? I think we also want `log_info(gc,start)` at the beginning of the method. I think `ShenandoahConcurrentPhase` helper did all that, can we still use it?

We can restore the log messages, but I don't think `ShenandoahConcurrentPhase` and friends will like being used outside of a cycle. I'll look into it.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22019#discussion_r1838644895
PR Review Comment: https://git.openjdk.org/jdk/pull/22019#discussion_r1838646433