RFR (L): 8064702: Remove the CMS foreground collector

[Kim Barrett]

On Nov 12, 2014, at 11:49 AM, Bengt Rutisson <bengt.rutisson at oracle.com> wrote:
> 
> Here's an updated webrev:
> http://cr.openjdk.java.net/~brutisso/8064702/webrev.01/
> 
> And just the diff compared to the last webrev:
> http://cr.openjdk.java.net/~brutisso/8064702/webrev.00-01.diff/

------------------------------------------------------------------------------

src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.hpp

1150   // Does a "full" (forced) collection invoked on this generation collect
1151   // all younger generations as well? Note that the second conjunct is a
1152   // hack to allow the collection of the younger gen first if the flag is
1153   // set.
1154   virtual bool full_collects_younger_generations() const {
1155     return UseCMSCompactAtFullCollection && !ScavengeBeforeFullGC;
1156   }

=>

1123   virtual bool full_collects_younger_generations() const {
1124     return !ScavengeBeforeFullGC;
1125   }

The complete removal of the comment is eliminating the information in
the final sentence, regarding the second conjunct (e.g. the code that
is being retained) being a hack.  Presumably someone (long ago and
lost in the mists of pre-mercurial source control) thought that test
was a bit of a kludge and might deserve a rethink.

Note that there is an identical comment on
TenuredGeneration::full_collects_younger_generations(), and the updated
implementation here is now identical to the implementation for
TenuredGeneration.  Keeping the associated comments consistent for
these identical functions would be nice.

------------------------------------------------------------------------------

src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp

1248   // For debugging purposes, change the type of collection.
1249   // If the rotation is not on the concurrent collection
1250   // type, don't start a concurrent collection.
1251   NOT_PRODUCT(
1252     if (RotateCMSCollectionTypes) {
1253       return _cmsGen->debug_concurrent_cycle();
1254     }
1255   )

Code was changed, but the comment looks like it needs to be updated
too, since there is no longer a "concurrent collection type" in the
previous sense.

------------------------------------------------------------------------------

src/share/vm/gc_implementation/concurrentMarkSweep/concurrentMarkSweepGeneration.cpp 

1254     if (RotateCMSCollectionTypes &&
1255         (_cmsGen->debug_collection_type() !=
1256           ConcurrentMarkSweepGeneration::Concurrent_collection_type)) {
1257       assert(_cmsGen->debug_collection_type() !=
1258         ConcurrentMarkSweepGeneration::Unknown_collection_type,
1259         "Bad cms collection type");
1260       return false;
1261     }

=>

1252     if (RotateCMSCollectionTypes) {
1253       return _cmsGen->debug_concurrent_cycle();
1254     }

I spent a large amount of time studying this change, because there are
behavioral changes here that looked suspicious.  I think I've decided
it's ok, but RotateCMSCollectionTypes is now badly named, and was
never well described and had (and still has) unclear semantics.  I
think I've puzzled out more or less what it means, but that required
reading a lot of code, and I'm still not confident that I know what
would happen if I used it, or why I might want to.

------------------------------------------------------------------------------