On Wed, 19 Apr 2023 19:55:38 GMT, Aleksey Shipilev <[email protected]> wrote:
>> Java API has the `Thread.sleep(millis, nanos)` method exposed to users. The
>> documentation for that method clearly says the precision and accuracy are
>> dependent on the underlying system behavior. However, it always rounds up
>> `nanos` to 1ms when doing the actual sleep. This means users cannot do the
>> micro-second precision sleeps, even when the underlying platform allows it.
>> Sub-millisecond sleeps are useful to build interesting primitives, like the
>> rate limiters that run with >1000 RPS.
>>
>> When faced with this, some users reach for more awkward APIs like
>> `java.util.concurrent.locks.LockSupport.parkNanos`. The use of that API for
>> sleeps is not in line with its intent, and while it "seems to work", it
>> might have interesting interactions with other uses of `LockSupport`.
>> Additionally, these "sleeps" are no longer visible to monitoring tools as
>> "normal sleeps", e.g. as `Thread.sleep` events. Therefore, it would be
>> prudent to improve current `Thread.sleep(millis, nanos)` for sub-millisecond
>> granularity.
>>
>> Fortunately, the underlying code is almost ready for this, at least on POSIX
>> side. I skipped Windows paths, because its timers are still no good. Note
>> that on both Linux and MacOS timers oversleep by about 50us. I have a few
>> ideas how to improve the accuracy for them, which would be a topic for a
>> separate PR.
>>
>> Additional testing:
>> - [x] New regression test
>> - [x] New benchmark
>> - [x] Linux x86_64 `tier1`
>> - [x] Linux AArch64 `tier1`
>
> Aleksey Shipilev has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Adjust test times
Looking much cleaner/simpler. A couple more comments below. Thanks.
src/hotspot/os/windows/os_windows.cpp line 5263:
> 5261: if (nanos_left > 0) {
> 5262: millis++;
> 5263: }
You could simplify this to:
if (nanos > millis * NANOSECS_PER_MILLISEC) {
millis++;
}
src/hotspot/os/windows/park_windows.hpp line 53:
> 51: void park () ;
> 52: void unpark () ;
> 53: int park(jlong millis);
While you are here could you get rid of the spaces on park/unpark before the
parentheses - thanks.
src/hotspot/share/runtime/javaThread.cpp line 1983:
> 1981: }
> 1982:
> 1983: bool JavaThread::sleep(jlong millis) {
Perhaps a comment:
// Internal convenience function for millisecond resolution sleeps.
src/java.base/share/classes/java/lang/Thread.java line 1:
> 1: /*
Why doesn't `Thread.sleep(long millis)` simply call `Thread.sleep(millis, 0)`
instead of duplicating most of the code?
-------------
PR Review: https://git.openjdk.org/jdk/pull/13225#pullrequestreview-1393107848
PR Review Comment: https://git.openjdk.org/jdk/pull/13225#discussion_r1172011960
PR Review Comment: https://git.openjdk.org/jdk/pull/13225#discussion_r1172013223
PR Review Comment: https://git.openjdk.org/jdk/pull/13225#discussion_r1172014393
PR Review Comment: https://git.openjdk.org/jdk/pull/13225#discussion_r1172017034
