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?
_recursions = 0;
ret = simple_wait(self, millis);
if (self->is_Java_thread()) { // JavaThread re-enter
JavaThread* jt = self->as_Java_thread();
{
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