RFR: Implementation of JEP 387: Elastic Metaspace (round two)

[Coleen Phillimore]

I think Leo's original point that a few of the names are too generic was 
a good one:

http://cr.openjdk.java.net/~stuefe/jep387/review/2020-08-11/webrev-all/webrev/src/hotspot/share/memory/metaspace/counter.hpp.html

This should probably be memRangeCounter.hpp.  There's an RFE to make 
this a useful utility, so this should be good for now.

http://cr.openjdk.java.net/~stuefe/jep387/review/2020-08-11/webrev-all/webrev/src/hotspot/share/memory/metaspace/internStat.hpp.html

This should be internalStats.hpp

http://cr.openjdk.java.net/~stuefe/jep387/review/2020-08-11/webrev-all/webrev/src/hotspot/share/memory/metaspace/settings.hpp.html

Maybe Settings should be prefixed with Metaspace like MetaspaceSettings 
- and metaspaceSettings.hpp even though it's already in namespace 
metaspace.  Like metaspaceContext.hpp/cpp.

Thanks,
Coleen



On 9/14/20 7:29 AM, Leo Korinth wrote:
> I am okay with reverting. I am very sorry for creating this mess for 
> you Thomas. Hopefully it will be solved in the build system in the 
> future.
>
> Thanks,
> Leo
>
> On 14/09/2020 06:10, David Holmes wrote:
>> +1
>>
>> No need to prefix all new metaspace/* files with ms.
>>
>> Thanks,
>> David
>>
>> On 13/09/2020 8:54 am, Reingruber, Richard wrote:
>>> Hi Thomas,
>>>
>>> I’d think the 'ms' prefix is not really needed. There are quite a 
>>> few gc implementations and there it may be helpful to add a prefix 
>>> to the source files. But for Metaspace the distinction by directory 
>>> should be sufficient.
>>>
>>> Just my .02€ :)
>>> Richard.
>>>
>>> From: Thomas Stüfe <thomas.stuefe at gmail.com>
>>> Sent: Samstag, 12. September 2020 12:24
>>> To: Coleen Phillimore <coleen.phillimore at oracle.com>; Leo Korinth 
>>> <leo.korinth at oracle.com>; Reingruber, Richard 
>>> <richard.reingruber at sap.com>; Doerr, Martin <martin.doerr at sap.com>; 
>>> Albert Yang <albert.m.yang at oracle.com>
>>> Cc: Hotspot dev runtime <hotspot-runtime-dev at openjdk.java.net>; 
>>> Hotspot-Gc-Dev <hotspot-gc-dev at openjdk.java.net>
>>> Subject: Re: RFR: Implementation of JEP 387: Elastic Metaspace 
>>> (round two)
>>>
>>> Hi Reviewers,
>>>
>>> about the file renamings with the ms prefix, could I ask for more 
>>> opinions? I am fine with both new and old variant, but at the moment 
>>> it's a tie, since Coleen's and Leo's requests contradict each other.
>>>
>>> Thanks a lot!
>>>
>>> .:Thomas
>>> 1) Aesthetics
>>>
>>> Some of you requested style changes so there are a lot:
>>>
>>> - Most files in memory/metaspace/ now follow a common theme with a 
>>> common prefix ("ms"). The only exception is 
>>> metaspacesSizesSnapshot.(cpp|hpp) which I plan to remove in a follow 
>>> up (see JDK-8251342).
>>>
>>>
>>> I really don't like the ms prefix on all the files at all.  I didn't 
>>> think there were any conflicting names in the metaspace directory 
>>> but if there were, they could be renamed.  Now the class names don't 
>>> match the file names!  The other thing about sort of generic names 
>>> in the metspace directory, even if they don't match other names in 
>>> the system, like binList.hpp or blockTree.hpp, is that they can 
>>> provide a hint to people before they write similar code.  These 
>>> could be a basis for generalization into the utilities directory if 
>>> possible.
>>>
>>> http://cr.openjdk.java.net/~stuefe/jep387/review-2020-09-04/webrev-all/webrev/src/hotspot/share/memory/metaspace/msContext.cpp.html 
>>>
>>>
>>> For example in this case, the class name is MetaspaceContext which 
>>> is a much better name than MSContext.
>>>
>>> Sorry I didn't object to this sooner on the thread.
>>>