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? _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