RFR (M): 8048084: Need to abort concurrent next bitmap clear when marking is aborted during concurrent next bitmap clearing

[Thomas Schatzl]

Hi all,

On Mon, 2014-07-07 at 16:03 +0200, Thomas Schatzl wrote:
> Hi all,
> 
> On Mon, 2014-07-07 at 15:53 +0200, Thomas Schatzl wrote:
> > Hi all,
> > 
> >   can I have reviews for the following change that is a pre-requisite
> > for JDK-8038423: G1: Decommit memory within the heap?
> > 
> > The change addresses a problem when concurrent next bitmap clearing is
> > interrupted by Full GC, we need to abort concurrent next bitmap clear.
> > 
> > On the one hand, the concurrent marking thread might still be working on
> > a just uncommitted region as next bitmap mark is chunked (so with
> > JDK-8038423, which leads to instant crashes), and on the other hand this
> > is superfluous work because the Full GC already did that during mark
> > abort handling.
> > 
> > There are other workarounds for this issue that could somehow consider
> > whether the given region they are working on has been uncommitted in the
> > meantime.
> > However I think this adds unnecessary logic into the code. Full GC
> > already clears the next mark bitmap completely anyway, so why continue
> > at this point.
> > 
> > Webrev:
> > http://cr.openjdk.java.net/~tschatzl/8048084/webrev/
> > 
> > CR:
> > https://bugs.openjdk.java.net/browse/JDK-8048084
> > 
> > Testing:
> > JPRT, lots of aurora ad-hoc testing with memory uncommit changes
> 
> Please hold off reviewing this change for now as testing showed a
> problem right now.

  I am going to retract this CR. The problem is that this functionality
cannot be split out from other changes needed for JDK-8038423 unlike I
tried.

The problem is that the change makes the clearAll() method clear only
available regions (committed) instead of the entire mark bitmap.

Which is somewhat desirable because it avoids unnecessary work.

However this misses clearing of the mark bitmap of regions committed at
the end of full gc, causing random marks to appear. The current code
does not automatically clear this data on commit.

So to make this change work, we would need additional code that does
that, that would be removed soon anyway.

Sorry again,
  Thomas