RFR: 8305092: Improve Thread.sleep(millis, nanos) for sub-millisecond granularity [v3]

[Aleksey Shipilev]

On Wed, 19 Apr 2023 06:14:37 GMT, David Holmes <dholmes at openjdk.org> wrote:

>> Aleksey Shipilev has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   Fix gtests
>
> src/hotspot/os/posix/os_posix.cpp line 1545:
> 
>> 1543: 
>> 1544: int PlatformEvent::park_nanos(jlong nanos) {
>> 1545:   assert(0 <= nanos, "nanos are in range");
> 
> `nanos` should never be zero else you call the untimed park.

OK, I see how is that guaranteed in the Windows case. In POSIX case, calling `park()` is untimed wait, but `park(0)` is converted to absolute time that is already passed, and so `pthread_cond_timedwait` would return immediately, right? So `park(0)` is not equivalent to just `park()`? Still, the strongest behavior from Windows case takes precedence here. Changed the assert.

> src/hotspot/os/posix/park_posix.hpp line 57:
> 
>> 55:   void park();
>> 56:   int  park_millis(jlong millis);
>> 57:   int  park_nanos(jlong nanos);
> 
> Still not sure we need this API split but if we keep `park(jlong millis)` and just add `park_nanos(jlong nanos)` then you can avoid touching so many places in the code.

I thought the exposure to `park` -> `park_millis` renames would be smaller, but apparently there is a considerable number of uses. I left `park(millis)` (old) and added `park_nanos(nanos)` (new), and reverted `park_millis` changes.

> src/hotspot/share/runtime/javaThread.hpp line 1145:
> 
>> 1143: public:
>> 1144:   bool sleep_millis(jlong millis);
>> 1145:   bool sleep_nanos(jlong nanos);
> 
> I prefer just one sleep that takes nanos.

If we do only `sleep(jlong nanos)`, then there is an accident waiting to happen, when some unfixed code would call `sleep` with `millis` argument, not knowing it is now `nanos`. That was the reason why I made the names explicit. `sleep_millis` also does the conversion to nanos that does not overflow. But, like with `park` above, I think there is an argument to keep `sleep(millis)` and add `sleep_nanos(nanos)`, to keep code changes at minimum.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/13225#discussion_r1171103473
PR Review Comment: https://git.openjdk.org/jdk/pull/13225#discussion_r1171103664
PR Review Comment: https://git.openjdk.org/jdk/pull/13225#discussion_r1171103560