RFR 8015391: NPG: With -XX:+UseCompressedKlassPointers OOME due to exhausted metadat, a space could occur when metaspace is almost empty

Jon Masamitsu jon.masamitsu at oracle.com
Fri Jun 28 14:22:48 PDT 2013


Coleen,

http://cr.openjdk.java.net/~coleenp/8015391_2/src/share/vm/memory/metaspace.cpp.frames.html
> 1301   // The reason for someone using this flag is to limit reserved space.  So
> 1302   // for non-class virtual space that is what we compare against.  For class
> 1303   // virtual space, we only compare against the used space, not reserved space,
> 1304   // because this is a larger region prereserved for compressed class pointers.
> 1305   if (!FLAG_IS_DEFAULT(MaxMetaspaceSize)) {
> 1306     size_t real_allocated = Metaspace::space_list()->virtual_space_total() +
> 1307               Metaspace::class_space_list()->used_words_sum() * BytesPerWord;
> 1308     if (real_allocated >= MaxMetaspaceSize) {
> 1309       return false;
> 1310     }
> 1311   }

At line 1307 the function used_words_sum() iterated over the virtual spaces.
There are usually not that many virtual spaces but I think the iteration
should still be avoided.  I think you should use

MetaspaceAux::allocated_capacity_bytes()

In a virtual space any space that has been allocated to a Metaspace is 
considered "used".
Not all that space holds Metadata yet.    I think that corresponds to 
the "allocated_capacity_bytes()"
above.  That is the running sum (i.e. fast) of bytes allocated to a 
Metaspace.  The allocated capacity
also does not necessarily hold Metadata yet.  The other choice would be
MetaspaceAux::allocated_used_bytes() which is a running sum of the space 
which
holds Metadata.  Though it is a "used" quantity, I think it is different 
than the
way "used" is in "used_words_sum". " used_words_sum()" should probably
be renamed "allocated_words_sum()".  But that's another clean up.

http://cr.openjdk.java.net/~coleenp/8015391_2/src/share/vm/runtime/vmStructs.cpp.frames.html

Why the delete of entries and not changing perm to Metadata named 
variables?  I'm not
challenging your change, just trying to learn about such things.

Jon




On 6/27/2013 6:14 PM, Coleen Phillimore wrote:
>
> Hi,  I have made a few changes to this changeset.  I reverted the 
> special case for application class loader until we have more tuning 
> information to decide how large this metaspace should be.    I also 
> fixed the check in should_expand so that if you specify 
> MaxMetaspaceSize, it compares the MaxMetaspaceSize to (reserved for 
> data space + used for class space).   So the test in the bug report 
> runs with the smaller sizes for MaxMetaspaceSize and also for smaller 
> ClassMetaspaceSize without hitting OOM for either sort of space.   
> Hope the comment there is clear.
>
> http://cr.openjdk.java.net/~coleenp/8015391_2/ 
> <http://cr.openjdk.java.net/%7Ecoleenp/8015391_2/>
>
> Reran nsk quick testlist and some jtreg tests.   Please (re)review.
>
> Thanks,
> Coleen
>
> On 6/24/2013 5:03 PM, Coleen Phillimore wrote:
>>
>> On 06/24/2013 02:43 PM, Jon Masamitsu wrote:
>>>
>>> On 6/24/13 11:23 AM, Coleen Phillimore wrote:
>>>>
>>>> On 06/24/2013 01:46 PM, Jon Masamitsu wrote:
>>>>> http://cr.openjdk.java.net/~coleenp/8015391/src/share/vm/memory/metaspace.cpp.frames.html 
>>>>>
>>>>>
>>>>>> 2673   size_t specialized_count = 0, small_count = 0, 
>>>>>> medium_count = 0, large_count = 0;
>>>>>> 2675   size_t cls_specialized_count = 0, cls_small_count = 0, 
>>>>>> cls_medium_count = 0, cls_large_count = 0;
>>>>>
>>>>> Not introduced by your change but
>>>>>
>>>>> humongous_count instead of large_count
>>>>> and
>>>>> cls_humongous_count instead of cls_large_count
>>>>>
>>>>> would be more consistent.
>>>>
>>>> Yes, it is.  I changed it.
>>>>
>>>>>
>>>>> http://cr.openjdk.java.net/~coleenp/8015391/src/share/vm/classfile/classLoaderData.cpp.frames.html 
>>>>>
>>>>>
>>>>> I'd suggest adding Metaspace::AppClassMetaspaceType
>>>>>>   385     } else if 
>>>>>> (SystemDictionary::is_app_class_loader(class_loader())) {
>>>>>>   386       if (TraceClassLoaderData && Verbose && class_loader() 
>>>>>> != NULL) {
>>>>>>   387 tty->print_cr("sun/misc/Launcher$AppClassLoader class 
>>>>>> loader");
>>>>>>   388       }
>>>>>>   389       set_metaspace(new Metaspace(_metaspace_lock, 
>>>>>> Metaspace::BootMetaspaceType));
>>>>> and adding a flag InitialAppClassLoaderMetaspaceSize. It's not 
>>>>> obvious to me
>>>>> that we should always be reserving  the same amount of space for 
>>>>> the AppClassLoader
>>>>> as the BootClassLoader.   The same default is OK with me.
>>>>
>>>> Hmm.  Ok.  I don't like adding new flags, so can I add an 
>>>> AppMetaspaceType and give it the same initial size as 
>>>> BootMetaspaceSize until proven that it doesn't need to be the same 
>>>> size.
>>>
>>> With the BootClassLoader the space for the initial Metaspace is 
>>> committed
>>> when the first system class is loaded.  The same would be true for any
>>> Java application (all have some type of AppLoader, right)? We know 
>>> we're
>>> going to use the space committed for the BootClassLoader Metaspace.  
>>> Are
>>> we going to get yelled at for committing 4m and not using it.
>>>>
>>>> I suppose adding an AppMetaspaceInitialSize flag will give me more 
>>>> to talk about at the Permgen Elimination JavaOne talk though (but 
>>>> I'd still rather not).
>>>
>>> When you have 100000000000000000 flags, who's going to
>>> notice the 100000000000000001-st :-)
>>
>> Okay, you convinced me to revert this part of the change.   It wasn't 
>> necessary to make this stress test finish and there hasn't been very 
>> much tuning for "normal" application classes, just a suspicion of mine.
>>
>> Committing an extra 4m might be bad for embedded platforms.
>>
>> Thanks,
>> Coleen
>>
>>>
>>> Jon
>>>>
>>>> Coleen
>>>>
>>>>>
>>>>> Rest looks good.
>>>>>
>>>>> Jon
>>>>>
>>>>>
>>>>> On 6/20/13 9:26 AM, Coleen Phillimore wrote:
>>>>>> Summary: Allocate medium chunks for class metaspace when class 
>>>>>> loader has lots of classes
>>>>>>
>>>>>> I originally made class metaspace keep small chunks and not start 
>>>>>> allocating from medium chunks, because I thought with only Klass 
>>>>>> objects, small chunks is enough.  This test has a large set of 
>>>>>> class loaders who have a lot of classes, so allocated thousands 
>>>>>> of small chunks each. Class loaders with that many classes should 
>>>>>> start allocating from medium chunks, for class metaspace.
>>>>>>
>>>>>> I also increased the ClassMediumChunk size to 4k and measured a 
>>>>>> lot less fragmentation waste than 1K or 2K for this test.  
>>>>>> Lastly, the AppClassLoader instance should have as large of an 
>>>>>> initial metaspace as the bootclass loader.
>>>>>>
>>>>>> Tested with vm.quick.testlist and the failing test.
>>>>>>
>>>>>> open webrev at http://cr.openjdk.java.net/~coleenp/8015391/
>>>>>> bug link at http://bugs.sun.com/view_bug.do?bug_id=8015391
>>>>>> local bug link https://jbs.oracle.com/bugs/browse/JDK-8015391
>>>>>>
>>>>>> Thanks,
>>>>>> Coleen
>>>>>>
>>>>>
>>>>
>>>
>>
>



More information about the hotspot-dev mailing list