On Thu, 29 Aug 2024 16:17:09 GMT, Stefan Karlsson <stef...@openjdk.org> wrote:
>> Please review this cleanup, where we rename `MEMFLAGS` to `MemType`. >> >> `MEMFLAGS` implies that we can use more than one at the same time, but those >> are exclusive values, so `MemType` is much more suitable name. >> >> There is a bunch of other related cleanup that we can do, but I will leave >> for follow up issues such as [NMT: rename NMTUtil::flag to >> NMTUtil::type](https://bugs.openjdk.org/browse/JDK-8337836) > > I much prefer to see MemType, but I'm warming up to NMTCategory. > > - MemType: Succinct - matches part of the code (E.g. the mt in mtGC) > - MemTypeFlag: Too many words for my preference. > - NMTCat: Meuw. :) > - NMTCategory: Parts of the code call these categories, so I'm not entirely > against this. > - NMTGroup: "Group" is a new name for this that currently isn't reflected at > all in the code. > - NMT_MemType: I think we should try get rid of names using this style. > - NMT::MemType: The `::` makes all function declarations noisier for very > little benefit, IMO. I continue to mostly agree with @stefank. I think this name shouldn't be considered in isolation. There are already a bunch of "NMT_" prefixed names. That's the common idiom for things like this (often (maybe even usually?) without the "_"). Why are we proposing to adopt a new style. (For just this? That would be weird. Or more broadly? That certainly needs more discussion.) Implementation question: Where does the NMT "namespace" come from? Presumably the enum definition is going to be wrapped up in an AllStatic class? A similar effect can be achieved using a `namespace`, but HotSpot avoids using those except in some specific cases that don't apply to this type in isolation. But if we're going to consider the broader scope (as we should), then a namespace might well be appropriate. I'd prefer namespaces (if we were to start using them) use snake_case style naming myself, so "nmt::MemType". ------------- PR Comment: https://git.openjdk.org/jdk/pull/20497#issuecomment-2318768006
