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

[Stefan Johansson]

On 2014-03-05 15:58, Jon Masamitsu wrote:
>
> 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.
I don't feel that fixing the Flags is something I have time for right 
now, so I think that will have to wait.

I agree that it feels strange to store that information in the 
CollectorPolicy, but we do the same for the max heap size:
// Needed to keep information if MaxHeapSize was set on the command line
// when the flag value is aligned etc by ergonomics.
bool _max_heap_size_cmdline;

The reason I would like to use FLAG_SET_ERGO is that it is possible to 
look at the flags, their values and how they were set through JMC. It 
might be confusing for a user if they've set NewSize on the command line 
to one value, but JMC reports that it was set to an other value on the 
command line.

Makes sense?

Stefan
>
> 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
>