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

[Jon Masamitsu]

On 1/25/2013 1:37 AM, Stefan Karlsson wrote:
> ...
>> Do you mean that I should put the check should_expand() after
>> line 991.  That would be better.
>
>
> Yes. It seemed like you could return NULL without using all memory in 
> the VirtualSpace.

Fixed.

Jon

>
> StefanK
>
>>
>> Or do you mean that the test should look at the current capacity (as it
>> did before) and not at the capacity after the addition of the chunk (for
>> comparing to the HWM)?
>>
>>>  987
>>>  988   // Allocate a chunk out of the current virtual space.
>>>  989   if (next == NULL) {
>>>  990     next = 
>>> current_virtual_space()->get_chunk_vs(grow_chunks_by_words);
>>>  991   }
>>>
>>> Shouldn't this line be checking "less than or equal":
>>>
>>> *!    if (_(_vsl->capacity_words_sum(_) + expansion_word_size_) < 
>>> metaspace_size_words ||*
>>>         capacity_until_GC() == 0) {
>>>       set_capacity_until_GC(metaspace_size_words);
>>>       return true;
>>>     }
>>
>> Yes. Fixed.
>>
>>>
>>>> Webrev with the Min/MaxMetaspaceFreeRatio changes is
>>>>
>>>> http://cr.openjdk.java.net/~jmasa/8005452/webrev.02/
>>>
>>> This looks good.
>>>
>>> Though, I think you need to update the descriptions of the new flags:
>>> +   product(uintx, MinMetaspaceFreeRatio, 
>>> 10,                              \
>>> +           "Min percentage of heap free after GC to avoid 
>>> expansion")        \
>>> + \
>>> +   product(uintx, MaxMetaspaceFreeRatio, 
>>> 20,                              \
>>> +           "Max percentage of heap free after GC to avoid 
>>> shrinking")        \
>>
>> Fixed.
>>
>> Jon
>>>
>>> thanks,
>>> StefanK
>>>
>>>>
>>>> Jon
>>>>
>>>> On 1/23/2013 9:01 AM, Coleen Phillimore wrote:
>>>>>
>>>>> It looks okay except I think passing SpaceManager* to 
>>>>> get_new_chunk() is really gross just to do a print out.  I'd 
>>>>> rather see the printing at line 1014 moved to 2170 whether you get 
>>>>> a new virtual space or not.  There's already a ton of output from 
>>>>> TraceMetadataChunkAllocation && Verbose, but you can leave the 
>>>>> printing at 1013 so you know which one created a new virtual space.
>>>>>
>>>>> Coleen
>>>>>
>>>>>
>>>>> On 01/14/2013 10:30 AM, Jon Masamitsu 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/
>>>>>>
>>>>>> Thanks.
>>>>>
>>>
>>>
>