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

Bengt Rutisson bengt.rutisson at oracle.com
Mon Nov 17 09:38:42 UTC 2014


Hi all,

Ramki reviewed the change and noticed that the removed method 
decide_foreground_collection_type() method had a side effect that I had 
missed. It would set the _incremental_collection_failed state that is 
used for the sizing heuristics. Thanks for looking at this, Ramki!

I copied that code in to the place where 
decide_foreground_collection_type() used to be called.

Here's an updated webrev:
http://cr.openjdk.java.net/~brutisso/8064702/webrev.03/

Here's the diff compared to the last webrev:
http://cr.openjdk.java.net/~brutisso/8064702/webrev.02-03.diff/

Thanks,
Bengt

On 2014-11-14 08:08, Bengt Rutisson wrote:
>
> Hi Kim,
>
> On 2014-11-13 22:28, Kim Barrett wrote:
>> On Nov 13, 2014, at 11:11 AM, Kim Barrett <kim.barrett at oracle.com> 
>> wrote:
>>> On Nov 13, 2014, at 2:23 AM, Bengt Rutisson 
>>> <bengt.rutisson at oracle.com> wrote:
>>>>> ------------------------------------------------------------------------------ 
>>>>>
>>>>>
>>>>> 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.
>>>> Right. I agree that it is unclear if this code is needed. I have 
>>>> never been running with RotateCMSCollectionTypes but I did not want 
>>>> to remove it as part of this change.
>>>>
>>>> […]
>>>>
>>>> I do agree that the naming is bad. But if we decide to remove this 
>>>> functionality there is not much need to spend time on the naming 
>>>> discussion.
>>> My inclination would be to remove it. […]
>> Just to be clear, I’m ok with deferring removal to a later task.  
>> Please file a bug report…
>
> Great, thanks!
>
> I filed a bug (JDK-8064865) and sent out a review request.
>
> Thanks again for looking at this and thanks for the review!
> Bengt
>
>
>>
>




More information about the hotspot-gc-dev mailing list