RFR (S): 8006088: Incompatible heap size flags accepted by VM

[Jon Masamitsu]

This comment is only correct if InitialHeapSize is not set on the
command line or at least that's  my understanding.

2355       // Currently the minimum size and the initial heap sizes are the same.

Until InitialHeapSize was introduced, the initial heap size was the 
minimum heap size.

Sometime during jdk7 development the default heap size for the client VM 
was changed
to use the same (or at least similar ergonomics heap sizing as with the 
server VM) with
the exception of windows (maybe, not sure about the windows exception).  
The minimum
heap size was not changed as far as I know.   Part of the ergonomics 
heap sizing is
that the initial heap size is calculated and is not the minimum heap 
size.  The motivation
for the initial heap size change was faster start up (didn't have to go 
through GC's to get a
bigger heap).  The motivation for the client VM using the ergonomics 
heap sizing was
easy of use.  With the minimum heap size staying the same, if the 
application did not
need a heap that was as large as the initial heap size,  the heap sizing 
would shrink
the heap back down.  That way any footprint increase would be transitory 
at start up
and the heap size would evolve to what it had been before (was the hope).

The actual flag InitialHeapSize was introduced to give the user some 
control over
the InitialHeapSize if such control was really needed but I don't think 
it was supposed
to affect the minimum heap size.

If I run fastdebug  jdk8 on solaris with

java -client -XX:+PrintGCDetails -XX:+Verbose -XX:InitialHeapSize=32m 
-version

part of the Verbose output says

Minimum heap 5242880  Initial heap 33554432  Maximum heap 268435456

I think this changeset would make the minimum heap 32m.

Jon


On 3/22/2013 5:12 AM, Thomas Schatzl wrote:
> Hi all,
>
>    ping?
>
> On Mon, 2013-03-18 at 13:53 +0100, Thomas Schatzl wrote:
>> Hi all,
>>
>> On Thu, 2013-03-14 at 11:17 +0100, Mikael Gerdin wrote:
>>> Thomas,
>>>
>>> On 2013-03-14 09:28, Thomas Schatzl wrote:
>>>> Hi all,
>>>>
>>>>     please have a look at the following change which tries to make heap
>>>> size argument processing more intuitive.
>>>>
>>>> The second two are no bugs imo as the user did not mention a bound for
>>>> the maximum heap size in the arguments, so it would be free to choose
>>>> anything >= InitialHeapSize.
>>>>
>>>>
>>>> Webrev:
>>>> http://cr.openjdk.java.net/~tschatzl/8006088/webrev/
>>>>
>>>> The problem with the VM starting up when InitialHeapSize > MaxHeapSize
>>>> is fixed in the first hunk of collectorPolicy.cpp where the respective
>>>> sanity check has been added.
>>>>
>>>> The change for the "incompatible minimum and initial heap size is located
>>>> in arguments.cpp. I simply made the processing of -XX:InitialHeapSize the
>>>> same as -Xms and -XX:MaxHeapSize the same as -Xmx.
>>>>
>>>> The latter for consistency (using the same code path for -Xmx and
>>>> -XX:MaxHeapSize), as at the moment it does not set any internal variables.
>>>>
>>>> The issues in (3) are fixed in the last two hunks of collectorPolicy.cpp
>>>> where if New- and OldSize are greater than MaxHeapSize *and* MaxHeapSize
>>>> has been set on the command line, New- and OldSize are shrunk to fit.
>>> The code looks like it does what you describe :)
>>>
>>> Since the arguments parsing code is kind of hard to follow IMO and
>>> seemingly hard to get right it might be a good idea to write a
>>> regression test for this.
>>>
>>> Just writing a short jtreg test to launch vms with these arg
>>> combinations that you described above and verify that we get the
>>> expected outcome should not be too much code with the ProcessTools test
>>   done, added jtreg tests.
>>
>> Webrev:
>> http://cr.openjdk.java.net/~tschatzl/8006088/webrev.1/
>>
>> Note that the code additionally removes two asserts that are imo invalid
>> to do at this point.
>>
>> I.e.
>>
>> assert(max_heap >= InitialHeapSize, "Error");// or >= OldSize in 7196080
>> assert(max_heap >= NewSize, "Error");
>>
>> The max_heap >= OldSize assert is problematic because its default value
>> is 4M. So again, if you set MaxHeapSize to e.g. 1-3M, the assert will
>> fail.
>>
>> The same applies to NewSize with a sufficiently small heap.
>>
>> There is the new sanity check collectorPolicy.cpp that checks whether
>> MaxHeapSize >= InitialHeapSize also in product mode (i.e. conflicting
>> command line params have been set).
>>
>> Additionally, NewSize and OldSize are adjusted to MaxHeapSize (or
>> MaxHeapSize to NewSize + OldSize) later in the collector policy too.
>>
>> These asserts also cause different behavior with/without debug mode.
> Note that in 7196080 these asserts have already been removed.
>
> Thomas
>
>
>