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