RFR (S) CR 8014964: @Contended breaks has_nonstatic_fields invariant

[Vladimir Kozlov]

On 5/22/13 2:16 PM, Aleksey Shipilev wrote:
> Thanks for the ideas!
>
> On 05/21/2013 08:14 PM, Vladimir Kozlov wrote:
>>> Could you move third check on next line so you can see all on one screen?
>
> Done.
>
>>> You are right. So parsed_annotations->is_contended() is true for any
>>> combinations of class and fields annotation. Right?
>
> (parsed_annotations->is_contended() == true) iff (@Contended on class).
>
> This is yet another thing to clean up.

Okay.

>>> This could be affected by alignment even more than your assert. Could
>>> you look?
>
> Hm... Ok, I can compare against the non-aligned offset. It can only not
> equal the start if we do indeed have the fields.

Good idea with notaligned_*_end variable.

>
> But nonstatic_field_size still has the same problem. Moreover, assert
> would seem to fail even in the simple case when the super-class is
> @Contended; added the test case for that (class R4).
>
> This is the current webrev:
>    http://cr.openjdk.java.net/~shade/8014964/webrev.03/

Looks good.

>
> Testing:
>    - hotspot/test/runtime/contended/ regression tests (failing)

Could you run all hotspot/test tests (you have to list them)? It should 
not take long.

>    - JPRT cycle vs hotspot-rt is running now  (just in case)
>
> I actually start to wonder if no size/offset is reliable for this
> assert; and counting the fields will yield the same expression over
> fac[...], which is dumb to assert. The asserts there are only to fire
> the regression test. Hence, this might seem to be that rare case where
> the regression test is impractical?

The asserts and test are a protection from future bugs in this code (as 
most of other asserts in VM). This code is complex and is easy to screw 
up :)

Thanks,
Vladimir

>
> -Aleksey.
>