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

Aleksey Shipilev shade at openjdk.org
Tue Nov 19 20:26:58 UTC 2024


On Tue, 12 Nov 2024 19:22:58 GMT, William Kemper <wkemper at openjdk.org> wrote:

>> 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`.

OK, nevermind. We can fix it later if it becomes a problem.

>> 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.

Yeah, at least restore the log format and add `gc+start` log as well.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/22019#discussion_r1849006729
PR Review Comment: https://git.openjdk.org/jdk/pull/22019#discussion_r1849011585


More information about the shenandoah-dev mailing list