RFR (L): 8065972: Remove support for ParNew+SerialOld and DefNew+CMS

[Bengt Rutisson]

Hi Mikael,

Thanks for looking at this!

On 2014-11-26 14:07, Mikael Gerdin wrote:
> Hi Bengt,
>
> On 2014-11-26 13:09, Bengt Rutisson wrote:
>>
>> Hi everyone,
>>
>> Can I have a couple of reviews for this change to remove the support for
>> ParNew+SerialOld and DefNew+CMS?
>>
>> This work is part of  JEP 214 (http://openjdk.java.net/jeps/214).
>>
>> The main change is in the hotspot repo:
>> http://cr.openjdk.java.net/~brutisso/8065972/hotspot-webrev.00/
>
> Not a complete review yet, but:
>
> * Should we make CMSCollector::_young_gen a ParNewGeneration* now that 
> the type is statically known?
> ** In that case _young_gen->as_DefNewGeneration() calls can be removed 
> as well.

Good point. I prefer to do this a separate change. I filed this bug to 
track that work:

Change CMSCollector::_young_gen to be a ParNewGeneration*
https://bugs.openjdk.java.net/browse/JDK-8065992

>
> * I think you removed the only usage of ParGCAllocBufferWithBOT (don't 
> forget to remove it from the mapfiles as well if you do remove it)

Good catch! Will remove it.

>
> * You can also remove the whole machinery around 
> set_avoid_promotion_undo in ParNew, since CMS does not allow promotion 
> undo. This should rid us of some code duplication since 
> ParNewGeneration::copy_to_survivor_space_with_undo can go away and 
> ParNewGeneration::copy_to_survivor_space__avoiding_promotion_undo can 
> be folded into ParNewGeneration::copy_to_survivor_space.

Great. I'll remove that too.

>
> * In fact, OneContigSpaceCardGeneration can be merged with 
> TenuredGeneration, but that may be more appropriate in a separate change.

Good point. Filed a bug for that too:

Merge OneContigSpaceCardGeneration with TenuredGeneration
https://bugs.openjdk.java.net/browse/JDK-8065993

Will send out an updated webrev soon.

Thanks,
Bengt

>
> /Mikael
>
>>
>> I also removed the ParNew runs from the jprt configuration:
>> http://cr.openjdk.java.net/~brutisso/8065972/root-webrev.00/
>>
>> JBS bug:
>> https://bugs.openjdk.java.net/browse/JDK-8065972
>>
>> Some notes about the change:
>>
>> - UseParNewGC is now a redundant flag. It can only be used with CMS, so
>> it will always have the same value as UseConcMarkSweepGC. Since
>> UseParNewGC was not one of the flags that were deprecated in JDK 8, we
>> decided to leave the flag in for JDK 9 but print a warning message that
>> it is deprecated. We plan on removing UseParNewGC in JDK 10.
>>
>> Here's how the VM will behave with different values of UseParNewGC after
>> my change:
>>
>> $ java -XX:+UseParNewGC -version
>> It is not possible to combine the ParNew young collector with the Serial
>> old collector.
>> Error: Could not create the Java Virtual Machine.
>> Error: A fatal exception has occurred. Program will exit.
>>
>> $ java -XX:-UseParNewGC -version
>> Java HotSpot(TM) 64-Bit Server VM warning: The UseParNewGC flag is
>> deprecated and will likely be removed in a future release
>> java version "1.9.0-internal-debug"
>> Java(TM) SE Runtime Environment (build
>> 1.9.0-internal-debug-brutisso_2014_11_05_09_40-b00)
>> Java HotSpot(TM) 64-Bit Server VM (build
>> 1.9.0-internal-debug-brutisso_2014_11_05_09_40-b00, mixed mode)
>>
>> $ java -XX:+UseParNewGC -XX:+UseConcMarkSweepGC -version
>> Java HotSpot(TM) 64-Bit Server VM warning: The UseParNewGC flag is
>> deprecated and will likely be removed in a future release
>> java version "1.9.0-internal-debug"
>> Java(TM) SE Runtime Environment (build
>> 1.9.0-internal-debug-brutisso_2014_11_05_09_40-b00)
>> Java HotSpot(TM) 64-Bit Server VM (build
>> 1.9.0-internal-debug-brutisso_2014_11_05_09_40-b00, mixed mode)
>>
>> $ java -XX:-UseParNewGC -XX:+UseConcMarkSweepGC -version
>> It is not possible to combine the DefNew young collector with the CMS
>> collector.
>> Error: Could not create the Java Virtual Machine.
>> Error: A fatal exception has occurred. Program will exit.
>>
>> - The NEW_C_HEAP_ARRAY and "new" calls in initialize_generations()
>> methods will exit if they fail. No need to check for null after those
>> calls. This simplified the two implementations significantly.
>>
>> - At a few places I've changed from "DefNewGeneration* dng =
>> (DefNewGeneration*)_young_gen;" to "DefNewGeneration* dng =
>> _young_gen->as_DefNewGeneration();". The as_DefNewGeneration() method
>> does the asserts that were often placed before the original casting 
>> code.
>>
>> - In concurrentMarkSweepGeneration.cpp I left the casting to
>> DefNewGeneration even though we could now use the more specific
>> ParNewGeneration. I prefer to do such an update later (if at all) since
>> that might come with its own complications regarding virtual calls etc.
>>
>> - The two methods must_be_youngest() and must_be_oldest() were unused.
>> Removing them was not strictly needed for this change, but seemed like a
>> nice thing to do anyway and I don't think it clutters the webrev much.
>>
>> - UseParNewGC is now always true when UseConcMarkSweepGC is true. And it
>> is always false when UseConcMarkSweepGC is false. If statements that
>> used to test against both flags have been simplified to only test
>> against UseConcMarkSweepGC. This means that UseParNewGC is now only used
>> in arguments.cpp
>>
>> - Similarly JTreg tests that used UseParNewGC have been changed to use
>> UseConcMarkSweepGC instead. This should make it easy to remove in JDK 
>> 10.
>>
>> - The change to make/jprt.properties only affects the internal testing
>> system we have. It will no longer test the ParNew+SerialOld combination
>> (which is no longer supported). Note that DefNew+CMS was not tested at
>> all by JPRT.
>>
>> Thanks,
>> Bengt