RFR: 8265753: Remove manual JavaThread transitions to blocked [v4]

[David Holmes]

On 20/05/2021 5:10 pm, Robbin Ehn wrote:
> On Wed, 19 May 2021 14:25:41 GMT, Richard Reingruber <rrich at openjdk.org> wrote:
> 
>> Just one more, rather unimportant comment...
>>
>> Either way: LGTM!
>>
>> Thanks, Richard.
> 
> Thanks!
> 
>> src/hotspot/share/prims/jvmtiRawMonitor.cpp line 382:
>>
>>> 380:
>>> 381:     _recursions = 0;
>>> 382:     ret = simple_wait(self, millis);
>>
>> IMHO the guarantee at L379 is redundant with the newly added identical guarantee in `JvmtiRawMonitor::simple_wait()` at L240.
>>
>> In case you agree to remove the guarantee, I don't see why the following statements cannot be pulled out of the if-statement.
>>
>>      _recursions = 0;
>>      ret = simple_wait(self, millis);
>>      _recursions = save;
> 
> Yes, it can be re-written as something below, but is this more readable?

I'd say yes with two minor modifications:

> 
>    _recursions = 0;
>    ret = simple_wait(self, millis);

+ // Now we need to re-enter the monitor. For JavaThread's
+ // we need to manage suspend requests.

>    if (self->is_Java_thread()) { // JavaThread re-enter
>      JavaThread* jt = self->as_Java_thread();
>      {

I think this extra block scope can also go.

Cheers,
David
-----

>        ThreadInVMfromNative tivmfn(jt);
>        for (;;) {
>          ExitOnSuspend eos(this);
>          {
>            ThreadBlockInVMPreprocess<ExitOnSuspend> tbivmp(jt, eos);
>            simple_enter(jt);
>          }
>          if (!eos.monitor_exited()) {
>            break;
>          }
>        }
>        if (jt->is_interrupted(true)) {
>          ret = M_INTERRUPTED;
>        }
>      }
>    } else { // Non-JavaThread re-enter
>      assert(ret != M_INTERRUPTED, "Only JavaThreads can be interrupted");
>      simple_enter(self);
>    }
>    _recursions = save;
> 
> -------------
> 
> PR: https://git.openjdk.java.net/jdk/pull/3875
>