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.java.net/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.java.net/pipermail/hotspot-gc-dev/attachments/20130116/70d4a7c1/jesper_wilhelmsson.vcf>


More information about the hotspot-gc-dev mailing list