RFR: 8177544: Restructure G1 Full GC code

Thomas Schatzl thomas.schatzl at oracle.com
Mon Jul 10 13:01:20 UTC 2017


Hi Stefan,

On Wed, 2017-06-14 at 16:45 +0200, Stefan Johansson wrote:
> Thanks Thomas for reviewing,
> 
> On 2017-06-13 11:36, Thomas Schatzl wrote:
> > 
> > Hi,
> > 
> >    thanks for your hard work on the parallel full gc that starts
> > with this refactoring :)
> :)
> > 
> > On Thu, 2017-06-08 at 14:35 +0200, Stefan Johansson wrote:
> > > 
> > > Hi,
> > > 
> > > Please review this enhancement:
> > > https://bugs.openjdk.java.net/browse/JDK-8177544
> > > 
> > > Webrev:
> > > http://cr.openjdk.java.net/~sjohanss/8177544/hotspot.00/
> > > 
> > [... lots of suggested changes from me...]

Thanks for these changes.

> > 
> > Actually, if it were for me, I would put the whole full gc setup
> > and
> > teardown into a separate class/file.
> > 
> > Have public gc_prologue()/collect()/gc_epilogue() methods where
> > gc_prologue() is the first part of do_full_collection_inner() until
> > application of the G1SerialCollector, collect() the instantiation
> > and application of G1SerialCollector, and gc_epilogue() the
> > remainder.
> > 
> > E.g. in G1CollectedHeap we only have the calls to these three
> > methods (there is no need to have all three).
> > 
> > At least I think it would help a lot if all that full gc stuff
> > would be separate physically from do-all-G1CollectedHeap.
> > With the G1FullGCScope there is almost no reference to
> > G1CollectedHeap afaics.
> > 
> > (There is _allocator->init_mutator_alloc_region() call)
> I see your point and I think it would be good. But as we discussed
> over chat, might be something to look at once everything else in this
> area is done. Will create a RFE for this.

Yes, that's fine.

> 
> > 
> >    - g1CollectedHeap.hpp: please try to sort the definitions of the
> > new methods in order of calling them.
> Done.
> 
> Here are updated webrevs:
> Full: http://cr.openjdk.java.net/~sjohanss/8177544/hotspot.01/
> Inc: http://cr.openjdk.java.net/~sjohanss/8177544/hotspot.00-01/
> 

Looks good to me. Sorry for the late reply.

Thanks,
  Thomas




More information about the hotspot-gc-dev mailing list