RFR(s): 8178364: Command-line flags of type double should accept integer values

David Holmes david.holmes at oracle.com
Mon Apr 10 12:49:28 UTC 2017

Hi Per,

On 10/04/2017 9:22 PM, Per Liden wrote:
> Hi David,
> On 2017-04-10 13:03, David Holmes wrote:
>> On 10/04/2017 5:40 PM, Per Liden wrote:
>>> Hi David,
>>> On 2017-04-10 09:27, David Holmes wrote:
>>>> Hi Per,
>>>> On 10/04/2017 5:05 PM, Per Liden wrote:
>>>>> Hi,
>>>>> For convenience, command-line flags of type double should accept
>>>>> integer
>>>>> values (as well as the normal double values). This would allow
>>>>> users to
>>>>> write the short form, e.g:
>>>>> -XX:G1ConcMarkStepDurationMillis=10
>>>>> instead of always having to do:
>>>>> -XX:G1ConcMarkStepDurationMillis=10.0
>>>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8178364
>>>>> Webrev: http://cr.openjdk.java.net/~pliden/8178364/webrev.0/
>>>>> Testing: JTreg test added, passes jprt
>>>> I don't understand why we aren't using set_fp_numeric_flag in this case
>>>> - and surely that already handles this case ??
>>> With the way this code is structured we only end up in
>>> set_fp_numeric_flag() if the string looks like a float to begin with,
>>> i.e. has the format "<flag>=<num>.<num>". If the flag looks like
>>> "<flag>=<num>" we end up in set_numeric_flag(). Have a look at the call
>>> sites for set_fp_numeric_flag() and set_numeric_flag() and you'll see.
>> That's somewhat odd - shouldn't the type of the flag dictate how we try
>> to parse it ??
> Indeed, Arguments::parse_argument() does things a bit backwards if you
> ask me, by "guessing" what's on the other side of the "=" instead of
> letting the flag type dictate how it's parsed. Maybe there is (or was) a
> reason for doing so that I don't see.
> Anyway, I think my patch is in line with the style/intent of the current
> code. Overhauling Arguments::parse_argument() is probably a good idea,
> but I think that's a separate enhancement.

Okay - I agree that what you have is a logical extension of the logic in 
set_numeric_flag. (but the logic above that is really quite odd to me :) ).

Was wondering if the testcase could somehow be added to one of the 
existing options validation tests? I admit I couldn't see where to try 
and fit it, but it sounds like something that an "options validation" 
test should be testing. :) If not, then test is okay.


> cheers,
> Per
>> David
>>> cheers,
>>> Per
>>>> Thanks,
>>>> David
>>>>> cheers,
>>>>> Per

More information about the hotspot-runtime-dev mailing list