RFR: 8220795: Introduce TimedYield utility to improve time-to-safepoint on Windows

David Holmes david.holmes at oracle.com
Thu Apr 25 08:44:55 UTC 2019 

+1

Thanks,
David

On 25/04/2019 5:13 pm, Robbin Ehn wrote:
> Still good, thanks!
> 
> /Robbin
> 
> On 4/24/19 3:12 PM, Robin Westberg wrote:
>> Hi David,
>>
>>> On 23 Apr 2019, at 03:39, David Holmes <david.holmes at oracle.com> wrote:
>>>
>>> Hi Robin,
>>>
>>> Sorry, now Easter break got in the way :)
>>>
>>> <trimming>
>>>
>>> On 17/04/2019 11:55 pm, Robin Westberg wrote:
>>>>> On 12 Apr 2019, at 11:15, David Holmes <david.holmes at oracle.com> 
>>>>> wrote:
>>>>> I'd prefer to fix a windows problem, just on windows. I'm not hung 
>>>>> up on having sleep in the name, but if you prefer timed_yield to 
>>>>> naked_short_nanosleep then that's fine (and avoids people wondering 
>>>>> what the "naked" part means).
>>>>>
>>>>> If we need the TimedYield capability in the future then lets 
>>>>> revisit that then.
>>>> Sure, here’s a lighter version of this change that changes the 
>>>> Windows implementation of naked_short_nanosleep, with a few 
>>>> adjustments to some assumptions in the waiting-for-safepoint backoff 
>>>> strategy.
>>>> Still passes tier1, with the same performance improvements on 
>>>> Windows (and no obvious regressions on Linux).
>>>> New webrev:
>>>> https://cr.openjdk.java.net/~rwestberg/8220795/webrev.02/
>>>
>>> Windows changes look fine - thanks.
>>>
>>> Safepoint backoff change seems okay but what affect does it have on 
>>> performance on non-Windows? (javaTimeNanos can sometimes be expensive)
>>
>> In the case where we have to wait for threads to stop it shouldn't 
>> matter, it will be insignificant compared to the time spent actually 
>> sleeping. But in the case where all threads manage to stop before we 
>> resort to waiting there is indeed an unnecessary call to javaTimeNanos 
>> - I have not observed any measurable difference, but I’ve reworked the 
>> code a little bit to avoid this just in case:
>>
>> Full webrev: https://cr.openjdk.java.net/~rwestberg/8220795/webrev.03/
>> Incremental: https://cr.openjdk.java.net/~rwestberg/8220795/webrev.02-03/
>>
>> Best regards,
>> Robin
>>
>>>
>>> Thanks,
>>> David
>>>
>>>> Best regards,
>>>> Robin
>>>>>
>>>>> Thanks,
>>>>> David
>>>>> -----
>>>>>
>>>>>> Best regards,
>>>>>> Robin
>>>>>>>
>>>>>>> ?
>>>>>>>
>>>>>>> Thanks,
>>>>>>> David
>>>>>>> -----
>>>>>>>
>>>>>>>
>>>>>>>> Best regards,
>>>>>>>> Robin
>>>>>>>>> On 5 Apr 2019, at 13:54, Robin Westberg 
>>>>>>>>> <robin.westberg at oracle.com> wrote:
>>>>>>>>>
>>>>>>>>> Hi David,
>>>>>>>>>
>>>>>>>>>> On 5 Apr 2019, at 12:10, David Holmes 
>>>>>>>>>> <david.holmes at oracle.com> wrote:
>>>>>>>>>>
>>>>>>>>>> On 5/04/2019 7:53 pm, Robin Westberg wrote:
>>>>>>>>>>> Hi David,
>>>>>>>>>>> Thanks for taking a look!
>>>>>>>>>>>> On 5 Apr 2019, at 10:49, David Holmes 
>>>>>>>>>>>> <david.holmes at oracle.com> wrote:
>>>>>>>>>>>>
>>>>>>>>>>>> Hi Robin,
>>>>>>>>>>>>
>>>>>>>>>>>> On 5/04/2019 6:05 pm, Robin Westberg wrote:
>>>>>>>>>>>>> Hi all,
>>>>>>>>>>>>> Please review the following change that modifies the way 
>>>>>>>>>>>>> safepointing waits for other threads to stop. As part of 
>>>>>>>>>>>>> JDK-8203469, os::naked_short_nanosleep was used for all 
>>>>>>>>>>>>> platforms. However, on Windows, this function has 
>>>>>>>>>>>>> millisecond granularity which is too coarse and caused 
>>>>>>>>>>>>> performance regressions. Instead, we can take advantage of 
>>>>>>>>>>>>> the fact that Windows can tell us if anyone else is waiting 
>>>>>>>>>>>>> to run on the current cpu. For other platforms the original 
>>>>>>>>>>>>> waiting is used.
>>>>>>>>>>>>
>>>>>>>>>>>> Can't you just make the new code the implementation of 
>>>>>>>>>>>> os::naked_short_nanosleep on Windows and avoid adding yet 
>>>>>>>>>>>> another sleep/yield abstraction? If Windows nanosleep only 
>>>>>>>>>>>> has millisecond granularity then it's broken.
>>>>>>>>>>> Right, I considered that approach, but it's not obvious to me 
>>>>>>>>>>> what semantics would be appropriate for that. Depending on 
>>>>>>>>>>> whether you want an accurate sleep or want other threads to 
>>>>>>>>>>> make progress, it could conceivably be either implemented as 
>>>>>>>>>>> pure spinning, or spinning combined with yielding (either 
>>>>>>>>>>> cpu-local or Sleep(0)-style).
>>>>>>>>>>> That said, perhaps os_naked_short_nanosleep should be 
>>>>>>>>>>> removed, the only remaining usage is in spinYield, which 
>>>>>>>>>>> turns to sleeping after giving up on yielding. For windows, 
>>>>>>>>>>> it may be more appropriate to switch to Sleep(0) in that case.
>>>>>>>>>>
>>>>>>>>>> I definitely don't think we need to introduce another 
>>>>>>>>>> abstraction (though I'm okay with replacing one with a new one).
>>>>>>>>>
>>>>>>>>> Okay, then I’d prefer to drop the os::naked_short_nanosleep. As 
>>>>>>>>> I see it, you would use SpinYield when waiting for something 
>>>>>>>>> like a CAS race, and TimedYield when waiting for a thread 
>>>>>>>>> rendezvous.
>>>>>>>>>
>>>>>>>>> I did try to make TimedYield into a different “policy” for the 
>>>>>>>>> existing SpinYield class at first, but didn’t quite feel like I 
>>>>>>>>> found a nice API for it. So perhaps it's fine to just have them 
>>>>>>>>> as separate classes.
>>>>>>>>>
>>>>>>>>>> It was probably discussed at the time but the 
>>>>>>>>>> naked_short_nanosleep on Windows definitely seems to have far 
>>>>>>>>>> too much overhead - even having to create a new waitable-timer 
>>>>>>>>>> each time. Do we know if the overhead is actually the 
>>>>>>>>>> resolution of the timer or whether it's all the extra work 
>>>>>>>>>> that method needs to do? I should try to dig up the email on 
>>>>>>>>>> that. :)
>>>>>>>>>
>>>>>>>>> Yes, as far as I know the best possible sleep resolution you 
>>>>>>>>> can obtain on Windows is one millisecond (perhaps 0.5ms in 
>>>>>>>>> theory if you go directly for NtSetTimerResolution). The 
>>>>>>>>> SetWaitableTimer approach is the best though to actually obtain 
>>>>>>>>> 1ms resolution, plain Sleep(1) usually sleeps around 2ms.
>>>>>>>>>
>>>>>>>>> Best regards,
>>>>>>>>> Robin
>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> Thanks,
>>>>>>>>>> David
>>>>>>>>>>
>>>>>>>>>>> Best regards,
>>>>>>>>>>> Robin
>>>>>>>>>>>>
>>>>>>>>>>>> Thanks,
>>>>>>>>>>>> David
>>>>>>>>>>>> -----
>>>>>>>>>>>>
>>>>>>>>>>>>> Various benchmarks (specjbb2015, specjm2008) show better 
>>>>>>>>>>>>> numbers for Windows and no regressions for Linux.
>>>>>>>>>>>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8220795
>>>>>>>>>>>>> Webrev: 
>>>>>>>>>>>>> http://cr.openjdk.java.net/~rwestberg/8220795/webrev.00/
>>>>>>>>>>>>> Testing: tier1
>>>>>>>>>>>>> Best regards,
>>>>>>>>>>>>> Robin
>>

More information about the hotspot-runtime-dev mailing list