RFR: 8035057: NewSize ergonomics wrong when setting small or unaligned size on command line

[Jon Masamitsu]

On 3/4/2014 4:12 PM, Stefan Johansson wrote:
> Jon,
>
> Thanks for looking at this, see inline.
>
> On March 4, 2014 8:54:37 PM CET, Jon Masamitsu <jon.masamitsu at oracle.com> wrote:
>> Stefan,
>>
>> I got side tracked on this review.  I thought that the macro
>> FLAG_IS_CMDLINE would remember if the flag was sent on
>> the command line.  Unfortunately, it does not.
>>
>> Is the problem that NewSize is set somewhere with
>> FLAG_SET_ERGO when it should not have been?
>> That is, if it has been set on the command line,
>> it should not be reset ergonomically.
>>
> The problem here is when the size set on the command line is unaligned, then we need to change it. We could reset it using FLAG_SET_CMDLINE, but then we lose the information that we change it. The best would of course be if the flags could remember all their states, but that's more like a feature than a simple bug fix.

It doesn't feel right that we're saving that a flag was set on the
command line in CollectorPolicy.   Also, I usually think of
FLAG_SET_ERGO as setting a flag because of an ergonomic
policy.  That a size has to be correctly aligned holds for however
the size was originally set.  I'm tempted to just do a
naked assignment for fixing the alignment.  Unless your
interested in fixing the Flags (globals.hpp) so that it
remembers that it was set on the command line.

Jon

>
> Stefan
>
>> Jon
>>
>> On 2/20/14 8:52 AM, Stefan Johansson wrote:
>>> Hi,
>>>
>>> Can I please have some reviews for this small fix for:
>>> https://bugs.openjdk.java.net/browse/JDK-8035057
>>>
>>> Webrev:
>>> http://cr.openjdk.java.net/~sjohanss/8035057/
>>>
>>> Summary:
>>> When fixing JDK-8033426, the case when NewSize is set on the command
>>> line but not aligned was not handled. This small change makes sure
>>> that if NewSize is given on the command line we will align the value
>>> and use it as both initial and minimum young generation size.
>>>
>>> Testing:
>>> * Build on JPRT.
>>> * Manual sanity testing.
>>>
>>> Thanks,
>>> Stefan