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

[Thomas Schatzl]

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