RFR: 8145725: Remove the WorkAroundNPTLTimedWaitHang workaround

Dmitry Dmitriev dmitry.dmitriev at oracle.com
Tue Feb 9 08:06:42 UTC 2016


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
>>



More information about the hotspot-runtime-dev mailing list