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

Derek White derek.white at oracle.com
Tue Aug 25 22:02:42 UTC 2015


Thanks Karen!

If I don't get any other feedback, can I get you to sponsor the change 
for me? After folding in your fixes...

  - Derek

On 8/25/15 2:05 PM, Karen Kinnear wrote:
> Derek,
>
> Many thanks for the latest changes. And thank you for the verify_special_jvm_flags -- that will remind us to
> clean up the flags when the version number changes. So the expired_in information is for internal use only - we
> don't print that information for users, we simply tell them "in a future release".
>
> Looks good. No need for my further code review.
>
> Minor comments:
> 1. arguments.hpp
> line 421 "In the case" -> "In this case"
>
> 2. arguments.cpp comments
>
> I love the high level comments - could you possibly update them?
>     - e.g. they still talk about the deprecated_jvm_flags and obsolete_jvm_flags tables.
>     - e.g. expired and obsolete options should have their global variable definitions and
>       related implementations removed ...
>
> comment line 2119 - extra spaces at the front? (or webrev artifact?)
>
> thanks very much,
> Karen
>
>
>
>
> On Aug 24, 2015, at 1:27 PM, Derek White wrote:
>
>> RFR:
>>
>> This version incorporates Dave and Karen's suggestions to automatically handle deprecated-> obsolete transitions. This is implemented with a unified table of deprecated and obsolete options ("special_jvm_flags").
>>
>> Karen also suggested (offline) that it would be good to verify that obsolete and expired options no longer have a "globals" variable defined. This revision adds debug checking of the special_jvm_flags table to check a bunch of constraints, such as deprecated version must be < obsolete version < expired version, and checks for duplicate table entries as well as looking for left-over "globals" variables.
>>
>> http://cr.openjdk.java.net/~drwhite/8066821/webrev.08
>> https://bugs.openjdk.java.net/browse/JDK-8066821
>>
>> I also have a webrev of webrev.07 vs. webrev.08:
>> http://cr.openjdk.java.net/~drwhite/8066821/webrev.7.v.8
>>
>> Thanks for looking!
>>
>> - Derek
>>
>> On 8/11/15 3:33 PM, Derek White wrote:
>>> Hi Karen,
>>>
>>> As much as I want to get this RFE over and out, I see your and David H's point :-)
>>>
>>> I'll spin another rev...
>>>
>>> - Derek
>>>
>>> On 8/10/15 6:07 PM, Karen Kinnear wrote:
>>>> Derek,
>>>>
>>>> Thank you for all of your work on this and responses to suggestions.
>>>>
>>>> I had a couple of questions/comments:
>>>>
>>>> I appreciate that you have created mechanisms to allow:
>>>> 1. 2 step obsolete: obsolete_jvm_flags
>>>>      - step 1: obsolete: warn and ignore
>>>>      - step 2: expire
>>>>      - and I expect we will use this for most -XX flags, especially develop flags
>>>>
>>>> 2. 2 step deprecate: deprecated_jvm_flags
>>>>      - step 1: deprecate: warn and handle
>>>>      - step 2: expire
>>>>
>>>> I see this approach as useful for aliasing and for a few other flags, but not
>>>> the common path.
>>>>
>>>> My concerns:
>>>>
>>>> With my understanding of how this is set up today, for the flags that are most exposed to
>>>> customers, which would want the 3 step deprecation, it would be too easy for an engineer
>>>> to add the flag to the deprecated_jvm_flags and forget to add it to the obsolete_jvm_flags,
>>>> as the comments under deprecated_jvm_flags suggest.  And we don't want folks to have
>>>> to go back and do a second step later for flags if they could do it all at once.
>>>>
>>>> I think it would be confusing to add a flag to both deprecated_jvm_flags and obsolete_jvm_flags
>>>> at the same time, since then the expiration release is not clear.
>>>>
>>>> 3. For product non-XX, commercial and common -XX flags what I would like to see is
>>>> a three-step table which I would have called deprecated_jvm_flags, which did
>>>>      - step 1: deprecate: warn and handle
>>>>      - step 2: obsolete: warn and ignore
>>>>      - step 3: expire: unrecognized error
>>>>
>>>> Would you be willing to add a table like that which is the recommended approach
>>>> for external supported flags?
>>>>
>>>> So one way to do this would be to change the current deprecated_jvm_flags to have
>>>> three fields, so that the usual path would be to have three releases listed, and
>>>> for aliased flags leave the middle field with the undefined version.
>>>>
>>>> Another approach would be to have the three fields and have it used by
>>>> all three approaches, just fill in the steps that are appropriate.
>>>>
>>>> minor comment:
>>>>
>>>>
>>>> 1. arguments.hpp
>>>> Could you possibly change the comments from "and has expired (should be ignored)" to
>>>> "and has expired (unrecognized error)"
>>>>
>>>> unless what you mean really is should be ignored, which is not the same expired.
>>>>
>>>> thanks,
>>>> Karen
>>>>
>>>>
>>>>
>>>> On Aug 10, 2015, at 12:41 PM, Derek White wrote:
>>>>
>>>>> Thanks David!
>>>>>
>>>>> - Derek
>>>>>
>>>>> On 8/10/15 1:53 AM, David Holmes wrote:
>>>>>> Hi Derek,
>>>>>>
>>>>>> I don't have any further comments.
>>>>>>
>>>>>> Thanks,
>>>>>> David
>>>>>>
>>>>>> On 7/08/2015 12:46 PM, Derek White wrote:
>>>>>>> Another RFR.
>>>>>>>
>>>>>>> I've updated based on David and Kim's last comments.
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~drwhite/8066821/webrev.07/
>>>>>>> https://bugs.openjdk.java.net/browse/JDK-8066821
>>>>>>>
>>>>>>> I also have a webrev of webrev.06 vs. webrev.07:
>>>>>>> http://cr.openjdk.java.net/~drwhite/8066821/webrev.6.v.7/
>>>>>>>
>>>>>>> [This webrev is confused about CommandLineOptionTest.java. I
>>>>>>> double-checked with a recursive diff and the only differences are in
>>>>>>> arguments.cpp and arguments.hpp.]
>>>>>>>
>>>>>>> Thanks for looking!
>>>>>>>
>>>>>>>   - Derek
>>>>>>> ....



More information about the hotspot-dev mailing list