RFR 8015391: NPG: With -XX:+UseCompressedKlassPointers OOME due to exhausted metadat, a space could occur when metaspace is almost empty
Coleen Phillimore
coleen.phillimore at oracle.com
Fri Jun 28 15:17:39 PDT 2013
Thanks Jon! Comments inline.
On 6/28/2013 5:22 PM, Jon Masamitsu wrote:
> 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.
You must have sent this new code to hotspot-gc when I wasn't watching.
I didn't notice the different running counts. So
allocated_capacity_bytes(Metaspace::ClassType) is how much space has
been allocated for chunks in the class metaspace. allocated_used_bytes()
is the running count of returned for class types.
I think for comparison purposes to MaxMetaspaceSize, we want
capacity_bytes() because that's what is committed in the class virtual
space. So we use reserved(data_space) + committed(class_space) to
determine if the MaxMetaspaceSize is reached. I updated the comment
also (and reran the tests).
See version 3
http://cr.openjdk.java.net/~coleenp/8015391_3/
<http://cr.openjdk.java.net/%7Ecoleenp/8015391_3/>
>
> 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.
We pass these objects to the serviceability agent but it never used
them, so I decided to not pass these things. The SA doesn't need to
know which exceptions we've preallocated.
Coleen
>
> 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