RFR(s): 8025279 metaspace/flags/maxMetaspaceSize throws OOM: out of Compressed Klass space

[Jesper Wilhelmsson]

OK, then I have no objections.

Ship it!
/Jesper


Mikael Gerdin skrev 26/9/13 10:21 AM:
> Jesper,
>
> thanks for looking at this!
> I've replied to your question inline.
>
> On 09/26/2013 10:03 AM, Jesper Wilhelmsson wrote:
>> Mikael,
>>
>> The metaspace is not my domain so I may be wrong here, but there are two
>> more place where there is a similar check as the ones you have protected
>> with using_class_space() in your change.
>>
>> In Metaspace::get_space_list() and Metaspace::get_chunk_manager(). I
>> believe that at least the check in get_space_list could use a similar
>> check. The class_space_list is not initialized if !use_class_space(),
>> right?
>
> You are correct in that the class_space_list is not initialized in that case.
> Several callers of get_space_list and get_chunk_manager depend on those
> functions returning NULL for ClassType when there is no compressed class space.
>
> The pattern mostly used to get the amount of memory used, see for example the
> call chain for getting the number of committed bytes:
>
> size_t MetaspaceAux::committed_bytes() {
>    return committed_bytes(Metaspace::ClassType) +
>           committed_bytes(Metaspace::NonClassType);
> }
>                    =>
> size_t MetaspaceAux::committed_bytes(Metaspace::MetadataType mdtype) {
>    VirtualSpaceList* list = Metaspace::get_space_list(mdtype);
>    return list == NULL ? 0 : list->committed_bytes();
> }
>
> If I change get_space_list and get_chunk_manager I'll need to change this
> pattern all over the place.
>
> Additionally, the reason I named the new function "is_class_space_allocation" is
> that I wanted to it to answer the question "if I were to allocate memory of this
> type, would it go to the compressed class space?".
> The accessors for SpaceManager and ChunkManager are used in a context where the
> callers have more knowledge of the underlying details and I don't think they
> should hide the fact that there may not be a compressed class space.
>
>
> /Mikael
>
>> /Jesper
>>
>>
>> Mikael Gerdin skrev 26/9/13 9:26 AM:
>>> Hi
>>>
>>> Please review this small fix for throwing OutOfMermoryError with the
>>> correct
>>> cause string when failing to allocate a Klass.
>>>
>>> The code incorrectly sets the cause string to "Compressed class space"
>>> if the
>>> allocation type is ClassType even if the VM is configured to not use a
>>> separate
>>> compressed class space.
>>>
>>> The fix adds a helper function to determine if a MetadataType would be
>>> allocated
>>> in the compressed class space or not and uses that to figure out what
>>> string to
>>> send and which OOME to throw.
>>>
>>> Webrev: http://cr.openjdk.java.net/~mgerdin/8025279/webrev.0
>>> Bug link: https://bugs.openjdk.java.net/browse/JDK-8025279
>