RFR: 8066821(S) Enhance command line processing to manage deprecating and obsoleting -XX command line arguments

[David Holmes]

Hi Derek,

Sorry but I'm finding this a bit confused and inconsistent. Comments below.

On 18/07/2015 3:30 AM, Derek White wrote:
> Request for review:
>
> [This updated webrev is being sent to wider audience, and has been
> merged with Gerard's option constraints check-in. Also factored out
> -XX:+AggressiveHeap processing into it's own chapter. I mean function :-)]
>
> http://cr.openjdk.java.net/~drwhite/8066821/webrev.06/
> https://bugs.openjdk.java.net/browse/JDK-8066821

argument.cpp:

  258  *               been not scheduled).

-> not been scheduled

  260  *     OBSOLETE: An option may be removed (and deleted from 
globals.hpp), but still be accepted on the command
  261  *               line. A warning is printed to let the user know 
that support may be removed in the future.


Isn't this the stronger case that support has already been removed (the 
option does nothing) and it will be removed at the next major release. 
At the start of a major release we should be able to delete all entries 
from the obsolete table - else it wasn't obsolete but deprecated.

Not sure "obsolete" is the right term here - obsolete things still 
function. For us an obsolete option does nothing (it exists only in the 
obsolete table).

  276  * Tests:  Aliases are tested in VMAliasOptions.java.
  277  *         Deprecated options are tested in VMDeprecatedOptions.java.
  278  *         Obsolete options are tested in various files.

We don't normally document this kind of thing in the source.

  281 // Obsolete or deprecated -XX flag.

I can tell this is going to get confusing already.

  284   JDK_Version obsoleted_in; // When the warning started (obsolete 
or deprecated).

But there is a difference: deprecated == warn but still process; 
obsolete == warn and ignore.

  288 // When a flag is eliminated, it can be added to this list in order to
  289 // continue accepting this flag on the command-line, while issuing 
a warning
  290 // and ignoring the value.  Once the JDK version reaches the 
'accept_until'
  291 // limit, we flatly refuse to admit the existence of the flag.
  292 static SpecialFlag const obsolete_jvm_flags[] = {

When a flag is eliminated it is gone from this table. As soon as the 
accept_until version is the current version we wipe the table of all 
such entries. This should be one of the first things done in a new release.

320 // When a flag is deprecated, it can be added to this list in order 
to issuing a warning when the flag is used.
  321 // Once the JDK version reaches the 'accept_until' limit, we 
flatly refuse to admit the existence of the flag.
  322 static SpecialFlag const deprecated_jvm_flags[] = {

A deprecated flag should be obsoleted before it reaches the accept_until 
limit. Which suggests that when we start a new version we wipe the 
obsoleted table and move the deprecated options into it.


776     case 1: {
  777       if (warn) {
  778         char version[256];
  779         since.to_string(version, sizeof(version));
  780         if (real_name != arg) {
  781           warning("Option %s was deprecated in version %s and will 
likely be removed in a future release. Use option %s instead.",
  782                   arg, version, real_name);
  783         } else {
  784           warning("Option %s was deprecated in version %s and will 
likely be removed in a future release.",
  785                   arg, version);
  786         }

This isn't distinguishing between deprecated and obsoleted ???

997       warning("Ignoring option %s; support was removed in %s", 
stripped_argname, version);

You have changed a handful of warnings so they start with a capital 
letter, and have changed the associated tests. But this seems a 
pointless "convention" because we have dozens of warnings that don't 
start with a capital.

Thanks,
David
-----

> This webrev adds support for handling "/deprecated/" -XX options
> (options that still *do* something but are planned for removal) and
> "/alias/" options (alternate names for other -XX options) by simply
> adding entries to the deprecated_jvm_flags and/or aliased_jvm_flags
> tables. This follows the example of the existing obsolete_jvm_flags table.
>
> This replaces a lot of ad-hoc and occasionally wrong code in
> arguments.cpp as well as supporting automatically disabling options
> after a certain version.
>
> Tests:
> Deprecated and alias options can be tested by adding entries to tables
> in new tests:
>    VMAliasOptions.java
>    VMDeprecatedOptions.java
>
> The new tests subsume these existing tests:
>    test/gc/startup_warnings/TestDefaultMaxRAMFraction.java
>    test/gc/startup_warnings/TestNoParNew.java
>
> Tests run:
>      jprt
>      jtreg (investigating errors in resource mgmt tests running on SE
> embedded that seems unrelated to these changes.)
>
> Thanks,
>
>   - Derek
>