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

Zhengyu Gu zgu at redhat.com
Wed Mar 14 13:29:24 UTC 2018


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-class-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-class-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
> 


More information about the hotspot-runtime-dev mailing list