request for review (s) - 8005452: Create new flags for Metaspace resizing policy

[Jon Masamitsu]

I added you improved message.  Thanks.

Jon

On 1/29/2013 5:18 AM, Jesper Wilhelmsson wrote:
> Jon,
>
> OK, in that case I think the error message could be slightly more 
> informative, just so that it is clear to someone that only sets one of 
> them that the flag they set conflicts with the default value of 
> another flag. How about this:
>
> jio_fprintf(defaultStream::error_stream(),
>             "MinMetaspaceFreeRatio (%s" UINTX_FORMAT ") must be less 
> than or "
>             "equal to MaxMetaspaceFreeRatio (%s" UINTX_FORMAT ")\n",
>             FLAG_IS_DEFAULT(MinMetaspaceFreeRatio) ? "Default: " : "",
>             MinMetaspaceFreeRatio,
>             FLAG_IS_DEFAULT(MaxMetaspaceFreeRatio) ? "Default: " : "",
>             MaxMetaspaceFreeRatio);
>
> If you don't like this I'm fine with your current patch as well.
> /Jesper
>
>
> On 28/1/13 8:30 PM, Jon Masamitsu wrote:
>> Jesper,
>>
>> If the user is increasing MinMetaspaceFreeRatio, and
>> MaxMetaspaceFreeRatio is not compatible, maybe we
>> should be forcing the user to think about MaxMetaspaceFreeRatio.
>> It's not obvious to me that something like
>>
>> MaxMetaspaceFreeRatio = MinMetaspaceFreeRatio + 1
>>
>> is a good choice.
>>
>> Jon
>>
>> On 01/28/13 07:08, Jesper Wilhelmsson wrote:
>>> On 2013-01-26 00:50, Jon Masamitsu wrote:
>>>> I've update the webrev2 (now 2 separate webrevs)  for review comments.
>>>>
>>>> 8005452: NPG: Create new flags for Metaspace resizing policy
>>>>
>>>> http://cr.openjdk.java.net/~jmasa/8005452/webrev.03/
>>>
>>> I have looked at the flag changes and they look good. I have a
>>> question though. How do we want to handle the case where the user sets
>>> only MinMetaspaceFreeRatio = 30 ? With your current change this will
>>> give an error because the min is larger than the default max (20).
>>>
>>> Would it make sense to assume that the user actually wants to use
>>> min=30 and increase max to 30 as well? (or slightly more if they can't
>>> be equal) And maybe issue a warning that the value of max has been
>>> changed.
>>> The error would then just be given if the user specifies both flags
>>> and they don't work out.
>>>
>>> I'm not asking you to do this change now but it relates to other
>>> changes we have done recently.
>>> /Jesper
>>>
>>>
>>>>
>>>> 8006815: NPG: Trigger a GC for metadata collection just before the
>>>> threshold
>>>> is exceeded.
>>>>
>>>> http://cr.openjdk.java.net/~jmasa/8006815/webrev.01/
>>>>
>>>>
>>>> On 1/16/2013 8:50 AM, Jon Masamitsu wrote:
>>>>> I've added checks to arguments.cpp that are analogous to the
>>>>> checks for MinHeapFreeRatio / MaxHeapFreeRatio
>>>>>
>>>>> Changes since webrev.00 are in arguments.cpp
>>>>>
>>>>> http://cr.openjdk.java.net/~jmasa/8005452/webrev.01/
>>>>>
>>>>> Thanks, Vitaly.
>>>>>
>>>>> Jon
>>>>>
>>>>> On 1/15/2013 5:55 AM, Vitaly Davidovich wrote:
>>>>>> Hi Jon,
>>>>>>
>>>>>> Does it make sense to validate that the new flags are consistent
>>>>>> (I.e. max
>>>>>>> = min)? That is, if user changes one or both such that max<  min,
>>>>>>> should
>>>>>> VM report an error and not start?
>>>>>>
>>>>>> Thanks
>>>>>>
>>>>>> Sent from my phone
>>>>>> On Jan 14, 2013 10:31 AM, "Jon Masamitsu"<jon.masamitsu at oracle.com>
>>>>>> wrote:
>>>>>>
>>>>>>> 8005452: Create new flags for Metaspace resizing policy
>>>>>>>
>>>>>>> Previously the calculation of the metadata capacity at which
>>>>>>> to do a GC (high water mark, HWM) to recover
>>>>>>> unloaded classes used the MinHeapFreeRatio
>>>>>>> and MaxHeapFreeRatio to decide on the next HWM.  That
>>>>>>> generally left an excessive amount of unused capacity for
>>>>>>> metadata.  This change adds specific flags for metadata
>>>>>>> capacity with defaults more conservative in terms of
>>>>>>> unused capacity.
>>>>>>>
>>>>>>> Added an additional check for doing a GC before expanding
>>>>>>> the metadata capacity.  Required adding a new parameter to
>>>>>>> get_new_chunk().
>>>>>>>
>>>>>>> Added some additional diagnostic prints.
>>>>>>>
>>>>>>> http://cr.openjdk.java.net/~**jmasa/8005452/webrev.00/<http://cr.openjdk.java.net/~jmasa/8005452/webrev.00/> 
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>>
>>>>>>> Thanks.
>>>>>>>