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

[Robbin Ehn]

Thanks, David!

/Robbin

On 12/20/18 7:46 AM, David Holmes wrote:
> Hi Robbin,
> 
> Looks fine - sorry for the delay.
> 
> Thanks,
> David
> 
> On 18/12/2018 7:23 pm, Robbin Ehn wrote:
>> Hi David,
>>
>> On 11/30/18 5:43 AM, David Holmes wrote:
>>> 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. :)
>>
>> You are correct, it failed 7 of 200 windows runs.
>>
>> I removed gtest... not really happy about that, but not sure what else to do.
>>
>> Inc:
>> http://cr.openjdk.java.net/~rehn/8214180/v3/inc/webrev/
>> Full:
>> http://cr.openjdk.java.net/~rehn/8214180/v3/full/webrev/
>>
>> Thanks, Robbin
>>
>>>
>>> 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