On Thu, 20 Apr 2023 02:14:53 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> Aleksey Shipilev has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Adjust test times
>
> 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++;
> }
True, simplified.
> 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.
Right, did so.
> 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.
Added!
> 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?
That would change the stack depth, for which some tests are sensitive.
https://github.com/openjdk/jdk/pull/13225#issuecomment-1514461529. I think
merging `sleep` implementations is something to do in a targeted cleanup, which
would handle those test updates too.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/13225#discussion_r1172237713
PR Review Comment: https://git.openjdk.org/jdk/pull/13225#discussion_r1172238273
PR Review Comment: https://git.openjdk.org/jdk/pull/13225#discussion_r1172237000
PR Review Comment: https://git.openjdk.org/jdk/pull/13225#discussion_r1172236374
