RFR (smallish?) 8061611: Remove deprecated command line flags

Bengt Rutisson bengt.rutisson at oracle.com
Mon Dec 15 12:51:23 UTC 2014



Hi Derek,

On 2014-12-12 15:35, Derek White wrote:
> This is a request for final re-review.
>
> Updated for comments, and added regression test for arguments. This 
> test is a bit overkill for this bug, but it's extensible for other 
> deprecated and aliased options.
>
> Re-merged with tree.
>
> Webrev: http://cr.openjdk.java.net/~drwhite/8061611/webrev.01/

The code changes look good to me.

A few comments about the test.

I don't think it is worth testing that the removed flags produce error 
messages. Starting a VM takes some time and doing it 13 times just to 
check that no one by mistake added the flags back seems like a waste of 
resources to me. The risk that the flags are suddenly added back is 
almost zero, right?

I am also not convinced that a general and extensible test is the way to 
go here. I think I would prefer a more specific test for the flags that 
were deprecated now. For tests I think it is more important that the 
tests are easily readable and understandable than that code duplication 
is avoided. Thus, I strongly prefer small but specific tests that 
clearly communicates what went wrong when they fail and are easy to 
understand how they failed. So, in this case maybe we should even have 
two tests? TestDeprecatedMarkStackFlags and 
TestDeprecatedConcGCThreadsFlags.

Instead of parsing the PrintFlagsFinal output you could use 
ManagementFactoryHelper.getDiagnosticMXBean().getVMOption() to get the 
option values and check if they are set correctly. I think that will 
reduce the number of lines in the test further.

Thanks,
Bengt

>
> Thanks!
>
>  - Derek
>
> On 11/18/14 11:40 AM, Derek White wrote:
>> Hi Team,
>>
>> First review request. Please let me know if I've forgotten something 
>> or have gone completely off the rails.
>>
>> The main point of this bug is to remove deprecated -XX options which 
>> are alias for other options.
>>
>> The only complicated part is that one case, 
>> /CMSParPromoteBlocksToClaim/ was not a true alias for /OldPLABSize/ 
>> but a parallel option with different defaults that were synchronized 
>> in ergo processing. This fix removes the /CMSParPromoteBlocksToClaim 
>> /variable but preserves using different defaults in the CMS case.
>>
>> Also in this fix I added warning messages to several remaining 
>> undocumented command line options aliases. This should ease removal 
>> of these options in the future
>> CMSMarkStackSize ==> MarkStackSize        (MarkStackSize not documented either, but came in jdk6)
>> G1MarkStackSize  ==> MarkStackSize
>> CMSMarkStackSizeMax ==> MarkStackSizeMax  (MarkStackSizeMax not documented either)
>> ParallelMarkingThreads =>  ConcGCThreads  (ConcGCThreads came in jdk6)
>> ParallelCMSThreads     ==> ConcGCThread
>>
>> Thanks,
>>  - Derek
>>
>> *Webrev*: http://cr.openjdk.java.net/~drwhite/8061611/webrev.00/
>>
>> *Bug*: https://bugs.openjdk.java.net/browse/JDK-8061611
>>
>> *Testing*:
>> jprt:
>>     Saw 1-2 intermittent failures that went away on retesting - hangs 
>> and timeouts.
>>
>> refworkload:
>>     no differences
>>
>> jtreg: Saw a few unexplained results. Not sure if typical or not:
>>
>>
>>             Execution failed: `main' threw exception:
>>             java.lang.Exception: jmap -heap exited with error code: 1
>>
>>       * gc/metaspace/CompressedClassSpaceSizeInJmapHeap.java
>>         <http://oklahoma.us.oracle.com/net/bussund0416//export/users/drwhite/hs-gc-mine/hotspot/test/JTwork/gc/metaspace/CompressedClassSpaceSizeInJmapHeap.jtr>:
>>         Checks that jmap -heap contains the flag
>>         CompressedClassSpaceSize
>>
>>
>>             Program
>>             `/export/users/drwhite/test-builds/j2sdk-image.11.17.2014/bin/java'
>>             interrupted! (timed out?)
>>
>>       * closed/runtime/AppCDS/SharedArchiveConsistency.java
>>         <http://oklahoma.us.oracle.com/net/bussund0416//export/users/drwhite/hs-gc-mine/hotspot/test/JTwork/closed/runtime/AppCDS/SharedArchiveConsistency.jtr>:
>>         SharedArchiveConsistency
>>
>>     Plus a bunch of tests that are labelled "ignored".
>>
>>
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/hotspot-gc-dev/attachments/20141215/97a321f6/attachment.html>


More information about the hotspot-gc-dev mailing list