RFR (S): JDK-6348447 - Specifying -XX:OldSize crashes 64-bit VMs
Jesper Wilhelmsson
jesper.wilhelmsson at oracle.com
Wed Jan 16 13:24:08 UTC 2013
On 2013-01-16 14:23, Vitaly Davidovich wrote:
>
> Looks good Jesper. Maybe just a comment there that NewRatio hasn't been
> checked yet but if it's 0, VM will exit later on anyway - basically, what
> you said in the email :).
>
Yes, I'll add a comment about that.
Thanks,
/Jesper
> Cheers
>
> Sent from my phone
>
> On Jan 16, 2013 7:49 AM, "Jesper Wilhelmsson" <jesper.wilhelmsson at oracle.com
> <mailto:jesper.wilhelmsson at oracle.com>> wrote:
>
>
> On 2013-01-16 09:23, Bengt Rutisson wrote:
>> On 1/15/13 2:41 PM, Jesper Wilhelmsson wrote:
>>> On 2013-01-15 14:32, Vitaly Davidovich wrote:
>>>>
>>>> Hi Jesper,
>>>>
>>>> Is NewRatio guaranteed to be non-zero when used inside
>>>> recommended_heap_size?
>>>>
>>> As far as I can see, yes. It defaults to two and is never set to zero.
>>
>> No, there is no such guarantee this early in the argument parsing. The
>> check to verify that NewRatio > 0 is done in
>> GenCollectorPolicy::initialize_flags(), which is called later in the
>> start up sequence than your call to
>> CollectorPolicy::recommended_heap_size() and it is never called for G1.
>>
>> Running with your patch crashes:
>>
>> java -XX:OldSize=128m -XX:NewRatio=0 -version
>> Floating point exception: 8
>
> Oh, yes, you're right. Sorry!
>
> Good catch Vitaly!
>
> New webrev:
> http://cr.openjdk.java.net/~jwilhelm/6348447/webrev.3
> <http://cr.openjdk.java.net/%7Ejwilhelm/6348447/webrev.3>
>
> I'm just skipping the calculation if NewRatio is zero. The VM will abort
> anyway as soon as it realizes that this is the case.
> /Jesper
>
>
>> Bengt
>>> /Jesper
>>>
>>>> Thanks
>>>>
>>>> Sent from my phone
>>>>
>>>> On Jan 15, 2013 8:11 AM, "Jesper Wilhelmsson"
>>>> <jesper.wilhelmsson at oracle.com
>>>> <mailto:jesper.wilhelmsson at oracle.com>> wrote:
>>>>
>>>> Jon,
>>>>
>>>> Thank you for looking at this! I share your concerns and I have
>>>> moved the knowledge about policies to CollectorPolicy.
>>>> set_heap_size() now simply asks the collector policy if it has
>>>> any recommendations regarding the heap size.
>>>>
>>>> Ideally, since the code knows about young and old generations, I
>>>> guess the new function "recommended_heap_size()" should be placed
>>>> in GenCollectorPolicy, but then the code would have to be
>>>> duplicated for G1 as well. However, CollectorPolicy already know
>>>> about OldSize and NewSize so I think it is OK to put it there.
>>>>
>>>> Eventually I think that we should reduce the abstraction level in
>>>> the generation policies and merge CollectorPolicy,
>>>> GenCollectorPolicy and maybe even TwoGenerationCollectorPolicy
>>>> and if possible G1CollectorPolicy, so I don't worry too much
>>>> about having knowledge about the two generations in CollectorPolicy.
>>>>
>>>>
>>>> A new webrev is available here:
>>>> http://cr.openjdk.java.net/~jwilhelm/6348447/webrev.2/
>>>> <http://cr.openjdk.java.net/%7Ejwilhelm/6348447/webrev.2/>
>>>>
>>>> Thanks,
>>>> /Jesper
>>>>
>>>>
>>>>
>>>> On 2013-01-14 19:00, Jon Masamitsu wrote:
>>>>
>>>> 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/
>>>> <http://cr.openjdk.java.net/%7Ejwilhelm/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
>>>>
>>>>
>>>
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20130116/70d4a7c1/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: jesper_wilhelmsson.vcf
Type: text/x-vcard
Size: 236 bytes
Desc: not available
URL: <https://mail.openjdk.org/pipermail/hotspot-gc-dev/attachments/20130116/70d4a7c1/jesper_wilhelmsson.vcf>
More information about the hotspot-gc-dev
mailing list