RFR (M): 8004687: G1: Parallelize object self-forwarding and scanning during an evacuation failure

[Thomas Schatzl]

Hi Mikael,

On Tue, 2015-07-21 at 17:07 +0200, Mikael Gerdin wrote:
> Hi Thomas,
> 
> On 2015-07-16 11:40, Thomas Schatzl wrote:
> > (resend with CR number in the subject)
> > Hi all,
> >
> >    can I have reviews for the following change from a student (Walter
> > Gugenberger; he has signed the OCA) who worked on this issue last
> > semester?
> >
> > The change parallelizes object self-forwarding and scanning during an
> > evacuation failure by:
> > - reuse the entire existing evacuation copy closure and work queue
> > mechanism in case of evacuation failure to store the work items.
> > This allows evacuation failure to automatically benefit from work
> > stealing etc.
> > - remove the dedicate evacuation failure handling closure because it is
> > not necessary any more.
> >    - there is one subtle change in behavior: the old evacuation failure
> > always had a NULL ReferenceProcessor, while now the standard reference
> > processor for references is applied. Since the NULL ReferenceProcessor
> > kept all references alive, now potentially less references will be kept
> > alive after an evacuation failure. I do not see a problem here.
> 
> Right. Another difference is that the evac failure closure 
> unconditionally called update_rs for each oop while the scanner closure 
> only calls update_rs for non-cset oops and defers the cset updating to 
> when a reference is popped from the work queue.

I did not think they were worth mentioning, but you are right.

> > - implementing per-thread preserved mark buffers
> >
> > As for actual performance improvements, there is none: the main problem
> > is that in case of evacuation failure the code will simply serialize on
> > the FreeList_lock when trying to get a new allocation region instead of
> > the EvacFailureStack_lock.
> >
> > Evacuation failure and the early-exit for evac failure as suggested in
> > the CR will only be really effective when the FreeList_lock can be
> > avoided. This will be added in a follow-up change.
> >
> > CR:
> > https://bugs.openjdk.java.net/browse/JDK-8004687
> >
> > Webrev:
> > http://cr.openjdk.java.net/~tschatzl/8004687/webrev/
> 
> Overall, a nice cleanup regardless of its lack of a general performance 
> improvement.

Just waiting that this change gets reviewed, and then we are going look
at the next one :)

> I think that this change removes the last use of G1BarrierEvac, so the 
> code for that in G1ParCopyClosure::do_oop_work can probably be removed.

I prepared a webrev that removes G1BarrierEvac related code too at

 http://cr.openjdk.java.net/~tschatzl/8004687/webrev.1 (full)
 http://cr.openjdk.java.net/~tschatzl/8004687/webrev.0_to_1 (diff)

More deletions basically :)

Thanks,
  Thomas