RFR(T): 8246359: clarify confusing comment in ObjectMonitor::EnterI()'s race with async deflation

[Daniel D. Daugherty]

Thanks David!

Dan


On 6/2/20 7:05 PM, David Holmes wrote:
> Looks good!
>
> Thanks,
> David
>
> On 3/06/2020 5:23 am, Daniel D. Daugherty wrote:
>> Greetings,
>>
>> I have a trivial update to a comment that was added by the following fix
>> that I pushed late yesterday:
>>
>>      JDK-8153224 Monitor deflation prolong safepoints
>>      https://bugs.openjdk.java.net/browse/JDK-8153224
>>
>> Here's the tracking bug:
>>
>>      JDK-8246359 clarify confusing comment in ObjectMonitor::EnterI()'s
>>                  race with async deflation
>>      https://bugs.openjdk.java.net/browse/JDK-8153224
>>
>> The comment update is to resolve a 8153224 code review comment from
>> Carsten. He's already reviewed the comment on the 8153224 review thread
>> and is happy with the new comment.
>>
>>
>> And here's the context diffs for the fix:
>>
>> $ hg diff
>> diff -r 629b14c63b75 src/hotspot/share/runtime/objectMonitor.cpp
>> --- a/src/hotspot/share/runtime/objectMonitor.cpp    Mon Jun 01 
>> 23:37:14 2020 -0400
>> +++ b/src/hotspot/share/runtime/objectMonitor.cpp    Tue Jun 02 
>> 13:54:32 2020 -0400
>> @@ -526,12 +526,17 @@
>>
>>     if (AsyncDeflateIdleMonitors &&
>>         try_set_owner_from(DEFLATER_MARKER, Self) == DEFLATER_MARKER) {
>> -    // Cancelled the in-progress async deflation. We bump 
>> contentions an
>> -    // extra time to prevent the async deflater thread from temporarily
>> -    // changing it to -max_jint and back to zero (no flicker to confuse
>> -    // is_being_async_deflated()). The async deflater thread will
>> -    // decrement contentions after it recognizes that the async
>> -    // deflation was cancelled.
>> +    // Cancelled the in-progress async deflation by changing owner from
>> +    // DEFLATER_MARKER to Self. As part of the contended enter 
>> protocol,
>> +    // contentions was incremented to a positive value before EnterI()
>> +    // was called and that prevents the deflater thread from winning 
>> the
>> +    // last part of the 2-part async deflation protocol. After EnterI()
>> +    // returns to enter(), contentions is decremented because the 
>> caller
>> +    // now owns the monitor. We bump contentions an extra time here to
>> +    // prevent the deflater thread from winning the last part of the
>> +    // 2-part async deflation protocol after the regular decrement
>> +    // occurs in enter(). The deflater thread will decrement 
>> contentions
>> +    // after it recognizes that the async deflation was cancelled.
>>       add_to_contentions(1);
>>       assert(_succ != Self, "invariant");
>>       assert(_Responsible != Self, "invariant");
>> @@ -656,12 +661,17 @@
>>
>>       if (AsyncDeflateIdleMonitors &&
>>           try_set_owner_from(DEFLATER_MARKER, Self) == 
>> DEFLATER_MARKER) {
>> -      // Cancelled the in-progress async deflation. We bump 
>> contentions an
>> -      // extra time to prevent the async deflater thread from 
>> temporarily
>> -      // changing it to -max_jint and back to zero (no flicker to 
>> confuse
>> -      // is_being_async_deflated()). The async deflater thread will
>> -      // decrement contentions after it recognizes that the async
>> -      // deflation was cancelled.
>> +      // Cancelled the in-progress async deflation by changing owner 
>> from
>> +      // DEFLATER_MARKER to Self. As part of the contended enter 
>> protocol,
>> +      // contentions was incremented to a positive value before 
>> EnterI()
>> +      // was called and that prevents the deflater thread from 
>> winning the
>> +      // last part of the 2-part async deflation protocol. After 
>> EnterI()
>> +      // returns to enter(), contentions is decremented because the 
>> caller
>> +      // now owns the monitor. We bump contentions an extra time 
>> here to
>> +      // prevent the deflater thread from winning the last part of the
>> +      // 2-part async deflation protocol after the regular decrement
>> +      // occurs in enter(). The deflater thread will decrement 
>> contentions
>> +      // after it recognizes that the async deflation was cancelled.
>>         add_to_contentions(1);
>>         break;
>>       }
>>
>> Thanks, in advance, for any comments, questions or suggestions.
>>
>> Dan
>>