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

[Erik Österlund]

Hi Dan,

Looks good to me.

/Erik

On 2020-06-02 21:23, 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
>