RFR: 8328866: Add raw monitor rank support to the debug agent. [v4]
Chris Plummer
cjplummer at openjdk.org
Mon May 13 18:42:04 UTC 2024
On Thu, 9 May 2024 12:32:12 GMT, Serguei Spitsyn <sspitsyn at openjdk.org> wrote:
>> But 2 and 1 are very different. You can call them both leaf violations, but they are leaf violations for very different reasons, and 2 is more akin to a rank violation than a leaf violation.
>>
>> I'm reversing the ranks and reworking the loop a bit (both the comments and how the errors are reported). I'll try to post later tonight after testing is done.
>
> This is updated function `assertOrderFailure()` to print "RANK ORDER" failure for the case #2 above:
>
> static void
> assertOrderFailure(jthread thread, DebugRawMonitorRank rank, DebugRawMonitor* dbg_monitor, char* msg)
> {
> char* threadName = getThreadName(thread);
> tty_message("DebugRawMonitor %s failure: (%s: %d > %d) for thread (%s)",
> msg, dbg_monitor->name, dbg_monitor->rank, rank, threadName);
>
> if (rank < FIRST_LEAF_DEBUG_RAW_MONITOR &&
> dbg_monitor ->rank >= FIRST_LEAF_DEBUG_RAW_MONITOR) {
> tty_message("DebugRawMonitor RANK ORDER failure: (%s: %d > %d) for thread (%s)",
> msg, dbg_monitor->name, dbg_monitor->rank, rank, threadName);
> }
> jvmtiDeallocate(threadName);
> JDI_ASSERT(JNI_FALSE);
> }
I've given some more thought to this "leaf monitor" concept and decided it's not worth having around. The only thing that results in branding a monitor as a leaf monitor is the fact that no additional monitors are locked while the leaf monitor is held. This is not actually a requirement, but simply an observation made for these monitors. If any additional locking was introduced while holding a leaf monitor, that is not necessarily a bad thing. The list of leaf monitors is simply what was left over at the bottom of the rankings after all monitors that needed a higher rank were given one (and this was the result of an iterative process of running tests and seeing what asserts were triggered because of misranking some monitors). This list started out long, but after fixing all the rank issues we ended up with just 4.
-------------
PR Review Comment: https://git.openjdk.org/jdk/pull/19044#discussion_r1598892053
More information about the serviceability-dev
mailing list