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

[Vladimir Kozlov]

I am not familiar with createJavaProcessBuilder API. Do we need to 
specify -d64 flag for created process to run 64-bit VM? Or it will use 
the same VM as main process?

Thanks,
Vladimir

On 7/31/13 12:02 PM, Thomas Schatzl wrote:
> Hi,
>
> On Wed, 2013-07-31 at 10:29 -0700, Vladimir Kozlov wrote:
>> 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.
>>
>
> Changed; I do not like this automatism as I've been tripped over it
> least once (e.g. as soon as you add a masked @run statement like @build,
> the test is not automatically run any more), so I left the @run
> statement.
>
> You are correct though about both issues, and I do not really have a
> strong opinion about this. I've updated the webrevs at
>
> hs24: http://cr.openjdk.java.net/~tschatzl/8021823/webrev.2.hs24/
> hs25: http://cr.openjdk.java.net/~tschatzl/8021823/webrev.2.hs25/
>
> Thanks,
> Thomas
>
>> 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:
>>>>>
>>>
>>>
>
>