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

Reingruber, Richard richard.reingruber at sap.com
Sat Sep 12 22:54:57 UTC 2020


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.


More information about the hotspot-runtime-dev mailing list