RFR(T): 8251543: add mention of INFLATING() to share/oops/markWord.hpp header comment
Daniel D. Daugherty
daniel.daugherty at oracle.com
Mon Aug 17 13:26:29 UTC 2020
On 8/16/20 10:42 PM, David Holmes wrote:
> 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. :(
No worries.
> 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.
When I first wrote the comment, I mentioned that INFLATING was all zeroes.
Later I went back and reread the comment and guessed that you wouldn't like
that I mentioned that level of detail. Kind of like when I add comments that
detail underlying memory barrier ops and you've asked me to be less
specific.
I can file a new bug to add the following to that "table":
// [0 ... 0| 00] inflating inflation in progress
> Sorry that my expectations were not clear enough.
No worries. I probably should have waited to hear from you since you
asked for the comment.
Dan
>
> 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
>>>
>>
More information about the hotspot-runtime-dev
mailing list