RFR(S) Contended Locking cleanup bucket (8062851)

[Daniel D. Daugherty]

On 11/5/14 5:16 PM, David Holmes wrote:
> On 6/11/2014 1:49 AM, Claes Redestad wrote:
>> Hi,
>>
>> On 11/05/2014 05:34 AM, Daniel D. Daugherty wrote:
>>> Greetings,
>>>
>>> I have a Contended Locking cleanup bucket fix ready for review.
>>>
>>> This fix was spun off from the Contended Locking fast enter bucket
>>> which was sent out for review late last week. This fix cleans up
>>> the computation of ObjectMonitor field pointers and gets rid of
>>> the use of literal '-2' in appropriate places. For example:
>>>
>>> -         ld_ptr(Rmark, ObjectMonitor::owner_offset_in_bytes() - 2,
>>> Rscratch);
>>> +         ld_ptr(Rmark, OM_OFFSET_NO_MONITOR_VALUE(owner), Rscratch);
>>>
>>> The OM_OFFSET_NO_MONITOR_VALUE macro computes the offset to the
>>> specified field and subtracts markOopDesc:monitor_value (2).
>>> There's a nice comment in src/share/vm/runtime/objectMonitor.hpp.
>>
>> any reason not to add it as a function in objectMonitor.hpp instead of a
>> macro? How about:
>>
>>    static int no_monitor_offset_in_bytes()  { return
>> offset_of(ObjectMonitor, _owner) - markOopDesc::monitor_value; }
>
> _owner is not the only field used so you would need a function for 
> each one.

David, thanks for jumping in on this part of the review thread.
I ended up being off-the-air yesterday from mid-morning on.

Claes, David is correct that your suggestion would require a function
for each field that we use currently and for completeness should have
a function for each field that has an offset_in_bytes() function. We
have 12 fields in ObjectMonitor for which we provide an offset_in_bytes()
so I don't think we want to go down that route.

Dan


>
> David
> -----
>
>> Example usage:
>>
>> -         ld_ptr(Rmark, ObjectMonitor::owner_offset_in_bytes() - 2,
>> Rscratch);
>> +         ld_ptr(Rmark, ObjectMonitor::no_monitor_offset_in_bytes(),
>> Rscratch);
>>
>>
>> Seems this should be inlined regardless and looks a bit cleaner to me.
>>
>> Thanks!
>>
>> /Claes
>>
>>>
>>> Thanks to David Holmes for his comments on JDK-8061553 that
>>> motivated this (long overdue) cleanup.
>>>
>>> This work is being tracked by the following bug ID:
>>>
>>>     JDK-8062851 cleanup ObjectMonitor offset adjustments
>>>     https://bugs.openjdk.java.net/browse/JDK-8062851
>>>
>>> Here is the webrev URL:
>>>
>>> http://cr.openjdk.java.net/~dcubed/8062851-webrev/0-jdk9-hs-rt/
>>>
>>> Here is the JEP link:
>>>
>>>     https://bugs.openjdk.java.net/browse/JDK-8046133
>>>
>>> Testing:
>>>
>>> - JPRT test jobs (since this is only syntax and comment cleanup)
>>>
>>> Thanks, in advance, for any comments, questions or suggestions.
>>>
>>> Dan
>>
>