RFR (S): CR 8012939: @Contended doesn't work correctly with inheritance

Vladimir Kozlov vladimir.kozlov at oracle.com
Tue May 14 16:24:41 PDT 2013


On 5/14/13 3:24 PM, Aleksey Shipilev wrote:
> Hi,
>
> It seems we have a plenty of failures in new SQE tests for @Contended
> because of this. Please see the updated webrev for the issue:
>    http://cr.openjdk.java.net/~shade/8012939/webrev.01/
>
> This is the minimal fix. Most of the instance_size calculations use
> first_nonstatic_field_offset as the boundary for the instance fields
> block. @Contended on the class hijacks this boundary with padding, and
> so the instance size is incorrect (also evidenced by the wrong assert).
>
> There is also a more thorough clean up of that code:
>    http://cr.openjdk.java.net/~shade/8012939/webrev.02/

I am fine with this version of the fix. But I would suggest more :) 
renaming. first_nonstatic_field_offset is not actually pointing to first 
nonstatic field, next_nonstatic_field_offset may point to it (it will 
not point to it if first field is contended (double padding)). Note that 
next_nonstatic_field_offset is not modified after we added pad to it. I 
am suggesting to rename first_nonstatic_field_offset to 
nonstatic_fields_start and next_nonstatic_field_offset to 
first_nonstatic_field_offset.

Also I think new variables names instead of aligned_*_offset should be:

  nonstatic_fields_end
  static_fields_end
  instance_end

Thanks,
Vladimir

>
> ...which is makes the intent cleaner. I can submit another RFE for that
> cleanup, although I think it prunes the source of the original
> confusion. Speak up if you want me to split these, and we push the
> webrev.01 alone then.
>
> Testing:
>    - 8003985 and 8012939 tests on Linux x86_64/fastdebug
>    - JPRT run for both webrevs
>
> Thanks,
> Aleksey.
>


More information about the hotspot-dev mailing list