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 15:37:34 PDT 2013
On 6/28/2013 3:17 PM, 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.
Yes, it went back not too long ago.
> So allocated_capacity_bytes(Metaspace::ClassType) is how much space
> has been allocated for chunks in the class metaspace.
Yes.
> allocated_used_bytes() is the running count of returned for class types.
If by "returned" you mean returned to the application, yes.
>
> 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/>
Looks good.
>
>>
>> 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.
Good. Less for me to know also (in terms of what SA has to know).
Jon
>
> 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