-XX:MaxDirectMemorySize argument parsing

[David Holmes]

Okay I've looked at this and the patch seems reasonable.

My only concern, also voiced by Alan, is that if anyone (real app or 
test program) is currently explicitly passing -1 today then they will 
start to fail with this change. But I think I can live with that as I 
don't see any references to being able to use -1 this way. Nor can I 
find any tests that set this to -1.

Interestingly JRockit's default for this is zero, which means it is 
unbounded.

A second review from runtime is still needed for this.

Do you have OpenJDK Contributor status Chris?

David

On 6/06/2012 9:28 PM, David Holmes wrote:
> On 6/06/2012 1:39 PM, David Holmes wrote:
>> Hi Chris,
>>
>> Does changing to uintx help enough? That still only permits a 4GB
>> maximum value. I would expect 64-bit to potentially want more.
>
> Ignore that. I didn't realize intx/uintx are actually intptr_t and so
> 64-bit on 64-bit.
>
> David
> -----
>
>> David
>>
>> On 6/06/2012 12:35 PM, Chris Dennis wrote:
>>> Hi David,
>>>
>>> Here's the patch I'm proposing - I've just changed the type to
>>> unsigned, and then I'm doing the usual memory size validation on the
>>> passed in value. I then pass in the explicit string "-1" if the flag
>>> is using it's default value and if not print the option value just as
>>> before. Does this all look reasonable?
>>>
>>> Chris
>>>
>>>
>>>
>>>
>>>
>>>
>>> On Jun 5, 2012, at 9:07 PM, David Holmes wrote:
>>>
>>>> Hi Chris,
>>>>
>>>> On 6/06/2012 1:36 AM, Chris Dennis wrote:
>>>>> This topic started life as a discussion around some test changes for
>>>>> 7172708. While working on modifying the LimitDirectMemory.sh test to
>>>>> cover the bug I discovered some deficiencies in the test that had
>>>>> allowed a few small regressions in code behavior. Starting a
>>>>> discussion brought up some more issues with the argument parsing.
>>>>> The original thread in jdk7u-dev contains the full context of this
>>>>> discussion (subject: "7172708: 32/64 bit type issues on Windows").
>>>>> Alan Bateman suggested we move this discussion here, so I'm going to
>>>>> attempt to summarize the issues as they currently stand (I'm not
>>>>> including the faulty behavior caused by 7172708).
>>>>>
>>>>> A: -XX:MaxDirectMemorySize=-1: This currently results in the JVM
>>>>> using the "default" value for MaxDirectMemorySize
>>>>> (Runtime.getRuntime().maxMemory()) since MaxDirectMemorySize uses
>>>>> "-1" as it's default value (and is consequently typed as intx and
>>>>> not uintx).
>>>>
>>>> Correct. -1 is used to indicate "use default". The default is handled
>>>> on the JDK side - in sun.misc.VM. There has to be some value for the
>>>> flag that means "use the default". So I don't see this changing.
>>>>
>>>>> B: -XX:MaxDirectMemorySize=-?: Any other negative value results in
>>>>> the JVM using 64M as the value for MaxDirectMemorySize.
>>>>
>>>> Again this is the behaviour of sun.misc.VM. It sets a default of 64M
>>>> and only updates it if the MaxDirectMemorySize is not -1 but is> 0
>>>>
>>>>> C: -XX:MaxDirectMemorySize=5g: On a 32-bit JVM this gets silently
>>>>> narrowed down to 1g.
>>>>
>>>> What you are seeing here is a simple truncation from 64-bit to
>>>> 32-bit, resulting in the following:
>>>>
>>>> 1G = 1073741824
>>>> 2G = -2147483648
>>>> 3G = -1073741824
>>>> 4G = 0
>>>> 5G = 1073741824
>>>>
>>>> The type of the flag is limiting its max value< 2048MB
>>>>
>>>>> D: -XX:MaxDirectMemorySize=foo: This causes the JVM to fail with:
>>>>> Error: Could not create the Java Virtual Machine.
>>>>> Error: A fatal exception has occurred. Program will exit.
>>>>
>>>> If you give the wrong type of value to an option it is treated as an
>>>> unknown option. You should see:
>>>> Unrecognized VM option 'MaxDirectMemorySize=foo'
>>>>
>>>> though that may depend on the VM version (7+ ?)
>>>>
>>>>> I'm in the process of fixing A, B and C, as I'm assuming the correct
>>>>> resolutions are pretty obvious: they should all be illegal values.
>>>>> Does anyone think fixing would cause problems (e.g. backwards
>>>>> compatibility related)?
>>>>
>>>> Seems to me that if this flag needs to be used to pass very large
>>>> values then it needs to be made a 64-bit "long".
>>>>
>>>> We should also sanity check the value ( -1 ||>0) and check it is<=
>>>> Integer.MAX_VALUE. Right now hotspot doesn't look at this flag, it
>>>> just sets its numeric value and uses that to set the property used on
>>>> the JDK side.
>>>>
>>>> David
>>>> -----
>>>>
>>>>> I haven't looked at the root cause of D yet, but this message is
>>>>> printed for all illegal options passed to JVM (rather then the old
>>>>> "Unrecognized..." message) so D to me looks like a more global issue.
>>>>>
>>>>> Chris
>>>