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

[Jon Masamitsu]

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.

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