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

[William Kemper]

On Tue, 26 Nov 2024 10:16:58 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> src/hotspot/share/gc/shenandoah/shenandoahUncommitThread.cpp line 75:
>> 
>>> 73:       MonitorLocker locker(&_stop_lock, Mutex::_no_safepoint_check_flag);
>>> 74:       if (!_stop_requested.is_set()) {
>>> 75:         locker.wait((int64_t)shrink_period);
>> 
>> I tried to test this on some of my toy examples, and realized this particular line may end up as `locker.wait(0)`, which means "wait indefinitely, until notified". This breaks periodic commits. The old code rode on control thread doing `MAX2(1, ...)`, so we never feed `0` into `wait`. I am also confused about units. The comment above says `shrink_period` is in seconds, but `locker.wait` accepts milliseconds?
>
> It sounds like this line should be:
> 
> 
>         locker.wait(MAX2<int64_t>(1, shrink_period * 1000));

Sorry, I missed your comments here. I noticed the same and have refactored this code.

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

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