RFR: 8194860: Cleanup Semaphore timed-wait time calculations

[David Holmes]

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
>