RFR: 8194860: Cleanup Semaphore timed-wait time calculations

[coleen.phillimore at oracle.com]

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
>>>