RFR: 8145725: Remove the WorkAroundNPTLTimedWaitHang workaround

[David Holmes]

Thanks Dmitry!

Can I have a second Reviewer please.

David

On 9/02/2016 6:06 PM, Dmitry Dmitriev wrote:
> Hello David,
>
> Looks good to me!
>
> Thanks,
> Dmitry
>
> On 09.02.2016 4:17, David Holmes wrote:
>> Hi Dmitry,
>>
>> Great cleanup suggestions. I also did some missing assert ->
>> assert_status conversions for the pthread_* functions. (And reverted a
>> change that shouldn't have been made - assert_status is for when the
>> "status" is the error code.)
>>
>> New webrev: http://cr.openjdk.java.net/~dholmes/8145725/webrev.v2/
>>
>> Thanks,
>> David
>> -----
>>
>> On 8/02/2016 11:24 PM, Dmitry Dmitriev wrote:
>>> Hello David,
>>>
>>> Looks good to me. Just few additional clean-ups which you can consider
>>> for implementing.
>>> 1) hotspot/src/os/bsd/vm/os_bsd.cpp module
>>>
>>> I think that you can remove duplicated logic for mutex unlock from
>>> Parker::unpark function.
>>>
>>> I.e. transform it from:
>>> 4376 void Parker::unpark() {
>>> ...
>>> 4381   if (s < 1) {
>>> 4382     status = pthread_mutex_unlock(_mutex);
>>> 4383     assert_status(status == 0, status, "invariant");
>>> 4384     status = pthread_cond_signal(_cond);
>>> 4385     assert_status(status == 0, status, "invariant");
>>> 4386   } else {
>>> 4387     pthread_mutex_unlock(_mutex);
>>> 4388     assert_status(status == 0, status, "invariant");
>>> 4389   }
>>> 4390 }
>>>
>>> To something like that:
>>> void Parker::unpark() {
>>> ...
>>>    status = pthread_mutex_unlock(_mutex);
>>>    assert_status(status == 0, status, "invariant");
>>>    if (s < 1) {
>>>      status = pthread_cond_signal(_cond);
>>>      assert_status(status == 0, status, "invariant");
>>>    }
>>>   }
>>>
>>> 2) hotspot/src/os/linux/vm/os_linux.cpp
>>>
>>> Similar refactoring for Parker::unpark for removing duplicated logic for
>>> mutex unlock.
>>>
>>> From:
>>> 5720 void Parker::unpark() {
>>> ...
>>> 5725   if (s < 1) {
>>> 5726     // thread might be parked
>>> 5727     if (_cur_index != -1) {
>>> 5728       // thread is definitely parked
>>> 5729       // must capture correct index before unlocking
>>> 5730       int index = _cur_index;
>>> 5731       status = pthread_mutex_unlock(_mutex);
>>> 5732       assert(status == 0, "invariant");
>>> 5733       status = pthread_cond_signal(&_cond[index]);
>>> 5734       assert(status == 0, "invariant");
>>> 5735     } else {
>>> 5736       pthread_mutex_unlock(_mutex);
>>> 5737       assert(status == 0, "invariant");
>>> 5738     }
>>> 5739   } else {
>>> 5740     pthread_mutex_unlock(_mutex);
>>> 5741     assert(status == 0, "invariant");
>>> 5742   }
>>> 5743 }
>>>
>>> To:
>>>
>>> void Parker::unpark() {
>>> ...
>>>    if ((s < 1) && (_cur_index != -1)) {
>>>        // thread is definitely parked
>>>        // must capture correct index before unlocking
>>>        int index = _cur_index;
>>>        status = pthread_mutex_unlock(_mutex);
>>>        assert(status == 0, "invariant");
>>>        status = pthread_cond_signal(&_cond[index]);
>>>        assert(status == 0, "invariant");
>>>     } else {
>>>       pthread_mutex_unlock(_mutex);
>>>       assert(status == 0, "invariant");
>>>     }
>>> }
>>>
>>>
>>> Thanks,
>>> Dmitry
>>>
>>> On 08.02.2016 10:16, David Holmes wrote:
>>>> webrev: http://cr.openjdk.java.net/~dholmes/8145725/webrev/
>>>>
>>>> bug: https://bugs.openjdk.java.net/browse/JDK-8145725
>>>>
>>>> In JDK-8130728 we disabled the workaround by default and have not seen
>>>> any issues (other than JDK-8029453). So now the workaround can be
>>>> removed completely.
>>>>
>>>> Thanks,
>>>> David
>>>
>