RFR: 8194860: Cleanup Semaphore timed-wait time calculations

David Holmes david.holmes at oracle.com
Thu Jan 17 12:50:26 UTC 2019


<sigh> Something has gone wrong with the OS X version - the VM won't 
run. Will take this up again next week.

David
-----

On 17/01/2019 5:01 pm, David Holmes wrote:
> Take 2: webrev updated in place.
> 
> For OSXSemaphore I simply changed the existing timedwait method to take 
> the single millis arg as used by the new PosixSemaphore so that calling 
> code is correct for both cases. I also deleted the unnecessary 
> currenttime() method. I chose not to implement the absolute timedwait 
> because the conversion to a relative wait as needed by the kernel 
> semaphore is non-trivial and as its unused it would be untested.
> 
> David
> 
> On 17/01/2019 2:58 pm, David Holmes wrote:
>> Withdrawn temporarily.
>>
>> The OS X changes were still being tested and they don't work. For some 
>> reason we're using kernel semaphores on OS X and they seem to take a 
>> relative timeout directly. That explains why they had a different API 
>> but doesn't help with the fact that the calling code expects 
>> OSXSemaphore and PosixSemaphore to have the same API.
>>
>> David
>>
>> On 17/01/2019 8:06 am, 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


More information about the hotspot-runtime-dev mailing list