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

[Bengt Rutisson]

Hi Mikael,

Thanks for the review!

StefanK found some more code to delete. :)

Here's an updated webrev:
http://cr.openjdk.java.net/~brutisso/8065972/hotspot-webrev.02/

And here's the diff compared to the last one:
cr.openjdk.java.net/~brutisso/8065972/hotspot-webrev.01-02.diff/

Thanks,
Bengt


On 2014-11-27 14:56, Mikael Gerdin wrote:
> Hi Bengt,
>
> On 2014-11-26 14:50, Bengt Rutisson wrote:
>>
>> Hi Stefan,
>>
>> Thanks for looking at this!
>>
>> On 2014-11-26 14:42, Stefan Karlsson wrote:
>>> Hi Bengt,
>>>
>>> The changes looks good. There are a couple of follow-up cleanups that
>>> can be done as separate changes, but I don't think they are blocking
>>> this patch.
>>
>> Agreed. I filed two cleanup bugs based on Mikael's review.
>>
>>>
>>> http://cr.openjdk.java.net/~brutisso/8065972/hotspot-webrev.00/test/gc/startup_warnings/TestParNewSerialOld.java.frames.html 
>>>
>>>
>>>
>>> I think the summary should be changed to:
>>>
>>>   28 * @summary Test that the unsupported ParNew+SerialOld combination
>>> does not start
>>
>> Good catch.
>>
>> Here's an updated webrev that fixes the summary and also removes the
>> code that Mikael found:
>>
>> http://cr.openjdk.java.net/~brutisso/8065972/hotspot-webrev.01/
>
> Excellent! 600+ deleted lines!
>
> Looks good.
>
> /Mikael
>
>>
>> Only the changes compared to the last webrev:
>>
>> http://cr.openjdk.java.net/~brutisso/8065972/hotspot-webrev.00-01.diff/
>>
>> Thanks,
>> Bengt
>>
>>>
>>>
>>> StefanK
>>>
>>> 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/
>>>>
>>>> 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
>>>
>>