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

[Jon Masamitsu]

On 8/29/13 5:41 AM, Tony Printezis wrote:
> Hi Thomas,
>
> Thanks for pointing me to the previous e-mail (I looked for that info 
> on the CR but I could only see the crash reports and not any info on 
> the failure).
>
> Re: test: I totally understand. I was just pointing out that the test, 
> as is, will probably not be very helpful to any other GC than G1.
>
> BTW, do you guys still use the GC test suite? It might be worth 
> extending it to do a few runs with different object alignments to 
> easily stress that code a bit more.

Yes, some of us still use the GC test suite.  It no longer has
a champion though.

Jon

>
> Tony
>
> On 8/28/13 2:14 PM, Thomas Schatzl wrote:
>> Hi,
>>
>> On Wed, 2013-08-28 at 12:29 -0400, Tony Printezis wrote:
>>> Thomas,
>>>
>>> The fix in concurrentMark.cpp looks OK (how did it work before?!?!?!).
>> Nobody ever used G1 with ObjectAlignmentInBytes >= 32? :)
>>
>> For OAIB of 8/16 the values before/after the change are the same due to
>> coincidence, see the original RFR at
>> http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/2013-July/007870.html 
>> .
>>
>>> Regarding the test: If I understand the test correctly, the only reason
>>> why it works is because a conc marking cycle in G1 starts with a young
>>> GC (the assert seems to happen during an evac pause, right?). So, note
>>> that this test will probably behave different with CMS (if that 
>>> matters).
>> If the alignment is not correct, at the latest during the cleanup phase
>> or memory reclamation the vm will crash/assert quite quickly :)
>>
>> Consider that memory reclamation will use the mark bits (which are
>> wrong) to find the live object start and sizes...
>>
>> There has been this discussion whether it makes sense to improve the
>> test, but to make it 100% bullet-proof it would require some additional
>> code to control gcs better imo. This seems out of scope here though.
>>
>> Thanks for looking at this,
>>    Thomas
>>
>>> Tony
>>>
>>> On 8/28/13 9:32 AM, Thomas Schatzl wrote:
>>>> Hi all,
>>>>
>>>> On Thu, 2013-08-01 at 07:49 +0200, Bengt Rutisson wrote:
>>>>> Hi Thomas,
>>>>>
>>>>> The code change looks good.
>>>>>
>>>>> One question about the test. I know that the current failure was with
>>>>> G1, but all of our GC should work with all the supported
>>>>> ObjectAlignments, right? Why do we only want to test this with G1?
>>>>> Maybe we could just remove " -XX:+UseG1GC" from the command line and
>>>>> get testing for all GCs? In that case I guess you probably don't need
>>>>> to use the ProcessTools either. You could have serveral @run commands
>>>>> instead.
>>>>     I still need another review on this issue, anyone?
>>>>
>>>> I only updated the test according to Bengt's suggestion, i.e. allow
>>>> running all collectors through the tests and using several @run
>>>> statements.
>>>>
>>>> bugs.sun.com:
>>>> http://bugs.sun.com/view_bug.do?bug_id=8021823
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~tschatzl/8021823/webrev.4/
>>>>
>>>> Testing:
>>>> jprt, jtreg test
>>>>
>>>> Thanks,
>>>> Thomas
>>>>
>>>>
>>
>