8203341: Add a safepoint-aware Semaphore

[Stefan Karlsson]

Updated webrevs:
  http://cr.openjdk.java.net/~stefank/8203341/webrev.02/
  http://cr.openjdk.java.net/~stefank/8203341/webrev.02.delta/

I removed the SemaphoreSafepointAware class, as Erik requested, but 
instead of adding a bool I added a new function for the safepoint aware 
waits.

Thanks,
StefanK

On 2018-05-17 17:59, Stefan Karlsson wrote:
> On 2018-05-17 17:38, Erik Österlund wrote:
>> Hi Stefan,
>>
>> I wonder if it would make sense to let Semaphore::wait have a bool 
>> safepoint_check, similar to Mutex (possibly with a default value), 
>> instead of introducing a new class with the same API, that only 
>> differs in this property.
> 
> That sounds reasonable to me.
> 
> I also need to change my patch so that the thread-local handshake code 
> pass in the existing JavaThread, so that we don't have to call 
> Thread::current()->is_JavaThread() unnecessarily.
> 
> Thanks,
> StefanK
> 
>>
>> Thanks,
>> /Erik
>>
>> On 2018-05-17 09:46, Stefan Karlsson wrote:
>>> Hi all,
>>>
>>> Please review this patch to add a safepoint-aware Semaphore (and 
>>> refactor the semaphore usage in thread-local handshakes).
>>>
>>> http://cr.openjdk.java.net/~stefank/8203341/webrev.01/
>>> https://bugs.openjdk.java.net/browse/JDK-8203341
>>>
>>> Both ZGC and the thread-local handshakes need to move the JavaThread 
>>> to a blocked state before waiting on a Semaphore. I propose that we 
>>> introduce a new SemaphoreSafepointAware wrapper around Semaphore.
>>>
>>> There are two things to consider with this patch:
>>>
>>> 1) In ZGC, where this patch originates from, we set the JavaThread's 
>>> osthread state with osthread->set_state(CONDVAR_WAIT) (using 
>>> OSThreadWaitState). This means that we get the following printed when 
>>> the threads are dumped: "waiting on condition ". I've kept this 
>>> behavior for the SemaphoreSafepointAware class. This means that we 
>>> now change the osthread state for JavaThreads blocking in 
>>> HandshakeState::process_self_inner.
>>>
>>> 2) The thread-local handshakes uses trywait before entering wait:
>>>    if (!_semaphore.trywait()) {
>>> -    ThreadBlockInVM tbivm(thread);
>>>      _semaphore.wait();
>>>    }
>>>
>>> At the time when SemaphoreSafepointAware was written, we didn't have 
>>> trywait on all platforms, now we do. Maybe we should always do the 
>>> trywait dance inside SemaphoreSafepointAware::wait() ?
>>>
>>> inline void SemaphoreSafepointAware::wait() {
>>>   if (Thread::current()->is_Java_thread()) {
>>>     if (!_semaphore.trywait) {
>>>       JavaThread* thread = JavaThread::current();
>>>
>>>       // Prepare to block and allow safepoints while blocked
>>>       ThreadBlockInVM tbivm(thread);
>>>       OSThreadWaitState osts(thread->osthread(), false /* not in 
>>> Object.wait() */);
>>>
>>>       // Wait for value
>>>      _semaphore.wait();
>>>    }
>>>
>>> Thanks,
>>> StefanK
>>
>