RFR (M) #5 CR 8003985: Support @Contended annotation

[Vladimir Kozlov]

Aleksey,

Please, file separate bug for jdk changes, create separate webrev and 
send it to core-libs-dev at openjdk.java.net for review (CC to 
hotspot-dev). And do it for future changes which involve jdk changes.

I did not look on this code before, I think.

You can set ContendedPaddingWidth lower and redefine it in 
vm_version_<arch>.cpp:

   if (cache_line_size > ContendedPaddingWidth)
     ContendedPaddingWidth = cache_line_size;

On other hand 128 value is very conservative and should satisfy all 
current platform VM run on. But you will wast java heap space.

There is no explanation in code what contending groups are. The only 
comment I see:

+          if (fs.contended_group() == 0) {
+            // special case, this default group is padded within

Why other groups don't need padding between fields?

Also why you padding after fields again? You did padding before fields 
already? You can wasting space at the end of object or in between super 
and subclass fields.

In vmSymbols.hpp Contended signature should not be in "error klasses" 
group. Separate it by comment.

In the test change copyright year to 2013. Also @summary should be bug's 
"Support Contended Annotation - JEP 142"

Thanks,
Vladimir

On 1/10/13 3:20 PM, Aleksey Shipilev wrote:
> Thanks Jesper!
>
> On 01/11/2013 03:15 AM, Jesper Wilhelmsson wrote:
>> I have looked through the change and I don't see anything obviously
>> wrong with it, but this change is in a part of HotSpot that I don't know
>> at all so I wouldn't count this as a review.
>>
>> You are adding a TODO in classFileParser.cpp , is that intentional?
>
> Yes, we would need to follow up on that once we have more usages, which
> will allow us to experiment more thoroughly on this.
>
>> I have followed the discussions around @contended over the last year and
>> I feel comfortable sponsoring this change once it has been approved by
>> proper reviewers.
>
> Good. The code was not changed since last review; the only culprit was
> pending CCC, and now it is approved. I think Vladimir, John, and Coleen
> were OK. Guys, can you confirm you still OK?
>
> -Aleksey.
>