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