RFR (smallish?) 8061611: Remove deprecated command line flags
bengt.rutisson at oracle.com
Mon Dec 15 12:51:23 UTC 2014
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
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.
> - 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
>> - Derek
>> *Webrev*: http://cr.openjdk.java.net/~drwhite/8061611/webrev.00/
>> *Bug*: https://bugs.openjdk.java.net/browse/JDK-8061611
>> Saw 1-2 intermittent failures that went away on retesting - hangs
>> and timeouts.
>> 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
>> Checks that jmap -heap contains the flag
>> interrupted! (timed out?)
>> * closed/runtime/AppCDS/SharedArchiveConsistency.java
>> Plus a bunch of tests that are labelled "ignored".
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the hotspot-gc-dev