On 20/05/2021 5:10 pm, Robbin Ehn wrote:
On Wed, 19 May 2021 14:25:41 GMT, Richard Reingruber <rr...@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
