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

[Tony Printezis]

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.

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
>>>
>>>
>

-- 
Tony Printezis | Staff Software Engineer | Twitter

@TonyPrintezis
tprintezis at twitter.com