RFR: 8145725: Remove the WorkAroundNPTLTimedWaitHang workaround
David Holmes
david.holmes at oracle.com
Tue Feb 9 01:17:17 UTC 2016
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