RFR(S/M): 8217659 monitor_logging updates from Async Monitor Deflation project

[coleen.phillimore at oracle.com]

This looks good!  Thank you for doing the refactoring!
Coleen

On 1/29/19 4:26 PM, Daniel D. Daugherty wrote:
> I've snipped this reply down to just the part about refactoring...
>
>
> On 1/28/19 5:22 PM, Daniel D. Daugherty wrote:
>> On 1/28/19 5:15 PM, coleen.phillimore at oracle.com wrote:
>>>>
>>>>> My eyes are terrible, but it looks like this is 
>>>>> ObjectMonitor::verify_free()
>>>>
>>>>> + if (n->is_busy()) {
>>>>> + out->print_cr("ERROR: monitor=" INTPTR_FORMAT ": free global 
>>>>> monitor "
>>>>> + "must not be busy.", p2i(n));
>>>>> + *error_cnt_p = *error_cnt_p + 1;
>>>>> + }
>>>>> + if (n->header() != NULL) {
>>>>> + out->print_cr("ERROR: monitor=" INTPTR_FORMAT ": free global 
>>>>> monitor "
>>>>> + "must have NULL _header field: _header=" INTPTR_FORMAT,
>>>>> + p2i(n), p2i(n->header()));
>>>>> + *error_cnt_p = *error_cnt_p + 1;
>>>>> + }
>>>>> + if (n->object() != NULL) {
>>>>> + out->print_cr("ERROR: monitor=" INTPTR_FORMAT ": free global 
>>>>> monitor "
>>>>> + "must have NULL _object field: _object=" INTPTR_FORMAT,
>>>>> + p2i(n), p2i(n->object()));
>>>>> + *error_cnt_p = *error_cnt_p + 1;
>>>>> + } ...
>
> I have refactored the common code for "verify free" into:
>
> +// Check a free monitor entry; log any errors.
> +void ObjectSynchronizer::ck_free_entry(JavaThread * jt, ObjectMonitor 
> * n,
> +                                       outputStream * out, int 
> *error_cnt_p) {
>
> And here's what one of the checks looks like. I tried to put it all into
> a single out->print_cr() call with conditional inclusion of format 
> strings
> and parameters, but the compiler really hated it. So I went with:
>
> +  if (n->is_busy()) {
> +    if (jt != NULL) {
> +      out->print_cr("ERROR: jt=" INTPTR_FORMAT ", monitor=" 
> INTPTR_FORMAT
> +                    ": free per-thread monitor must not be busy.", 
> p2i(jt),
> +                    p2i(n));
> +    } else {
> +      out->print_cr("ERROR: monitor=" INTPTR_FORMAT ": free global 
> monitor "
> +                    "must not be busy.", p2i(n));
> +    }
> +    *error_cnt_p = *error_cnt_p + 1;
> +  }
>
>
>
>>>>> And this is ObjectMonitor::verify_in_use()
>>>>>
>>>>> + if (n->header() == NULL) {
>>>>> + out->print_cr("ERROR: monitor=" INTPTR_FORMAT ": in-use global 
>>>>> monitor "
>>>>> + "must have non-NULL _header field.", p2i(n));
>>>>> + *error_cnt_p = *error_cnt_p + 1;
>>>>> + }
>>>>> + if (n->object() == NULL) {
>>>>> + out->print_cr("ERROR: monitor=" INTPTR_FORMAT ": in-use global 
>>>>> monitor "
>>>>> + "must have non-NULL _object field.", p2i(n));
>>>>> + *error_cnt_p = *error_cnt_p + 1;
>>>>> + } ...
>
> I have refactored the common code for "verify in use" into:
>
> +// Check an in-use monitor entry; log any errors.
> +void ObjectSynchronizer::ck_in_use_entry(JavaThread * jt, 
> ObjectMonitor * n,
> +                                         outputStream * out, int 
> *error_cnt_p) {
>
> And here's what one of the checks looks like:
>
> +  if (n->header() == NULL) {
> +    if (jt != NULL) {
> +      out->print_cr("ERROR: jt=" INTPTR_FORMAT ", monitor=" 
> INTPTR_FORMAT
> +                    ": in-use per-thread monitor must have non-NULL 
> _header "
> +                    "field.", p2i(jt), p2i(n));
> +    } else {
> +      out->print_cr("ERROR: monitor=" INTPTR_FORMAT ": in-use global 
> monitor "
> +                    "must have non-NULL _header field.", p2i(n));
> +    }
> +    *error_cnt_p = *error_cnt_p + 1;
> +  }
>
> Hopefully this is the last refactor for this bug fix... :-)
>
>
> Here's an incremental webrev:
>
> http://cr.openjdk.java.net/~dcubed/8217659-webrev/2-for-jdk13.inc/
>
> Here's a full webrev:
>
> http://cr.openjdk.java.net/~dcubed/8217659-webrev/2-for-jdk13.full/
>
> Another Mach5 builds-tier1,hs-tier1,jdk-tier1,hs-tier2,hs-tier3 test
> run is in process...
>
> Thanks, in advance, for any questions, comments or suggestions.
>
> Dan
>
>> The details are in the bug report:
>>
>>     JDK-8217659 monitor_logging updates from Async Monitor Deflation 
>> project
>> https://bugs.openjdk.java.net/browse/JDK-8217659
>