RFR: 8144578: TestOptionsWithRanges test only ever uses the default collector

Dmitry Dmitriev dmitry.dmitriev at oracle.com
Thu Jan 14 17:19:55 UTC 2016


Sangheon, thank you for the review!

Dmitry

On 14.01.2016 20:18, sangheon wrote:
> Hi Dmitry,
>
> Looks good to me.
>
> Thanks,
> Sangheon
>
>
> On 01/14/2016 05:59 AM, Dmitry Dmitriev wrote:
>> Hi Sangheon,
>>
>> Thank you for the review! Updated webrev: 
>> http://cr.openjdk.java.net/~ddmitriev/8144578/webrev.01/ 
>> <http://cr.openjdk.java.net/%7Eddmitriev/8144578/webrev.01/>
>> Comments inline.
>>
>> On 13.01.2016 21:50, sangheon wrote:
>>> Hi Dmitry,
>>>
>>> Thank you for fixing this.
>>> Overall seems good.
>>>
>>> --------------------------------------------------------------------
>>> test/runtime/CommandLine/OptionsValidation/TestOptionsWithRanges.java
>>>
>>> 87 /*
>>> 88 * JDK-8144578
>>> 89 * Temporarily remove testing of max range for ParGCArrayScanChunk 
>>> because
>>> 90 * JVM can hang when ParGCArrayScanChunk=4294967296 and ParallelGC 
>>> is used
>>> 91 */
>>> 92 excludeTestMaxRange("ParGCArrayScanChunk");
>>>
>>> issue number should be 8145204.
>> Fixed.
>>>
>>> --------------------------------------------------------------------
>>> test/runtime/CommandLine/OptionsValidation/common/optionsvalidation/JVMOptionsUtils.java 
>>>
>>> line 181
>>>
>>> - if (name.startsWith("G1")) {
>>> - option.addPrepend("-XX:+UseG1GC");
>>> - }
>>> -
>>> - if (name.startsWith("CMS")) {
>>> - option.addPrepend("-XX:+UseConcMarkSweepGC");
>>> - }
>>> -
>>>
>>> Is this change really needed for dedicated gc flags(starting with 
>>> "G1" or "CMS")?
>>> I thought this CR is targeted for non-dedicated gc flags such as 
>>> TLABWasteIncrement.
>> I return deleted lines.
>>
>> Thanks,
>> Dmitry
>>>
>>> And if you still think that above lines should be removed, please 
>>> remove line 224 as well.
>>>
>>>  224             case "NewSizeThreadIncrease":
>>>  225                 option.addPrepend("-XX:+UseSerialGC");
>>>
>>>
>>> Thanks,
>>> Sangheon
>>>
>>>
>>> On 01/13/2016 09:11 AM, Dmitry Dmitriev wrote:
>>>> Hello,
>>>>
>>>> Please review small enhancement to the command line option 
>>>> validation test framework which allow to run test with different GCs.
>>>> Few comments:
>>>> 1) Code which executed for testing was moved from 
>>>> JVMOptionsUtils.java to separate class(JVMStartup.java) to avoid 
>>>> overhead at java start-up for determining vm and gc type.
>>>> 2) runJavaWithParam method in JVMOption.java was refactored to 
>>>> avoid code duplication.
>>>>
>>>> JBS: https://bugs.openjdk.java.net/browse/JDK-8144578
>>>> webrev.00: http://cr.openjdk.java.net/~ddmitriev/8144578/webrev.00/ 
>>>> <http://cr.openjdk.java.net/%7Eddmitriev/8144578/webrev.00/>
>>>> Testing: tested on all platforms with different gc by RBT, failed 
>>>> flags were temporary removed from testing in 
>>>> TestOptionsWithRanges.java
>>>>
>>>> Thanks,
>>>> Dmitry
>>>
>>
>



More information about the hotspot-dev mailing list