RFR(xs): 8199431: Split up class Metaspace into a static and a non-static part

[Thomas Stüfe]

Thank you all for the feedback!

New webrev to address Coleens points:

complete:
http://cr.openjdk.java.net/~stuefe/webrevs/8199431-split-class-metaspace-into-two-parts/webrev.00/webrev/

First delta (everything but method reshuffling):
http://cr.openjdk.java.net/~stuefe/webrevs/8199431-split-class-metaspace-into-two-parts/webrev.01.a/webrev/

Second delta (method shuffling):
http://cr.openjdk.java.net/~stuefe/webrevs/8199431-split-class-metaspace-into-two-parts/webrev.01.b/webrev/

Best Regards,

Thomas


On Wed, Mar 14, 2018 at 2:29 PM, Zhengyu Gu <zgu at redhat.com> wrote:

> Other than mentioned by Coleen, looks good to me too.
>
> Thanks,
>
> -Zhengyu
>
>
> On 03/13/2018 06:02 PM, coleen.phillimore at oracle.com wrote:
>
>>
>> http://cr.openjdk.java.net/~stuefe/webrevs/8199431-split-cla
>> ss-metaspace-into-two-parts/webrev.00/webrev/src/hotspot/
>> share/classfile/classLoaderData.hpp.udiff.html
>>
>> Can you remove ro_metaspace and rw_metaspace.  They are not used anymore.
>>
>> + ClassLoaderMetaspace* metaspace_or_null() const { return _metaspace; }
>>
>>
>> Can you remove extra spaces after const since this doesn't line up with
>> anything.
>>
>> http://cr.openjdk.java.net/~stuefe/webrevs/8199431-split-cla
>> ss-metaspace-into-two-parts/webrev.00/webrev/src/hotspot/
>> share/memory/metaspace.cpp.udiff.html
>>
>>   //  SpaceManager - used by Metaspace to handle allocations
>>   class SpaceManager : public CHeapObj<mtClass> {
>>     friend class Metaspace;
>> + friend class ClassLoaderMetaspace;
>>
>> Metaspace probably doesn't need to be friends with this, does it?
>>
>> This all makes sense.  In metaspace.cpp are the ClassLoaderMetaspace
>> functions mixed up with Metaspace functions now?
>>
>> Can you provide a .02 patch that just moves the functions together for
>> each type with this change?
>>
>> This looks like a very good change.
>> Thanks,
>> Coleen
>>
>>
>> On 3/12/18 3:22 AM, Thomas Stüfe wrote:
>>
>>> Hi all,
>>>
>>> I please have reviews and a sponsor for this small change. This is
>>> another
>>> breakout from a breakout from 8185034.
>>>
>>> In short, this splits the Metaspace class into two classes, one holding
>>> the
>>> actual metaspace memory for a single class loader, one serving as a
>>> static
>>> namespace for metaspace-central static functions. This dual role made the
>>> code more difficult to read and was a source for errors (see e.g.
>>> 8199007).
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8199431
>>> webrev: http://cr.openjdk.java.net/~stuefe/webrevs/8199431-split-cla
>>> ss-metaspace-into-two-parts/webrev.00/webrev
>>>
>>> Note: may only apply atop of this patch:  http://cr.openjdk.java.net/~st
>>> uefe/webrevs/8199430-rename-metaspace-aux/webrev.00/webrev/
>>>
>>> Thanks, Thomas
>>>
>>
>>