RFR (L): 8217330: Split CollectionSetChooser into collection set candidate container and the chooser algorithm

Thomas Schatzl thomas.schatzl at oracle.com
Fri Feb 8 10:01:52 UTC 2019


Hi Kim,

On Wed, 2019-02-06 at 19:26 -0500, Kim Barrett wrote:
> > On Jan 25, 2019, at 9:11 AM, Thomas Schatzl <
> > thomas.schatzl at oracle.com> wrote:
> > All fixed in
> > http://cr.openjdk.java.net/~tschatzl/8217330/webrev.0_to_1 (diff)
> > http://cr.openjdk.java.net/~tschatzl/8217330/webrev.1 (full)
> > 
> > Thanks,
> >  Thomas
> 
> Looks good.
> 
> Just a couple minor nits, for which I don't need another webrev.
> 
> -------------------------------------------------------------------
> -----------
> src/hotspot/share/gc/g1/g1CollectionSet.hpp
>   28 #include "gc/g1/g1CollectionSetCandidates.hpp"
> ...
>  132   void clear_candidates() {
>  133     delete _candidates;
>  134     _candidates = NULL;
>  135   }
> 
> I think that if clear_candidates were not inlined here, the #include
> could be replaced with a forward declaration.
> 
> Of course, then some using files would need the #include, but they
> probably should anyway, to avoid implicit dependencies.
> 
> -------------------------------------------------------------------
> -----------
> src/hotspot/share/gc/g1/collectionSetChooser.hpp
>   37 class CollectionSetChooser : public AllStatic {
> ...
>   40   CollectionSetChooser();
> 
> Derived from AllStatic, so should no longer have a constructor.
> 

  thanks for your review. Going to fix all this before pushing.

Thomas




More information about the hotspot-gc-dev mailing list