RFR(s): 8214180: Need better granularity for sleeping

[David Holmes]

Hi Robbin,

On 28/11/2018 9:27 pm, Robbin Ehn wrote:
> Hi David,
> 
> Inc:
> http://cr.openjdk.java.net/~rehn/8214180/v2/inc/webrev/

Okay. I still have my doubts/concerns about Windows, but as long as the 
observed minimum "nanosleep" is no worse than the 1ms "short sleep" that 
was previously requested by SpinYield::yield_or_sleep() then it should 
be okay.

I also have some concerns the test might fail on some versions of 
Windows or running on particular hardware. Can you try to do a 
--test-repeat run in mach5 for Windows only so we hit as many of the 
Windows machines as possible. :)

Thanks,
David

> Full:
> http://cr.openjdk.java.net/~rehn/8214180/v2/webrev/
> 
> On 2018-11-27 00:08, David Holmes wrote:
>> Hi Robbin,
>>
>> On 22/11/2018 12:06 am, Robbin Ehn wrote:
>>> Hi all, please review.
>>>
>>> naked_short_sleep is to coarse grained on contemporary hardware/os:es.
>>> 1 ms as minimum when we can complete an entire safe-point in 0.5 ms 
>>> is a very long time.
>>> Sleeping a very short time instead yielding have several uses-cases.
>>
>> So you factored out os::naked_short_sleep into os_posix.cpp for use by 
>> all platforms except Windows. That seems fine. Solaris is already 
>> linked with -lrt so use of nanosleep should be fine there.
>>
>> You added os::naked_short_nanosleep, defined in os_posix.cpp, to use 
>> nanosleep. Also fine.
>>
>> Question: have you actually measured the observable minimum sleep time 
>> on different OS? (And it can even vary depending on hardware).
> 
> Windows ~1ms, Linux ~55us(can vary a lot depending on power saving, 
> scheduler timings etc..).
> 
>>
>> For Windows you create and use a WaitableTimer. That does not seem 
>> okay. That seems extremely heavyweight. The time taken to create and 
>> use the timer might be longer than what you intended to sleep for! And 
>> again there is the issue of the actual accuracy of the timer even if 
>> you can specify nanosecond times. I'm also unclear about the time 
>> value passed to the timer - the docs state it is supposed to be 
>> expressed in 100ns increments, and it's unclear if that also applies 
>> to the relative form ??
> 
> Yes, I commented the creation of the timer. I considered adding the 
> timer to each thread, but I rather not. And on Linux
> you don't need any syscall for creating such primitives, if you still 
> need to do that windows I don't know. But as it
> turns out it doesn't matter, since the scheduler delay is ~1ms on my 
> win10 box, if I'm luck I get 0.5ms. So the cost is
> not measurable. Presumably windows is still not tick-less?
> 
> Yes, I miss-read the docs, correct, it should be in hundreds of nanos, 
> not nanos, thanks.
> 
>>
>>> Here I add it SpinYield to get much smother back-off delay curve.
>>> Which means it will be usable in more places.
>>
>> Seems okay - assuming a 1 microsecond sleep time is achieveable.
> 
> As I said it is not achievable today, it should read as do not execute 
> for at least 1us.
> Arguably we could go higher or lower. I think of it the other way around:
> Your CAS have repeatedly failed, how many instruction should the 
> competing threads execute before it worth testing again.
> 
> Thanks!
> 
> /Robbin
> 
>>
>> Thanks,
>> David
>> -----
>>
>>> CR:
>>> https://bugs.openjdk.java.net/browse/JDK-8214180
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~rehn/8214180/webrev/
>>>
>>> Passes t1-3.
>>>
>>> Thanks, Robbin