8203341: Add a safepoint-aware Semaphore

[Stefan Karlsson]

I got offline suggestions that I should rename the function to 
wait_with_safepoint_check:

http://cr.openjdk.java.net/~stefank/8203341/webrev.03/
http://cr.openjdk.java.net/~stefank/8203341/webrev.03.delta/

Thanks,
StefanK

On 2018-05-18 14:21, Erik Österlund wrote:
> Hi Stefan,
> 
> Looks good.
> 
> Thanks,
> /Erik
> 
> On 2018-05-18 14:13, Stefan Karlsson wrote:
>> 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
>>>>
>>>
>