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

[Joseph Provino]

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