RFR (S): JDK-6348447 - Specifying -XX:OldSize crashes 64-bit VMs

[Jon Masamitsu]

Jesper,

I'm a bit concerned that set_heap_size() now knows about how
the CollectorPolicy uses OldSize and NewSize.   In the distant
past set_heap_size() did not know what kind of collector was
going to be used and probably avoided looking at those
parameters for that reason.  Today we know that a generational
collector is to follow but maybe you could hide that knowledge
in CollectorPolicy somewhere and have set_heap_size() call into
CollectorPolicy to use that information?

Jon


On 01/14/13 09:10, Jesper Wilhelmsson wrote:
> Hi,
>
> I would like a couple of reviews of a small fix for JDK-6348447 - 
> Specifying -XX:OldSize crashes 64-bit VMs
>
> Webrev:
> http://cr.openjdk.java.net/~jwilhelm/6348447/webrev/
>
> Summary:
> When starting HotSpot with an OldSize larger than the default heap 
> size one will run into a couple of problems. Basically what happens is 
> that the OldSize is ignored because it is incompatible with the heap 
> size. A debug build will assert since a calculation on the way results 
> in a negative number, but since it is a size_t an if(x<0) won't 
> trigger and the assert catches it later on as incompatible flags.
>
> Changes:
> I have made two changes to fix this.
>
> The first is to change the calculation in 
> TwoGenerationCollectorPolicy::adjust_gen0_sizes so that it won't 
> result in a negative number in the if statement. This way we will 
> catch the case where the OldSize is larger than the heap size and 
> adjust the OldSize instead of the young size. There are also some 
> cosmetic changes here. For instance the argument min_gen0_size is 
> actually used for the old generation size which was a bit confusing 
> initially. I renamed it to min_gen1_size (which it already was called 
> in the header file).
>
> The second change is in Arguments::set_heap_size. My reasoning here is 
> that if the user sets the OldSize we should probably adjust the heap 
> size to accommodate that OldSize instead of complaining that the heap 
> is too small. We determine the heap size first and the generation 
> sizes later on while initializing the VM. To be able to fit the 
> generations if the user specifies sizes on the command line we need to 
> look at the generation size flags a little already when setting up the 
> heap size.
>
> Thanks,
> /Jesper
>