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.) It is precisely because we already are using `NMT_TrackingStackDepth` and `NMT_TrackingLevel` (and the fact that MemType by itself was too general) that I suggested we adopt `NMT::` Yes, it would be all static class. Why not all lower letter `nmt::`? Again, because we already use `NMT_` elsewhere. My plan was later to switch from `NMT_` to `NMT::` for all of them. We can do `nmt::` too. So how about `MemTag`, `nmt::MemTag` or `NMT::MemTag`? ------------- PR Comment: https://git.openjdk.org/jdk/pull/20497#issuecomment-2319470647
