RFR: 8194860: Cleanup Semaphore timed-wait time calculations

[David Holmes]

Thanks Coleen.

David

On 22/01/2019 11:39 pm, coleen.phillimore at oracle.com wrote:
> Looks good!
> Coleen
> 
> On 1/20/19 9:29 PM, David Holmes wrote:
>> Here's the updated webrev:
>>
>> http://cr.openjdk.java.net/~dholmes/8194860/webrev.v2
>>
>> Only changes are in os_posix.cpp:
>>
>> - As Coleen requested I statically initialized the various static 
>> variables to simplify the conditional compilation for Solaris and 
>> removed the dynamic initialization to NULL/false.
>>
>> - I had a bug (exposed on old OS X versions) where I was using 
>> clock_gettime even though it might not be available.
>>
>> Thanks,
>> David
>>
>> On 18/01/2019 8:19 am, David Holmes wrote:
>>> Hi Coleen,
>>>
>>> Thanks for looking at this - though I still have to resolve my OS X 
>>> issue.
>>>
>>> On 17/01/2019 11:04 pm, coleen.phillimore at oracle.com wrote:
>>>> http://cr.openjdk.java.net/~dholmes/8194860/webrev/src/hotspot/os/posix/os_posix.cpp.frames.html 
>>>>
>>>>
>>>> For os_init2, I don't see why you have to sprinkle #ifndef SOLARIS. 
>>>> It's just logging, won't the logging be useful in the negative for 
>>>> solaris? 
>>>
>>> The problem is that regardless of whether the initialization works or 
>>> fails on Solaris, those "attributes" won't get used on Solaris 
>>> anyway, so the logging would be misleading/confusing. So I just 
>>> elided it.
>>>
>>>> Can these three statics default to false/NULL above?  Then you can 
>>>> save having to have this conditional as well:
>>>>
>>>> +#else
>>>> +  _use_clock_monotonic_condattr = false;
>>>> +#endif // !SOLARIS
>>>
>>> Yes I should be able to do that (famous last words!).
>>>
>>>> Thank you for resolving the differences between to_abstime and 
>>>> solaris unpackTime, which was one of the causes of this mess left by 
>>>> the refactoring.
>>>
>>> There's still a lot of mess in the Solaris code.
>>>
>>> Thanks again,
>>> David
>>>
>>>> Looks good, with this one suggestion to remove some #ifndef solaris 
>>>> above.
>>>> Coleen
>>>>
>>>>
>>>>
>>>> On 1/16/19 5:06 PM, David Holmes wrote:
>>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8194860
>>>>> webrev: http://cr.openjdk.java.net/~dholmes/8194860/webrev/
>>>>>
>>>>> Details in the bug report, but there was duplication in 
>>>>> functionality with regards to timeout conversions/calculations, and 
>>>>> the os_bsd.cpp code was broken due to OSXSemaphore and 
>>>>> PosixSemaphore having different API's.
>>>>>
>>>>> Summary:
>>>>> - expanded the API to take a relative timeout in millis as well as 
>>>>> the existing absolute timeout as a timespec:
>>>>>
>>>>>   // wait until the given absolute time is reached
>>>>>   bool timedwait(struct timespec ts);
>>>>>   // wait until the given relative time elapses
>>>>>   bool timedwait(int64_t millis);
>>>>>
>>>>> The millis version simply converts to a timespec and calls the 
>>>>> other version e.g
>>>>>
>>>>> bool PosixSemaphore::timedwait(int64_t millis) {
>>>>>   struct timespec ts;
>>>>>   os::Posix::to_RTC_abstime(&ts, millis);
>>>>>   return timedwait(ts);
>>>>> }
>>>>>
>>>>> - added os::Posix::to_RTC_abstime to do the relative to absolute 
>>>>> conversion using the existing routines in os::Posix used for 
>>>>> PlatformEvent etc.
>>>>>
>>>>> - changed callsites to use new millis version
>>>>>
>>>>> - I had to rearrange the ifndef SOLARIS in os_posix.cpp to expose 
>>>>> previously hidden code to Solaris. It isn't all needed by Solaris 
>>>>> but I chose not to clutter things with too many ifdefs and only 
>>>>> excluded things that really shouldn't be done on Solaris. Also 
>>>>> needed to call os::Posix::init on Solaris.
>>>>>
>>>>> Testing (still in progress):
>>>>> - mach 5 tiers 1 - 3 (Linux, Solaris, OS X, Windows)
>>>>> - test/jdk/jdk/jfr (as suspend/resume is the user of the semaphore 
>>>>> code)
>>>>>
>>>>> Thanks,
>>>>> David
>>>>
>