RFR: 8066821(S) Enhance command line processing to manage deprecating and obsoleting -XX command line arguments
thomas.schatzl at oracle.com
Wed Jan 14 12:28:47 UTC 2015
On Mon, 2015-01-12 at 17:10 -0500, Derek White wrote:
> 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 (including Arguments::check_deprecated_gc_flags) as well
> as supporting automatically disabling options after a certain version.
> Removed Code:
> - Removed global DefaultMaxRAMFraction, which was an "improper" alias
> for "MaxRAMFraction" (two variables that were roughly kept in sync vs.
> two names for the same variable).
> - Arguments::check_deprecated_gc_flags().
> - Alias handling code in Arguments::parse_each_vm_init_arg().
> It also avoids future ad-hoc and occasionally wrong code as new
> options get aliased and deprecated.
> Deprecated and alias options can be tested by adding entries to tables
> in new tests:
> The new tests subsume these existing tests:
- in the big comment please avoid adding special headers/footers to
sections of the file. Over time they just get orphaned and waste up
- I would prefer to use "special" comments like "/**". We "mostly"
also only ever use the "//" comment style for explanation in cpp files.
(Waiting for somebody to mention that in file XY we do differently -
okay just saw one in arguments.cpp, but generally even in that file "//"
is used even for multi-line comments ;)
- the comment is not about (general) -XX argument processing, but
processing of obsolete and deprecated arguments.
- note that not only globals.hpp may contain -XX arguments, but also
- typo: Obosloete -> Obsolete
- not sure if the type name "SpecialFlag" is very descriptive.
- comments always start with upper case (e.g. "// when the
warning ...") with full stop at the end in the SpecialFlag struct.
- I would prefer to remove the "Declare an" before the description of
SpecialFlag (or "Describes ..."). It does not seem to add anything. Also
I would move part of the description of the deprecated_vm_flags to the
SpecialFlag description, because it seems to describe what a SpecialFlag
is, and not what the deprecated_jvm_flags table contains.
- maybe add a one-liner what AliasedFlag really is to its description
instead of speaking generally about what an alias is (which has already
been done in the large introductory comment).
- please only describe the interface in the definition of a method
(i.e. in the hpp file), not in the implementation (cpp file). If you
have a general implementation specific comment you may put it into the
cpp file. (.e.g is_newly_obsolete)
- maybe line up the closing braces of the aliased_jvm_flags array too
like the others
- the comment for is_deprecated_flag() is wrongly indented. Also the
second sentence has odd additional spaces.
- do not try to repeat the exact same comment for a method in the cpp
file. Actually the one for is_deprecated_flag() in the cpp file is
already different and apparently outdated...
- extra closing bracket after "(should be ignored)"
- the comment for handle_aliases_and_deprecation() should start with
- what is a "real" name of an option argument?
- if the copyright for a particular file spans multiple years, the
official format is "X, Z", not "X, Y, Z"
Rest looks okay.
More information about the hotspot-gc-dev