RFR (XXS): 8048085: Aborting marking just before remark results in useless additional clearing of the next mark bitmap

[Thomas Schatzl]

Hi Bengt,

  thanks for the review.

On Mon, 2014-07-07 at 15:44 +0200, Bengt Rutisson wrote:
> Hi Thomas,
> 
> On 2014-07-07 14:46, Thomas Schatzl wrote:
> > Hi all,
> >
> >    can I have reviews for the following minor performance fix noticed
> > during work on JDK-8038423 (G1: Decommit memory within the heap)?
> >
> > The situation is that when a full GC aborts concurrent marking, the
> > concurrent mark thread needlessly clears the next mark bitmap again
> > (concurrently this time) in ConcurrentMark::abort().
> >
> > The fix is to skip this phase in this case.
> >
> > An alternative, always keeping this phase but not doing this bitmap
> > clear at Full GC abort (JDK-7098512) would be possible. However doing
> > that would make JDK-8048084 (sending out reviews soon) much harder as we
> > would need to keep track of regions that have become unavailable.
> >
> > This change also does not change anything about duration of Full GC. So
> > I would prefer to postpone JDK-7098512.
> >
> > Webrev:
> > http://cr.openjdk.java.net/~tschatzl/8048085/webrev/
> 
> Looks good.
> 
> A couple of minor comments:
> 
> The comment in ConcurrentMark::abort() for the bitmap clearing was 
> always misleading (as stated in JDK-7098512). Would maybe be good to 
> update it now to explain why it is really needed.
> 
> What do you think about asserting that the bitmap is clear in 
> ConcurrentMarkThread::run() if cm()->has_aborted() ?

  good thoughts.

I updated the change and JDK-7098512, new webrev at
http://cr.openjdk.java.net/~tschatzl/8048085/webrev.1/

Diff:
http://cr.openjdk.java.net/~tschatzl/8048085/webrev.0_to_1/

Testing:
jprt

Thanks,
  Thomas