RFR (XS): JDK-8058967 metaspace/shrink_grow/CompressedClassSpaceSize fails with OOM: Compressed class space

[Joseph Provino]

On 5/15/2015 2:29 PM, Jon Masamitsu wrote:
>
>
> On 5/15/2015 6:50 AM, Joseph Provino wrote:
>> Fixed the bug id in the subject line.
>>
>> On 5/14/2015 5:26 PM, Kim Barrett wrote:
>>> On May 14, 2015, at 3:45 PM, Joseph Provino 
>>> <joseph.provino at oracle.com> wrote:
>>>> Can I get reviews for the following fix?  The problem is that an 
>>>> attempt is made
>>>> to allocate a medium chunk for class metaspace but there are no 
>>>> medium chunks available.
>>>> However there are many small chunks available.
>>>>
>>>> If the allocation request size fits in a small chunk, the fix is to 
>>>> try allocating a small
>>>> chunk after failing to allocate a medium chunk.
>>>>
>>>> Changes are in one file.
>>>>
>>>> http://cr.openjdk.java.net/~jprovino/8058967/webrev.00
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8058967
>>>>
>>>> Passed JPRT
>>>>
>>>> Aurora ad-hoc test of vm.parallel_class_loading:
>>>>
>>>> http://aurora.ru.oracle.com/functional/faces/RunDetails.xhtml?names=882047.VMSQE.adhoc.JPRT-1 
>>>>
>>>>
>>>> thanks.
>>>>
>>>> joe
>>> The proposed change violates the policy described in
>>> calc_chunk_size().  Either that policy is important and prevents this
>>> change, or the policy description needs to be updated (and an
>>> explanation of why that's ok needs to be provided).
>> I think the policy is important to reduce fragmentation but it also 
>> causes problems
>> when there is no room to expand and there are no medium chunks 
>> available.
>>
>> It seems better to use one of the small chunks than to throw OOME.
>>
>> I could change the comment to say something like that.
>
> I think the statement of the policy should be updated.

How does this sound?  Should the comment be with the "if" statement or
where _small_chunk_limit is declared?

   /*
    * The policy is to allocate up to _small_chunk_limit small chunks
    * after which only medium chunks are allocated.  This is done to
    * reduce fragmentation.  In some cases, this can result in a lot
    * of small chunks being allocated to the point where it's not
    * possible to expand.  If this happens, there may be no medium chunks
    * available and  OOME would be thrown.  Instead of doing that,
    * if the allocation request size fits in a small chunk, an attempt
    * is made to allocate a small chunk.
    */
   if (next == NULL &&
       word_size + Metachunk::overhead() <= small_chunk_size() &&
       grow_chunks_by_words == medium_chunk_size()) {
     /*
      * There are no medium chunks available but a small chunk is big 
enough.
      * See if a small chunk is available.
      */
     next = get_new_chunk(word_size, small_chunk_size());
   }

joe

>
>
> Jon
>
>>
>>> I wonder if the real problem might be that the test is making
>>> unreasonable assumptions, and needs to be changed.
>> I don't think so.  The test creates enough class loaders to use up 
>> all of class
>> metaspace.  Then it clears references to all the class loaders and 
>> does a gc.
>> The gc frees the class metaspace.  Then the test tries to create one 
>> more
>> class loader and OOME is thrown.
>>
>> It's certainly an unusual test case but seems reasonable to expect it 
>> to work.
>>
>> I think the real problem is in how metaspace is handled.  The policy 
>> to only allocate
>> up to 4 small chunks per SpaceManager seems to be bad if most of the 
>> time <= 4 small chunks are needed.
>> When memory is freed there are going to be lots of small chunks and 
>> few if any medium chunks.
>>
>>>
>>> 2090   if (next == NULL && word_size + Metachunk::overhead() <= 
>>> small_chunk_size() &&
>>> 2091     grow_chunks_by_words == medium_chunk_size()) {
>>>
>>> I think line breaks between expressions would make that test a lot
>>> easier to read.  And indentation should be based on the open-paren
>>> rather than the if; as written, grow_chunks_by_words is indented
>>> appropriately for the body of the if.
>>>
>>
>> like this?
>>
>>   if (next == NULL &&
>>       word_size + Metachunk::overhead() <= small_chunk_size() &&
>>       grow_chunks_by_words == medium_chunk_size()) {
>>
>> joe
>