RFR: 6313903: Thread.sleep(3) might wake up immediately on windows

[Roger Riggs]

Hi David,

In os.cpp:1893, can the double read of javaTimeNanos be avoided on the 
first iteration?
Set newtime = prevtime before the loop and re-read it after the call to 
park(:1918)
Negible nanotime should have elapsed between 1881 and 1893.
Ditto the non-interruptable code in 1936-1947.
(Unless javaTimeNanos is free?).

$.02, Roger




On 9/2/19 2:06 AM, David Holmes wrote:
> Bug: https://bugs.openjdk.java.net/browse/JDK-6313903
> webrev: http://cr.openjdk.java.net/~dholmes/6313903/webrev/
>
> This is a fix to a very long term problem on Windows that really 
> should have been fixed many years ago: the Windows timed-wait routines 
> can return early by up to a clock tick** (which itself could be as 
> long as 16ms). The fix for Thread.sleep is to track the elapsed time 
> (via nanoTime) and simply adjust and repeat the sleep if we were going 
> to return earlier than requested. This is the same as what we have 
> been doing on non-Windows for many years.
>
> ** whether or not you observe this seems highly dependent on hardware 
> - see bug report for observed behaviours.
>
> The additional motivation for fixing this now is to reduce the amount 
> of platform specific sleep/interrupt code and to allow changes to the 
> shared interrupt code. There are some additional RFE's in the pipeline 
> for that.
>
> This fix does the following:
> - moves the os_posix.cpp os::sleep implementation to os.cpp to be 
> shared by all platforms and deletes the os_windows.cpp os::sleep 
> implementation
>   - there are some additional comments added and the JavaThread 
> assertion is moved to the head of the "interruptible" code path
> - updates os_windows.cpp in these additional ways:
>   - updates the commentary about creating the interrupt_event (which 
> is now only used by JDK library code for use by Process.waitFor)
>   - updates os::interrupt to also unpark the _SleepEvent
>   - moves the use of the HighResolutionInterval utility class from the 
> os::sleep code to the PlatformEvent::park code, so that sleep on 
> Windows will still make adjustments to the clock tick if needed (it 
> also means that Object.wait() calls will now make the same 
> adjustments**).
>
> ** This is a change behaviour but I'm not concerned about there being 
> any adverse effects from this. On many systems the code will have no 
> affect as the system will be using the 1ms clock tick all the time 
> anyway (e.g. my laptops do this). On systems with e.g. 15ms tick, what 
> you will now see is Object.wait returning on time rather than much 
> later than requested e.g. before this fix a timed-wait of 1ms returned 
> in 15.14ms, but after the fix it was 2.3ms. (Note that this 
> observation seems contrary to the general statement that windows 
> timed-wait event operations can return early by up to a clock tick - 
> as I said it seems highly hardware dependent.)
>
> Testing: tiers 1-3
>          custom testing to look at sleep and wait times, as per bug 
> report.
>
> Thanks,
> David
> -----