RFR (M): CR 8003985: Support @Contended Annotation - JEP 142

[David Holmes]

On 30/11/2012 4:56 AM, Aleksey Shipilev wrote:
> Thanks for review, Christian!
>
> On 11/29/2012 10:28 PM, Christian Thalinger wrote:
>>> The webrev for the change is here:
>>>   http://shipilev.net/pub/jdk/hotspot/contended/webrev-4/
>>
>> Is this a review for a push?
>
> Yes, I'm pretty much done with functionality, it passes the tests, and
> flies through JPRT without problems.

This work can not be pushed until the JEP has moved to the Funded state.

Meanwhile we should refine any coding issues regarding 
style/indents/copyrights etc.

I'd also like to know what testing has been done so far from the regular 
hotspot testsuites. There may be places where layout assumptions are 
being made eg oops first.

Note this is not a Review - I'm not familiar enough with this code to 
review, sorry.

Thanks,
David
-----


>> hotspot/src/share/vm/classfile/classFileParser.cpp:
>>
>> Is it possible to factor the while-loop to align the fields into a method?
>
> The whole method ClassFileParser::parseClassFile cries for refactoring.
> Alas, separating the contended handling code would be more messy than
> just fitting in the same flow, since it uses quite of few locals within
> that method.
>
>> hotspot/src/share/vm/classfile/classFileParser.hpp:
>>
>> +    bool hasContendedAnnotation() { return has_annotation(_sun_misc_Contended); }
>>
>> We don't use camel-case method names.
>
> OK.
>
>> hotspot/src/share/vm/classfile/vmSymbols.hpp:
>>
>> Please keep the indent aligned.
>
> OK.
>
>> hotspot/src/share/vm/oops/fieldInfo.hpp:
>>
>> I wonder if we have a spare access_flags bit somewhere we could use for is_contended.
>
> Looking at jvm.h, I see there is only single bit left, want me to burn
> it with contended flag?
>
>> jdk/src/share/classes/sun/misc/Contended.java:
>>
>> Misses copyright header.
>
> Will update.
>
> -Aleksey.
>