RFR (L): 8219100: Improve do_collection_pause_at_safepoint

[Thomas Schatzl]

Hi Sangheon,

On Fri, 2019-03-22 at 13:06 -0700, sangheon.kim at oracle.com wrote:
> Hi Thomas,
> 
> On 3/4/19 3:16 AM, Thomas Schatzl wrote:
> > Hi all,
> > 
> >    can I have reviews for this (relatively big) cleanup change that
> > reorganizes the G1CollectedHeap::do_collection_pause_at_safepoint()
> > method to be (imho) much more readable?
> > 
> > - refactored out some methods, mainly verification but also other
> > helpers
> > - grouped together "same" category calls so that e.g. collection
> > set related methods are not more or less randomly interspersed with
> > unrelated code
> > - moved code into the right "levels" of indentation
> > - moved code related to evacuation that kind of bleeded out of
> > pre/post_evacuate_collection_set() into it, also trying to improve
> > grouping and ordering
> > 
> > CR:
> > https://bugs.openjdk.java.net/browse/JDK-8219100
> > Webrev:
> > http://cr.openjdk.java.net/~tschatzl/8219100/webrev
> 
> Nice cleanup!
> 
> One minor nit:  the changed method name of 
> G1ConcurrentMark::verify_no_collection_set_oops_in_stacks() seems 
> questionable. Removing '_in_stacks' or updating its comment(as it
> says stacks and  fingers) looks better but it's your call.
> I'm okay as is too.

I pushed with reverting that name change. We can discuss this in a
separately.

Thanks for your review.

Thanks,
  Thomas