On Mon, 4 Mar 2024 03:20:41 GMT, David Holmes <dhol...@openjdk.org> wrote:
>> Serguei Spitsyn has updated the pull request incrementally with one >> additional commit since the last revision: >> >> rename after merge: jvmti_common.h to jvmti_common.hpp > > src/hotspot/share/prims/jvmtiEnvBase.cpp line 1505: > >> 1503: // the monitor threads after being notified. Here we are >> correcting the actual >> 1504: // number of the waiting threads by excluding those >> re-entering the monitor. >> 1505: nWait = i; > > Sorry I can't follow the logic here. Why is the whole block not simply: > > if (mon != nullptry) { > // The nWait count may include threads trying to re-enter the monitor, so > // walk the actual set of waiters to count those actually waiting. > int count = 0; > for (ObjectMonitor * waiter = mon->first_waiter(); waiter != nullptr; > waiter = mon->next_waiter(waiter)) { > count++; > } > } > ret.notify_waiter_count = count; Thank you for the suggestion. Let me check this. > src/hotspot/share/prims/jvmtiEnvBase.cpp line 1552: > >> 1550: // If the thread was found on the ObjectWaiter list, then >> 1551: // it has not been notified. This thread can't change the >> 1552: // state of the monitor so it doesn't need to be suspended. > > Not sure why suspension is being mentioned here. This is the original comment. I also do not quite understand it. It is confusing for sure. I can remove it if Dan does not object. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/17680#discussion_r1510906350 PR Review Comment: https://git.openjdk.org/jdk/pull/17680#discussion_r1510904664