RFR(S) Contended Locking cleanup bucket (8062851)

[Coleen Phillimore]

Dan,  I had a look at this change too.

On 11/5/14, 10:29 AM, Daniel D. Daugherty wrote:
> On 11/5/14 3:42 AM, David Holmes wrote:
>> Hi Dan,
>>
>> Reviewed.
>
> Thanks!
>
>
>> I find the name OM_OFFSET_NO_MONITOR_VALUE somewhat awkward but have 
>> no better suggestion.
>
> Understood. I didn't like the original "OFFSET_SKEWED" name
> especially since I was moving it to objectMonitor.hpp...
>
> If you think of a better, let me know... we can always change it.
>

So the -2 was a tag?  Then maybe a better name is UNTAGGED_OM_OFFSET ..  
Weird stuff anyway.

In 
http://cr.openjdk.java.net/~dcubed/8062851-webrev/0-jdk9-hs-rt/src/cpu/x86/vm/macroAssembler_x86.cpp.udiff.html

Can you make the whitespace changes to the lines you've changed:

+         movptr(tmpReg, Address (tmpReg, OM_OFFSET_NO_MONITOR_VALUE(owner)));   // rax, = m->_owner


to

+         movptr(tmpReg, Address(tmpReg, OM_OFFSET_NO_MONITOR_VALUE(owner)));   // rax, = m->_owner


In general, this looks like a great improvement not subtracting two from 
seemingly random places in assembly code.

thanks,
Coleen

>
>
>> In fact I have to ask what _is_ the object monitor tagging mechanism? 
>> I can't see it defined in the objectMonitor.* files. ??
>
> That would be this code:
>
> src/share/vm/oops/markOop.hpp:
>
>     317   static markOop encode(ObjectMonitor* monitor) {
>     318     intptr_t tmp = (intptr_t) monitor;
>     319     return (markOop) (tmp | monitor_value);
>     320   }
>
> and the other methods in that file that have to account for
> the monitor_value being set...
>
> Dan
>
>
>>
>> Thanks,
>> David
>>
>> On 5/11/2014 2:34 PM, 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.
>>>
>>> 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
>