RFR (XS): 8021823 G1: Concurrent marking crashes with -XX:ObjectAlignmentInBytes>=32 in 64bit VMs (patch for hs24+hs25)

[Vladimir Kozlov]

The fix looks good.

About test. You don't need "@run main/othervm TestObjectAlignment" line. 
First, you don't need /othervm since you don't use any options with main 
test. Second, as was explained me recently, jtreg will execute simple 
"@run main TestObjectAlignment" by itself so you don't need to specify it.

Regards,
Vladimir

On 7/31/13 10:05 AM, Thomas Schatzl wrote:
> Hi,
>
> On Wed, 2013-07-31 at 08:54 -0700, Vladimir Kozlov wrote:
>> Thomas,
>>
>> There is LogMinObjAlignment. Why you did not use it?
>
>    did not recall it at that time it for some reason...
>
> New Webrevs:
>
> hs24: http://cr.openjdk.java.net/~tschatzl/8021823/webrev.1.hs24/
> hs25: http://cr.openjdk.java.net/~tschatzl/8021823/webrev.1.hs25/
>
> Fixed now, thanks,
>    Thomas
>
>>
>> Thanks,
>> Vladimir
>>
>> On 7/31/13 5:48 AM, Thomas Schatzl wrote:
>>> Hi all,
>>>
>>>     can I have reviews for the following fix? The change addresses wrong
>>> initialization of the mark bitmaps in G1 when ObjectAlignmentInBytes is
>>> larger than or equal to 32.
>>>
>>> The concurrent marking uses a shift value for mapping heap address to
>>> mark bitmap bits. This shift value has been calculated wrongly depending
>>> on ObjectAlignment (=ObjectAlignmentInBytes in HeapWords), so that only
>>> worked for ObjectAlignmentInBytes values <= 16 it has been correct.
>>>
>>> Instead of the correct value of "log2(ObjectAlignment)" the code used
>>> "ObjectAlignment-1".
>>>
>>> I.e. some table will show the problem (64 bit VM with sizeof(HeapWord)
>>> == 8):
>>>
>>> ObjectAlignment (OAInBytes)  Old Shift value  New Shift value
>>>          8 (64)                      7                3
>>>          4 (32)                      3                2
>>>          2 (16)                      1                1
>>>          1 (8)                       0                0
>>>
>>> I.e. when mapping HeapWords to bits, the code previously used a much too
>>> high value for ObjectAlignmentInBytes, resulting in truncating important
>>> bits from the heap address, resulting in wrong locations to look for and
>>> set bits.
>>>
>>> Coincidently the old calculation resulted in the same values for
>>> ObjectAlignmentInBytes<=16, so this issue went unnoticed.
>>>
>>> There is a new test that simply looks for these crashes for all
>>> supported object alignments by running a few concurrent markings.
>>>
>>> JBS:
>>> https://jbs.oracle.com/bugs/browse/JDK-8021823
>>>
>>> Bugs.sun:
>>> http://bugs.sun.com/view_bug.do?bug_id=8021823
>>>
>>> Webrev:
>>> hs24: http://cr.openjdk.java.net/~tschatzl/8021823/webrev.hs24/
>>> hs25: http://cr.openjdk.java.net/~tschatzl/8021823/webrev.hs25/
>>>
>>> Testing:
>>> manual testing of all failing tests with ObjectAlignmentInBytes from 8
>>> to 256, test case failing without the patch, succeeding with the patch,
>>> jprt.
>>>
>>> Testing:
>>>
>
>