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
Tue Jul 2 05:40:17 PDT 2013
Thanks Mikael for the review and all the help on this change!
Coleen
On 07/02/2013 05:06 AM, Mikael Gerdin wrote:
> Coleen,
>
> On 2013-06-29 00:17, Coleen Phillimore wrote:
>>
>> 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/>
>
> This looks good to me.
> /Mikael
>
>>
>>>
>>> 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