RFR: 8165443: Free Collection Set serial phase takes very long on large heaps

[Thomas Schatzl]

Hi Stefan,

On Thu, 2019-12-05 at 19:27 +0100, Stefan Johansson wrote:
> Hi Thomas,
> 
> > 5 dec. 2019 kl. 14:00 skrev Thomas Schatzl <
> > thomas.schatzl at oracle.com>:
> > 
> > Hi Stefan,
> > 
> > 

[...]
> > - in G1FreeCollectionSetClosure::handle_evacuated_region, the code
> > could remove the call to clear_remset_locked() and set the
> > skip_remset parameter of the free_region call to false. Since this
> > seems to have been the only case of the skip_remset==true, the
> > parameter can be removed.
> > Also the "locked" parameter seems to be always true (now?), so this
> > one can be removed too. The removal of these parameters propagates
> > a bit, but that would reduce complexity of related methods quite a
> > bit :)
> 
> I removed the explicit call to clear_remset_locked() and changed the
> parameter to false. I think this is a good change for this patch.

:)

> 
> > 
> > Please add an is_at_safepoint() or so call there then.
> > 
> > I am open to moving this to a separate follow-up RFE, but it does
> > not seem that big of a change, and I think it makes the code easier
> > to understand.
> 
> I think the rest of this should be handled separately in follow up,
> filed: https://bugs.openjdk.java.net/browse/JDK-8235427

Good.

> 
> > 
> > - (this is imho) in the method FreeCSetStats::update_used_before()
> > and
> > FreeCSetStats::increment_regions_freed() are always called together
> > I recommend merging them. To me it distracts a bit from the flow of
> > the code with details (given that you already added methods to the
> > FreeCSetStats class). I would also merge the calls to
> > update_failure_used/update_failure_waste/update_used_after in
> > handle_failed_region.
> 
> I like this suggestion and I renamed the functions a bit too.
> 
> > 
> > Looks good otherwise.
> 
> Thanks, here are updated webrevs:
> Full: http://cr.openjdk.java.net/~sjohanss/8165443/02/
> Inc: http://cr.openjdk.java.net/~sjohanss/8165443/01-02/

  looks good.

Thomas