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

[David Holmes]

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
>