RFR(T): 8251543: add mention of INFLATING() to share/oops/markWord.hpp header comment

[David Holmes]

Hi Dan,

On 16/08/2020 12:04 am, Daniel D. Daugherty wrote:
> Tap, tap, tap... is this thing working? Any takers for a trivial review?

I had a long weekend. :(

I see you pushed this but it isn't really what I was looking for. 
Specifically I wanted the fact that an all-zero header meant "inflating" 
would be clear from the added text. At the moment if I see:

//    [ptr             | 00]  locked             ptr points to real 
header on stack

//    [header      | 0 | 01]  unlocked           regular object header

//    [ptr             | 10]  monitor            inflated lock (header 
is wapped out)

//    [ptr             | 11]  marked             used to mark an object

then it appears that checking for 00 in lsb is sufficient to identify a 
locked object. But that isn't true because all zeros means inflating. I 
had hoped to see that explicitly added e.g.

//    [0      ...    0| 00]  inflating             inflation in progress

then your new text. At a minimum I would have liked the new text to 
actually state that the INFLATING value is all zeroes.

Sorry that my expectations were not clear enough.

David
-----

> Dan
> 
> 
> On 8/13/20 5:16 PM, Daniel D. Daugherty wrote:
>> In a previous cleanup bug, we normalized 
>> src/share/runtime/synchronizer.cpp to
>> use "stack-lock" instead of "stack lock" and other variants. I'm going 
>> to change
>> the two "stack lock" uses to "stack-lock" below.
>>
>> Dan
>>
>>
>> On 8/13/20 10:58 AM, Daniel D. Daugherty wrote:
>>> Greetings,
>>>
>>> I have a trivial comment addition to 
>>> src/hotspot/share/oops/markWord.hpp.
>>>
>>> Here's the bug:
>>>
>>>     JDK-8251543 add mention of INFLATING() to share/oops/markWord.hpp 
>>> header comment
>>>     https://bugs.openjdk.java.net/browse/JDK-8251543
>>>
>>> And here's the proposed fix:
>>>
>>> $ hg diff
>>> diff -r 73778bfbffe3 src/hotspot/share/oops/markWord.hpp
>>> --- a/src/hotspot/share/oops/markWord.hpp    Thu Aug 13 10:02:35 2020 
>>> -0400
>>> +++ b/src/hotspot/share/oops/markWord.hpp    Thu Aug 13 10:51:44 2020 
>>> -0400
>>> @@ -86,6 +86,10 @@
>>>  //    [ptr             | 11]  marked             used to mark an object
>>>  //
>>>  //    We assume that stack/thread pointers have the lowest two bits 
>>> cleared.
>>> +//
>>> +//  - INFLATING() is a distinguished markword value that is used when
>>> +//    inflating an existing stack lock into an ObjectMonitor. See below
>>> +//    for is_being_inflated() and INFLATING().
>>>
>>>  class BasicLock;
>>>  class ObjectMonitor;
>>> @@ -226,7 +230,7 @@
>>>    bool is_being_inflated() const { return (value() == 0); }
>>>
>>>    // Distinguished markword value - used when inflating over
>>> -  // an existing stacklock.  0 indicates the markword is "BUSY".
>>> +  // an existing stack lock.  0 indicates the markword is "BUSY".
>>>    // Lockword mutators that use a LD...CAS idiom should always
>>>    // check for and avoid overwriting a 0 value installed by some
>>>    // other thread.  (They should spin or block instead.  The 0 value
>>>
>>>
>>> Thanks, in advance, for any comments, questions or suggestions.
>>>
>>> Dan
>>
>