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

Dmitry Dmitriev dmitry.dmitriev at oracle.com
Thu Jan 14 13:59:47 UTC 2016


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