RFR(S): 8222295 more baseline cleanups from Async Monitor Deflation project

[Daniel D. Daugherty]

Filed the following new bug:

     JDK-8222893 markOopDesc::print_on() is a bit confused
     https://bugs.openjdk.java.net/browse/JDK-8222893

Coleen, please let me know if I've captured the confusion here... :-)

Dan

P.S.
What can I say? It's code that deals with mark oops, on-stack locks,
biased locks and inflated locks... If there was ever code that had
a right to be confused... ROFL...


On 4/23/19 12:36 PM, Daniel D. Daugherty wrote:
> On 4/23/19 11:41 AM, coleen.phillimore at oracle.com wrote:
>> http://cr.openjdk.java.net/~dcubed/8153224-webrev/4-for-jdk13.8222295/src/hotspot/share/oops/markOop.cpp.frames.html 
>>
>>
>>   37     if (mon == NULL) {
>>   38       st->print("NULL (this should never be seen!)");
>>   39     } else {
>> 40 st->print("{contentions=0x%08x,waiters=0x%08x"
>>   41                 ",recursions=" INTPTR_FORMAT ",owner=" 
>> INTPTR_FORMAT "}",
>> 42 mon->contentions(), mon->waiters(), mon->recursions(),
>>   43                 p2i(mon->owner()));
>>   44     }
>>
>>
>> Following convention, it seems like this code should be in 
>> ObjectMonitor::print_on(outputStream* st) so markOop doesn't have to 
>> know objectMonitor fields/accessors.
>
> That's a really interesting point... When you take a look at the
> whole of the markOopDesc::print_on() function, it is trying to
> give _some_ visibility into the interpretation of the various
> things that we have encoded into the mark oop word/header.
> For example, if the mark "is locked", it has this code:
>
>   45   } else if (is_locked()) {
>   46     st->print(" locked(" INTPTR_FORMAT ")->", value());
>   47     if (is_neutral()) {
>   48       st->print("is_neutral");
>   49       if (has_no_hash()) {
>   50         st->print(" no_hash");
>   51       } else {
>   52         st->print(" hash=" INTPTR_FORMAT, hash());
>   53       }
>   54       st->print(" age=%d", age());
>   55     } else if (has_bias_pattern()) {
>   56       st->print("is_biased");
>   57       JavaThread* jt = biased_locker();
>   58       st->print(" biased_locker=" INTPTR_FORMAT, p2i(jt));
>   59     } else {
>   60       st->print("??");
>   61     }
>
> and if the mark "is unlocked", it has this code:
>
>   62   } else {
>   63     assert(is_unlocked() || has_bias_pattern(), "just checking");
>   64     st->print("mark(");
>   65     if (has_bias_pattern()) st->print("biased,");
>   66     st->print("hash " INTPTR_FORMAT ",", hash());
>   67     st->print("age %d)", age());
>   68   }
>
> So I understand the reasons for the limited peek into the
> ObjectMonitor for the mark "has monitor" case since we do
> that limited level of detail for the other interpretations
> of the mark oop header.
>
> Summary: I'm not planning on changing that for this bug.
>
> However, now that I've pasted these code snippets, I think I
> see some confusion here. The mark "is locked" and mark "is unlocked"
> branches both have code for biased locking. That seems strange to
> me, but that should be looked at separately.
>
>
>> Otherwise looks like a good self-contained cleanup to me.
>
> Thanks! You'll see some of your other requested changes in the
> review thread for JDK-8153224 (CR1/v2.01/4-for-jdk13).
>
> Dan
>
>
>>
>> Coleen